public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Chandan Rajendra <chandan@linux.ibm.com>
To: linux-xfs@vger.kernel.org
Cc: Allison Collins <allison.henderson@oracle.com>
Subject: Re: [PATCH v7 07/19] xfs: Factor out xfs_attr_leaf_addname helper
Date: Fri, 28 Feb 2020 12:21:22 +0530	[thread overview]
Message-ID: <2411462.H6IQt2SXnH@localhost.localdomain> (raw)
In-Reply-To: <20200223020611.1802-8-allison.henderson@oracle.com>

On Sunday, February 23, 2020 7:35 AM Allison Collins wrote: 
> Factor out new helper function xfs_attr_leaf_try_add. Because new delayed attribute
> routines cannot roll transactions, we carve off the parts of xfs_attr_leaf_addname
> that we can use, and move the commit into the calling function.
>

I don't see any logical errors.

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_attr.c | 88 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 57 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index cf0cba7..b2f0780 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -305,10 +305,30 @@ xfs_attr_set_args(
>  		}
>  	}
>  
> -	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
> +	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
>  		error = xfs_attr_leaf_addname(args);
> -	else
> -		error = xfs_attr_node_addname(args);
> +		if (error != -ENOSPC)
> +			return error;
> +
> +		/*
> +		 * Commit that transaction so that the node_addname()
> +		 * call can manage its own transactions.
> +		 */
> +		error = xfs_defer_finish(&args->trans);
> +		if (error)
> +			return error;
> +
> +		/*
> +		 * Commit the current trans (including the inode) and
> +		 * start a new one.
> +		 */
> +		error = xfs_trans_roll_inode(&args->trans, dp);
> +		if (error)
> +			return error;
> +
> +	}
> +
> +	error = xfs_attr_node_addname(args);
>  	return error;
>  }
>  
> @@ -620,20 +640,21 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
>   *========================================================================*/
>  
>  /*
> - * Add a name to the leaf attribute list structure
> + * Tries to add an attribute to an inode in leaf form
>   *
> - * This leaf block cannot have a "remote" value, we only call this routine
> - * if bmap_one_block() says there is only one block (ie: no remote blks).
> + * This function is meant to execute as part of a delayed operation and leaves
> + * the transaction handling to the caller.  On success the attribute is added
> + * and the inode and transaction are left dirty.  If there is not enough space,
> + * the attr data is converted to node format and -ENOSPC is returned. Caller is
> + * responsible for handling the dirty inode and transaction or adding the attr
> + * in node format.
>   */
>  STATIC int
> -xfs_attr_leaf_addname(
> -	struct xfs_da_args	*args)
> +xfs_attr_leaf_try_add(
> +	struct xfs_da_args	*args,
> +	struct xfs_buf		*bp)
>  {
> -	struct xfs_buf		*bp;
> -	int			retval, error, forkoff;
> -	struct xfs_inode	*dp = args->dp;
> -
> -	trace_xfs_attr_leaf_addname(args);
> +	int			retval, error;
>  
>  	/*
>  	 * Look up the given attribute in the leaf block.  Figure out if
> @@ -679,31 +700,36 @@ xfs_attr_leaf_addname(
>  	retval = xfs_attr3_leaf_add(bp, args);
>  	if (retval == -ENOSPC) {
>  		/*
> -		 * Promote the attribute list to the Btree format, then
> -		 * Commit that transaction so that the node_addname() call
> -		 * can manage its own transactions.
> +		 * Promote the attribute list to the Btree format.
> +		 * Unless an error occurs, retain the -ENOSPC retval
>  		 */
>  		error = xfs_attr3_leaf_to_node(args);
>  		if (error)
>  			return error;
> -		error = xfs_defer_finish(&args->trans);
> -		if (error)
> -			return error;
> +	}
> +	return retval;
> +}
>  
> -		/*
> -		 * Commit the current trans (including the inode) and start
> -		 * a new one.
> -		 */
> -		error = xfs_trans_roll_inode(&args->trans, dp);
> -		if (error)
> -			return error;
>  
> -		/*
> -		 * Fob the whole rest of the problem off on the Btree code.
> -		 */
> -		error = xfs_attr_node_addname(args);
> +/*
> + * Add a name to the leaf attribute list structure
> + *
> + * This leaf block cannot have a "remote" value, we only call this routine
> + * if bmap_one_block() says there is only one block (ie: no remote blks).
> + */
> +STATIC int
> +xfs_attr_leaf_addname(
> +	struct xfs_da_args	*args)
> +{
> +	int			error, forkoff;
> +	struct xfs_buf		*bp = NULL;
> +	struct xfs_inode	*dp = args->dp;
> +
> +	trace_xfs_attr_leaf_addname(args);
> +
> +	error = xfs_attr_leaf_try_add(args, bp);
> +	if (error)
>  		return error;
> -	}
>  
>  	/*
>  	 * Commit the transaction that added the attr name so that
> 


-- 
chandan




  parent reply	other threads:[~2020-02-28 13:12 UTC|newest]

Thread overview: 135+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-23  2:05 [PATCH v7 00/19] xfs: Delayed Ready Attrs Allison Collins
2020-02-23  2:05 ` [PATCH v7 01/19] xfs: Replace attribute parameters with struct xfs_name Allison Collins
2020-02-23  9:34   ` Amir Goldstein
2020-02-23 16:03     ` Allison Collins
2020-02-25  0:49       ` Dave Chinner
2020-02-24 12:08   ` Chandan Rajendra
2020-02-24 16:25     ` Allison Collins
2020-02-24 13:06   ` Brian Foster
2020-02-24 16:25     ` Allison Collins
2020-02-23  2:05 ` [PATCH v7 02/19] xfs: Embed struct xfs_name in xfs_da_args Allison Collins
2020-02-23 11:54   ` Amir Goldstein
2020-02-23 16:51     ` Allison Collins
2020-02-24  6:50       ` Amir Goldstein
2020-02-24  7:36         ` Allison Collins
2020-02-24  7:43           ` Amir Goldstein
2020-02-25  0:57   ` Dave Chinner
2020-02-25  2:00     ` Allison Collins
2020-02-25  4:06       ` Dave Chinner
2020-02-25  4:19         ` Allison Collins
2020-02-25  4:27           ` Darrick J. Wong
2020-02-25  6:07             ` Allison Collins
2020-02-25  6:30               ` Dave Chinner
2020-02-25 17:21             ` Christoph Hellwig
2020-02-25  6:56   ` Chandan Rajendra
2020-02-25 23:26     ` Allison Collins
2020-02-23  2:05 ` [PATCH v7 03/19] xfs: Add xfs_has_attr and subroutines Allison Collins
2020-02-23 12:20   ` Amir Goldstein
2020-02-23 17:28     ` Allison Collins
2020-02-24  6:58       ` Amir Goldstein
2020-02-25  6:26     ` Dave Chinner
2020-02-25  6:43       ` Amir Goldstein
2020-02-25 22:27         ` Dave Chinner
2020-02-24 13:08   ` Brian Foster
2020-02-24 21:18     ` Allison Collins
2020-02-25 13:25       ` Brian Foster
2020-02-26  2:31         ` Allison Collins
2020-02-25  9:49   ` Chandan Rajendra
2020-02-25 10:15     ` Chandan Rajendra
2020-02-26  2:19       ` Allison Collins
2020-02-26  2:18     ` Allison Collins
2020-02-23  2:05 ` [PATCH v7 04/19] xfs: Check for -ENOATTR or -EEXIST Allison Collins
2020-02-23 12:25   ` Amir Goldstein
2020-02-23 17:33     ` Allison Collins
2020-02-24 13:08   ` Brian Foster
2020-02-24 21:18     ` Allison Collins
2020-02-23  2:05 ` [PATCH v7 05/19] xfs: Factor out new helper functions xfs_attr_rmtval_set Allison Collins
2020-02-25 12:53   ` Chandan Rajendra
2020-02-26  2:20     ` Allison Collins
2020-02-23  2:05 ` [PATCH v7 06/19] xfs: Factor out trans handling in xfs_attr3_leaf_flipflags Allison Collins
2020-02-23 12:30   ` Amir Goldstein
2020-02-23 17:36     ` Allison Collins
2020-02-28  4:56   ` Chandan Rajendra
2020-02-23  2:05 ` [PATCH v7 07/19] xfs: Factor out xfs_attr_leaf_addname helper Allison Collins
2020-02-23 12:42   ` Amir Goldstein
2020-02-23 18:38     ` Allison Collins
2020-02-24  6:38       ` Amir Goldstein
2020-02-24  7:09         ` Allison Collins
2020-02-24  7:27           ` Amir Goldstein
2020-02-25  6:42   ` Dave Chinner
2020-02-25 13:26     ` Brian Foster
2020-02-25 23:26     ` Allison Collins
2020-02-28  6:51   ` Chandan Rajendra [this message]
2020-02-23  2:06 ` [PATCH v7 08/19] xfs: Refactor xfs_attr_try_sf_addname Allison Collins
2020-02-23 13:04   ` Amir Goldstein
2020-02-23 17:51     ` Allison Collins
2020-02-24 13:08   ` Brian Foster
2020-02-24 21:19     ` Allison Collins
2020-02-28  7:42   ` Chandan Rajendra
2020-02-28 18:14     ` Allison Collins
2020-02-23  2:06 ` [PATCH v7 09/19] xfs: Factor out trans roll from xfs_attr3_leaf_setflag Allison Collins
2020-02-28  7:59   ` Chandan Rajendra
2020-02-23  2:06 ` [PATCH v7 10/19] xfs: Factor out xfs_attr_rmtval_invalidate Allison Collins
2020-02-28 10:42   ` Chandan Rajendra
2020-02-23  2:06 ` [PATCH v7 11/19] xfs: Factor out trans roll in xfs_attr3_leaf_clearflag Allison Collins
2020-02-28 10:56   ` Chandan Rajendra
2020-02-23  2:06 ` [PATCH v7 12/19] xfs: Add helper function xfs_attr_rmtval_unmap Allison Collins
2020-02-24 13:40   ` Brian Foster
2020-02-24 21:44     ` Allison Collins
2020-02-25 13:27       ` Brian Foster
2020-02-26  3:29         ` Allison Collins
2020-02-26 13:47           ` Brian Foster
2020-02-25  7:21   ` Dave Chinner
2020-02-25 23:27     ` Allison Collins
2020-02-28 14:22   ` Chandan Rajendra
2020-02-23  2:06 ` [PATCH v7 13/19] xfs: Add delay ready attr remove routines Allison Collins
2020-02-24 15:25   ` Brian Foster
2020-02-24 17:03     ` Brian Foster
2020-02-24 23:14     ` Allison Collins
2020-02-24 23:56       ` Darrick J. Wong
2020-02-25 13:34       ` Brian Foster
2020-02-26  5:36         ` Allison Collins
2020-02-26 13:48           ` Brian Foster
2020-02-26 19:23             ` Allison Collins
2020-02-25  8:57   ` Dave Chinner
2020-02-26  0:57     ` Allison Collins
2020-02-26 22:34       ` Dave Chinner
2020-02-27  4:18         ` Allison Collins
2020-03-03  5:03   ` Chandan Rajendra
2020-03-03  5:40     ` Allison Collins
2020-02-23  2:06 ` [PATCH v7 14/19] xfs: Add delay ready attr set routines Allison Collins
2020-03-03 13:41   ` Chandan Rajendra
2020-03-03 17:07     ` Allison Collins
2020-02-23  2:06 ` [PATCH v7 15/19] xfs: Add helper function xfs_attr_node_shrink Allison Collins
2020-02-23 13:22   ` Amir Goldstein
2020-02-23 18:41     ` Allison Collins
2020-02-25  9:05   ` Dave Chinner
2020-02-26  1:48     ` Allison Collins
2020-02-23  2:06 ` [PATCH v7 16/19] xfs: Simplify xfs_attr_set_iter Allison Collins
2020-02-23 13:26   ` Amir Goldstein
2020-02-23 18:42     ` Allison Collins
2020-02-25  9:21   ` Dave Chinner
2020-02-26  2:13     ` Allison Collins
2020-02-26 22:39       ` Dave Chinner
2020-03-04  4:30   ` Chandan Rajendra
2020-03-04 17:04     ` Allison Collins
2020-03-05  3:39       ` Chandan Rajendra
2020-02-23  2:06 ` [PATCH v7 17/19] xfs: Add helper function xfs_attr_leaf_mark_incomplete Allison Collins
2020-02-23 13:47   ` Amir Goldstein
2020-02-23 18:43     ` Allison Collins
2020-02-25  9:31   ` Dave Chinner
2020-02-26  2:17     ` Allison Collins
2020-03-04  4:37   ` Chandan Rajendra
2020-02-23  2:06 ` [PATCH v7 18/19] xfs: Add remote block helper functions Allison Collins
2020-02-23 13:45   ` Amir Goldstein
2020-03-04  4:59   ` Chandan Rajendra
2020-02-23  2:06 ` [PATCH v7 19/19] xfs: Remove xfs_attr_rmtval_remove Allison Collins
2020-02-23 13:54   ` Amir Goldstein
2020-02-23 18:50     ` Allison Collins
2020-02-23  7:55 ` [PATCH v7 00/19] xfs: Delayed Ready Attrs Amir Goldstein
2020-02-23 16:02   ` Allison Collins
2020-02-24  6:30     ` Amir Goldstein
2020-02-24 16:23       ` Allison Collins
2020-02-25  5:53         ` Amir Goldstein
2020-02-24  8:31   ` Chandan Rajendra
2020-02-25  9:52   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2411462.H6IQt2SXnH@localhost.localdomain \
    --to=chandan@linux.ibm.com \
    --cc=allison.henderson@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox