xfs
[Top] [All Lists]

Re: [PATCH 1/3] Revert "xfs: block allocation work needs to be kswapd aw

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/3] Revert "xfs: block allocation work needs to be kswapd aware"
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Fri, 11 Jul 2014 08:32:10 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1405034779-2028-2-git-send-email-david@xxxxxxxxxxxxx>
References: <1405034779-2028-1-git-send-email-david@xxxxxxxxxxxxx> <1405034779-2028-2-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Fri, Jul 11, 2014 at 09:26:17AM +1000, Dave Chinner wrote:
> This reverts commit 1f6d64829db78a7e1d63e15c9f48f0a5d2b5a679.
> 
> This commit resulted in regressions in performance in low
> memory situations where kswapd was doing writeback of delayed
> allocation blocks. It resulted in significant parallelism of the
> kswapd work and with the special kswapd flags meant that hundreds of
> active allocation could dip into kswapd specific memory reserves and
> avoid being throttled. This cause a large amount of performance
> variation, as well as random OOM-killer invocations that didn't
> previously exist.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---

I was going to suggest to keep the bool types, but that's fixed up in
the subsequent patch. ;) Looks good...

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/xfs/xfs_bmap_util.c | 16 +++-------------
>  fs/xfs/xfs_bmap_util.h | 13 ++++++-------
>  2 files changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 703b3ec..057f671 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -258,23 +258,14 @@ xfs_bmapi_allocate_worker(
>       struct xfs_bmalloca     *args = container_of(work,
>                                               struct xfs_bmalloca, work);
>       unsigned long           pflags;
> -     unsigned long           new_pflags = PF_FSTRANS;
>  
> -     /*
> -      * we are in a transaction context here, but may also be doing work
> -      * in kswapd context, and hence we may need to inherit that state
> -      * temporarily to ensure that we don't block waiting for memory reclaim
> -      * in any way.
> -      */
> -     if (args->kswapd)
> -             new_pflags |= PF_MEMALLOC | PF_SWAPWRITE | PF_KSWAPD;
> -
> -     current_set_flags_nested(&pflags, new_pflags);
> +     /* we are in a transaction context here */
> +     current_set_flags_nested(&pflags, PF_FSTRANS);
>  
>       args->result = __xfs_bmapi_allocate(args);
>       complete(args->done);
>  
> -     current_restore_flags_nested(&pflags, new_pflags);
> +     current_restore_flags_nested(&pflags, PF_FSTRANS);
>  }
>  
>  /*
> @@ -293,7 +284,6 @@ xfs_bmapi_allocate(
>  
>  
>       args->done = &done;
> -     args->kswapd = current_is_kswapd();
>       INIT_WORK_ONSTACK(&args->work, xfs_bmapi_allocate_worker);
>       queue_work(xfs_alloc_wq, &args->work);
>       wait_for_completion(&done);
> diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
> index 075f722..935ed2b 100644
> --- a/fs/xfs/xfs_bmap_util.h
> +++ b/fs/xfs/xfs_bmap_util.h
> @@ -50,13 +50,12 @@ struct xfs_bmalloca {
>       xfs_extlen_t            total;  /* total blocks needed for xaction */
>       xfs_extlen_t            minlen; /* minimum allocation size (blocks) */
>       xfs_extlen_t            minleft; /* amount must be left after alloc */
> -     bool                    eof;    /* set if allocating past last extent */
> -     bool                    wasdel; /* replacing a delayed allocation */
> -     bool                    userdata;/* set if is user data */
> -     bool                    aeof;   /* allocated space at eof */
> -     bool                    conv;   /* overwriting unwritten extents */
> -     bool                    stack_switch;
> -     bool                    kswapd; /* allocation in kswapd context */
> +     char                    eof;    /* set if allocating past last extent */
> +     char                    wasdel; /* replacing a delayed allocation */
> +     char                    userdata;/* set if is user data */
> +     char                    aeof;   /* allocated space at eof */
> +     char                    conv;   /* overwriting unwritten extents */
> +     char                    stack_switch;
>       int                     flags;
>       struct completion       *done;
>       struct work_struct      work;
> -- 
> 2.0.0
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>