From: Allison Henderson <allison.henderson@oracle.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v13 09/10] xfs: Remove unused xfs_attr_*_args
Date: Thu, 12 Nov 2020 18:27:03 -0700 [thread overview]
Message-ID: <134ece78-8466-f322-eb06-5782353d015b@oracle.com> (raw)
In-Reply-To: <20201110200701.GD9695@magnolia>
On 11/10/20 1:07 PM, Darrick J. Wong wrote:
> On Thu, Oct 22, 2020 at 11:34:34PM -0700, Allison Henderson wrote:
>> Remove xfs_attr_set_args, xfs_attr_remove_args, and xfs_attr_trans_roll.
>> These high level loops are now driven by the delayed operations code,
>> and can be removed.
>>
>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
>> ---
>> fs/xfs/libxfs/xfs_attr.c | 97 +----------------------------------------
>> fs/xfs/libxfs/xfs_attr.h | 9 ++--
>> fs/xfs/libxfs/xfs_attr_remote.c | 4 +-
>> 3 files changed, 5 insertions(+), 105 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index edd5d10..b5e1e84 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -262,65 +262,6 @@ xfs_attr_set_shortform(
>> }
>>
>> /*
>> - * Checks to see if a delayed attribute transaction should be rolled. If so,
>> - * also checks for a defer finish. Transaction is finished and rolled as
>> - * needed, and returns true of false if the delayed operation should continue.
>> - */
>> -STATIC int
>> -xfs_attr_trans_roll(
>> - struct xfs_delattr_context *dac)
>> -{
>> - struct xfs_da_args *args = dac->da_args;
>> - int error = 0;
>> -
>> - if (dac->flags & XFS_DAC_DEFER_FINISH) {
>> - /*
>> - * The caller wants us to finish all the deferred ops so that we
>> - * avoid pinning the log tail with a large number of deferred
>> - * ops.
>> - */
>> - dac->flags &= ~XFS_DAC_DEFER_FINISH;
>> - error = xfs_defer_finish(&args->trans);
>> - if (error)
>> - return error;
>> - }
>> -
>> - return xfs_trans_roll_inode(&args->trans, args->dp);
>> -}
>> -
>> -/*
>> - * Set the attribute specified in @args.
>> - */
>> -int
>> -xfs_attr_set_args(
>> - struct xfs_da_args *args)
>> -{
>> - struct xfs_buf *leaf_bp = NULL;
>> - int error = 0;
>> - struct xfs_delattr_context dac = {
>> - .da_args = args,
>> - };
>> -
>> - do {
>> - error = xfs_attr_set_iter(&dac, &leaf_bp);
>
> Now that there's only one caller of xfs_attr_set_iter and it passes
> &dac->leaf_bp, I think you can get rid of this second parameter, right?
>
> It's nice to see so much code disappear now that we track attr
> operations with deferred ops. Everything else looks ok here. :)
>
> --D
Sure, I will collapse that parameter in then.
Allison
>
>> - if (error != -EAGAIN)
>> - break;
>> -
>> - error = xfs_attr_trans_roll(&dac);
>> - if (error)
>> - return error;
>> -
>> - if (leaf_bp) {
>> - xfs_trans_bjoin(args->trans, leaf_bp);
>> - xfs_trans_bhold(args->trans, leaf_bp);
>> - }
>> -
>> - } while (true);
>> -
>> - return error;
>> -}
>> -
>> -/*
>> * Set the attribute specified in @args.
>> * This routine is meant to function as a delayed operation, and may return
>> * -EAGAIN when the transaction needs to be rolled. Calling functions will need
>> @@ -363,11 +304,7 @@ xfs_attr_set_iter(
>> * continue. Otherwise, is it converted from shortform to leaf
>> * and -EAGAIN is returned.
>> */
>> - error = xfs_attr_set_shortform(args, leaf_bp);
>> - if (error == -EAGAIN)
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> -
>> - return error;
>> + return xfs_attr_set_shortform(args, leaf_bp);
>> }
>>
>> /*
>> @@ -398,7 +335,6 @@ xfs_attr_set_iter(
>> * same state (inode locked and joined, transaction
>> * clean) no matter how we got to this step.
>> */
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> return -EAGAIN;
>> case 0:
>> dac->dela_state = XFS_DAS_FOUND_LBLK;
>> @@ -455,32 +391,6 @@ xfs_has_attr(
>>
>> /*
>> * Remove the attribute specified in @args.
>> - */
>> -int
>> -xfs_attr_remove_args(
>> - struct xfs_da_args *args)
>> -{
>> - int error = 0;
>> - struct xfs_delattr_context dac = {
>> - .da_args = args,
>> - };
>> -
>> - do {
>> - error = xfs_attr_remove_iter(&dac);
>> - if (error != -EAGAIN)
>> - break;
>> -
>> - error = xfs_attr_trans_roll(&dac);
>> - if (error)
>> - return error;
>> -
>> - } while (true);
>> -
>> - return error;
>> -}
>> -
>> -/*
>> - * Remove the attribute specified in @args.
>> *
>> * This function may return -EAGAIN to signal that the transaction needs to be
>> * rolled. Callers should continue calling this function until they receive a
>> @@ -895,7 +805,6 @@ xfs_attr_leaf_addname(
>> if (error)
>> return error;
>>
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> return -EAGAIN;
>> }
>>
>> @@ -1192,7 +1101,6 @@ xfs_attr_node_addname(
>> * Restart routine from the top. No need to set the
>> * state
>> */
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> return -EAGAIN;
>> }
>>
>> @@ -1205,7 +1113,6 @@ xfs_attr_node_addname(
>> error = xfs_da3_split(state);
>> if (error)
>> goto out;
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> } else {
>> /*
>> * Addition succeeded, update Btree hashvals.
>> @@ -1246,7 +1153,6 @@ xfs_attr_node_addname(
>> if (error)
>> return error;
>>
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> dac->dela_state = XFS_DAS_ALLOC_NODE;
>> return -EAGAIN;
>> }
>> @@ -1516,7 +1422,6 @@ xfs_attr_node_remove_step(
>> if (error)
>> return error;
>>
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> dac->dela_state = XFS_DAS_RM_SHRINK;
>> return -EAGAIN;
>> }
>> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
>> index 8a08411..6d90301 100644
>> --- a/fs/xfs/libxfs/xfs_attr.h
>> +++ b/fs/xfs/libxfs/xfs_attr.h
>> @@ -244,10 +244,9 @@ enum xfs_delattr_state {
>> /*
>> * Defines for xfs_delattr_context.flags
>> */
>> -#define XFS_DAC_DEFER_FINISH 0x01 /* finish the transaction */
>> -#define XFS_DAC_NODE_RMVNAME_INIT 0x02 /* xfs_attr_node_removename init */
>> -#define XFS_DAC_LEAF_ADDNAME_INIT 0x04 /* xfs_attr_leaf_addname init*/
>> -#define XFS_DAC_DELAYED_OP_INIT 0x08 /* delayed operations init*/
>> +#define XFS_DAC_NODE_RMVNAME_INIT 0x01 /* xfs_attr_node_removename init */
>> +#define XFS_DAC_LEAF_ADDNAME_INIT 0x02 /* xfs_attr_leaf_addname init*/
>> +#define XFS_DAC_DELAYED_OP_INIT 0x04 /* delayed operations init*/
>>
>> /*
>> * Context used for keeping track of delayed attribute operations
>> @@ -297,11 +296,9 @@ int xfs_inode_hasattr(struct xfs_inode *ip);
>> int xfs_attr_get_ilocked(struct xfs_da_args *args);
>> int xfs_attr_get(struct xfs_da_args *args);
>> int xfs_attr_set(struct xfs_da_args *args);
>> -int xfs_attr_set_args(struct xfs_da_args *args);
>> int xfs_attr_set_iter(struct xfs_delattr_context *dac,
>> struct xfs_buf **leaf_bp);
>> int xfs_has_attr(struct xfs_da_args *args);
>> -int xfs_attr_remove_args(struct xfs_da_args *args);
>> int xfs_attr_remove_iter(struct xfs_delattr_context *dac);
>> bool xfs_attr_namecheck(const void *name, size_t length);
>> void xfs_delattr_context_init(struct xfs_delattr_context *dac,
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
>> index 45c4bc5..262d1870 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.c
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
>> @@ -751,10 +751,8 @@ xfs_attr_rmtval_remove(
>> if (error)
>> return error;
>>
>> - if (!done) {
>> - dac->flags |= XFS_DAC_DEFER_FINISH;
>> + if (!done)
>> return -EAGAIN;
>> - }
>>
>> return error;
>> }
>> --
>> 2.7.4
>>
next prev parent reply other threads:[~2020-11-13 1:29 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-23 6:34 [PATCH v13 00/10] xfs: Delayed Attributes Allison Henderson
2020-10-23 6:34 ` [PATCH v13 01/10] xfs: Add helper xfs_attr_node_remove_step Allison Henderson
2020-10-27 7:03 ` Chandan Babu R
2020-10-27 22:23 ` Allison Henderson
2020-10-27 12:15 ` Brian Foster
2020-10-27 15:33 ` Allison Henderson
2020-11-10 23:12 ` Darrick J. Wong
2020-11-13 1:38 ` Allison Henderson
2020-10-23 6:34 ` [PATCH v13 02/10] xfs: Add delay ready attr remove routines Allison Henderson
2020-10-27 9:59 ` Chandan Babu R
2020-10-27 15:32 ` Allison Henderson
2020-10-28 12:04 ` Chandan Babu R
2020-10-29 1:29 ` Allison Henderson
2020-11-14 0:53 ` Darrick J. Wong
2020-10-27 12:16 ` Brian Foster
2020-10-27 22:27 ` Allison Henderson
2020-10-28 12:28 ` Brian Foster
2020-10-29 1:03 ` Allison Henderson
2020-11-10 23:15 ` Darrick J. Wong
2020-11-10 23:43 ` Darrick J. Wong
2020-11-11 0:28 ` Dave Chinner
2020-11-13 4:00 ` Allison Henderson
2020-11-13 3:43 ` Allison Henderson
2020-11-14 1:18 ` Darrick J. Wong
2020-11-16 5:12 ` Allison Henderson
2020-10-23 6:34 ` [PATCH v13 03/10] xfs: Add delay ready attr set routines Allison Henderson
2020-10-27 13:32 ` Chandan Babu R
2020-11-10 21:57 ` Darrick J. Wong
2020-11-13 1:33 ` Allison Henderson
2020-11-13 9:16 ` Chandan Babu R
2020-11-13 17:12 ` Allison Henderson
2020-11-14 1:20 ` Darrick J. Wong
2020-11-10 23:10 ` Darrick J. Wong
2020-11-13 1:38 ` Allison Henderson
2020-11-14 1:35 ` Darrick J. Wong
2020-11-16 5:25 ` Allison Henderson
2020-10-23 6:34 ` [PATCH v13 04/10] xfs: Rename __xfs_attr_rmtval_remove Allison Henderson
2020-10-23 6:34 ` [PATCH v13 05/10] xfs: Set up infastructure for deferred attribute operations Allison Henderson
2020-11-10 21:51 ` Darrick J. Wong
2020-11-11 3:44 ` Darrick J. Wong
2020-11-13 17:06 ` Allison Henderson
2020-11-13 1:32 ` Allison Henderson
2020-11-14 2:00 ` Darrick J. Wong
2020-11-16 7:41 ` Allison Henderson
2020-10-23 6:34 ` [PATCH v13 06/10] xfs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred Allison Henderson
2020-11-10 20:15 ` Darrick J. Wong
2020-11-13 1:27 ` Allison Henderson
2020-11-14 2:03 ` Darrick J. Wong
2020-10-23 6:34 ` [PATCH v13 07/10] xfs: Add feature bit XFS_SB_FEAT_INCOMPAT_LOG_DELATTR Allison Henderson
2020-11-10 20:10 ` Darrick J. Wong
2020-11-13 1:27 ` Allison Henderson
2020-11-19 2:36 ` Darrick J. Wong
2020-11-19 4:01 ` Allison Henderson
2020-10-23 6:34 ` [PATCH v13 08/10] xfs: Enable delayed attributes Allison Henderson
2020-10-23 6:34 ` [PATCH v13 09/10] xfs: Remove unused xfs_attr_*_args Allison Henderson
2020-11-10 20:07 ` Darrick J. Wong
2020-11-13 1:27 ` Allison Henderson [this message]
2020-10-23 6:34 ` [PATCH v13 10/10] xfs: Add delayed attributes error tag Allison Henderson
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=134ece78-8466-f322-eb06-5782353d015b@oracle.com \
--to=allison.henderson@oracle.com \
--cc=darrick.wong@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.