Re: [PATCH] xfs: refine the allocation stack switch

[Brian Foster <bfoster@redhat.com>]

On Wed, Jun 18, 2014 at 06:55:41PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The allocation stack switch at xfs_bmapi_allocate() has served it's
> purpose, but is no longer a sufficient solution to the stack usage
> problem we have in the XFS allocation path.
> 
...
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c  |  4 +--
>  fs/xfs/libxfs/xfs_btree.c | 84 +++++++++++++++++++++++++++++++++++++++++++++--
>  fs/xfs/xfs_bmap_util.c    | 53 ------------------------------
>  fs/xfs/xfs_bmap_util.h    |  2 --
>  4 files changed, 84 insertions(+), 59 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 56f050e..43a9744 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -249,59 +249,6 @@ xfs_bmap_rtalloc(
>  }
>  
>  /*
> - * Stack switching interfaces for allocation
> - */
> -static void
> -xfs_bmapi_allocate_worker(
> -	struct work_struct	*work)
> -{
> -	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);
> -
> -	args->result = __xfs_bmapi_allocate(args);
> -	complete(args->done);
> -
> -	current_restore_flags_nested(&pflags, new_pflags);
> -}
> -
> -/*
> - * Some allocation requests often come in with little stack to work on. Push
> - * them off to a worker thread so there is lots of stack to use. Otherwise just
> - * call directly to avoid the context switch overhead here.
> - */
> -int
> -xfs_bmapi_allocate(
> -	struct xfs_bmalloca	*args)
> -{
> -	DECLARE_COMPLETION_ONSTACK(done);
> -
> -	if (!args->stack_switch)
> -		return __xfs_bmapi_allocate(args);

Looks like we can kill args->stack_switch up through the controlling
XFS_BMAPI_STACK_SWITCH flag. The rest of the code looks fine to me,
though I need to do some testing...

Brian

> -
> -
> -	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);
> -	destroy_work_on_stack(&args->work);
> -	return args->result;
> -}
> -
> -/*
>   * Check if the endoff is outside the last extent. If so the caller will grow
>   * the allocation to a stripe unit boundary.  All offsets are considered outside
>   * the end of file for an empty fork, so 1 is returned in *eof in that case.
> diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
> index 075f722..5f8abb9 100644
> --- a/fs/xfs/xfs_bmap_util.h
> +++ b/fs/xfs/xfs_bmap_util.h
> @@ -66,8 +66,6 @@ struct xfs_bmalloca {
>  int	xfs_bmap_finish(struct xfs_trans **tp, struct xfs_bmap_free *flist,
>  			int *committed);
>  int	xfs_bmap_rtalloc(struct xfs_bmalloca *ap);
> -int	xfs_bmapi_allocate(struct xfs_bmalloca *args);
> -int	__xfs_bmapi_allocate(struct xfs_bmalloca *args);
>  int	xfs_bmap_eof(struct xfs_inode *ip, xfs_fileoff_t endoff,
>  		     int whichfork, int *eof);
>  int	xfs_bmap_count_blocks(struct xfs_trans *tp, struct xfs_inode *ip,
> -- 
> 2.0.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs