Re: [PATCH v11 10/25] xfs: Refactor xfs_attr_rmtval_remove

Allison Collins
2020-07-25T00:08:19Z

On 7/21/20 5:24 PM, Darrick J. Wong wrote:
> On Tue, Jul 21, 2020 at 04:31:18PM -0700, Darrick J. Wong wrote:
>> On Mon, Jul 20, 2020 at 05:15:51PM -0700, Allison Collins wrote:
>>> Refactor xfs_attr_rmtval_remove to add helper function
>>> __xfs_attr_rmtval_remove. We will use this later when we introduce
>>> delayed attributes. This function will eventually replace
>>> xfs_attr_rmtval_remove
>>>
>>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>>> Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>
>>> Reviewed-by: Brian Foster <bfoster@redhat.com>
>>> ---
>>> fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++---------
>>> fs/xfs/libxfs/xfs_attr_remote.h | 1 +
>>> 2 files changed, 37 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
>>> index 4d51969..9b4c173 100644
>>> --- a/fs/xfs/libxfs/xfs_attr_remote.c
>>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
>>> @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove(
>>> xfs_dablk_t lblkno;
>>> int blkcnt;
>>> int error = 0;
>>> - int done = 0;
>>> + int retval = 0;
>>>
>>> trace_xfs_attr_rmtval_remove(args);
>>>
>>> @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove(
>>> */
>>> lblkno = args->rmtblkno;
>>> blkcnt = args->rmtblkcnt;
>>
>> Er... I think these local variables can go away here, right?
>>
>> --D
>>
>>> - while (!done) {
>>> - error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
>>> - XFS_BMAPI_ATTRFORK, 1, &done);
>>> - if (error)
>>> - return error;
>>> - error = xfs_defer_finish(&args->trans);
>>> - if (error)
>>> - return error;
>>> + do {
>>> + retval = __xfs_attr_rmtval_remove(args);
>>> + if (retval && retval != EAGAIN)
>
> Also this has to be -EAGAIN. Amazingly, nothing in fstests blew up on
> this.
Ok, will fix! If you are running with the full set, it wouldnt trip over
anything because this whole function is removed by the end of the series.

Allison

>
> --D
>
>>> + return retval;
>>>
>>> /*
>>> * Close out trans and start the next one in the chain.
>>> @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove(
>>> error = xfs_trans_roll_inode(&args->trans, args->dp);
>>> if (error)
>>> return error;
>>> - }
>>> + } while (retval == -EAGAIN);
>>> +
>>> return 0;
>>> }
>>> +
>>> +/*
>>> + * Remove the value associated with an attribute by deleting the out-of-line
>>> + * buffer that it is stored on. Returns EAGAIN for the caller to refresh the
>>> + * transaction and re-call the function
>>> + */
>>> +int
>>> +__xfs_attr_rmtval_remove(
>>> + struct xfs_da_args *args)
>>> +{
>>> + int error, done;
>>> +
>>> + /*
>>> + * Unmap value blocks for this attr.
>>> + */
>>> + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno,
>>> + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done);
>>> + if (error)
>>> + return error;
>>> +
>>> + error = xfs_defer_finish(&args->trans);
>>> + if (error)
>>> + return error;
>>> +
>>> + if (!done)
>>> + return -EAGAIN;
>>> +
>>> + return error;
>>> +}
>>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h
>>> index 3616e88..9eee615 100644
>>> --- a/fs/xfs/libxfs/xfs_attr_remote.h
>>> +++ b/fs/xfs/libxfs/xfs_attr_remote.h
>>> @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args);
>>> int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map,
>>> xfs_buf_flags_t incore_flags);
>>> int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
>>> +int __xfs_attr_rmtval_remove(struct xfs_da_args *args);
>>> #endif /* __XFS_ATTR_REMOTE_H__ */
>>> --
>>> 2.7.4
>>>