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

Allison Collins
2020-07-25T00:07:30Z

On 7/21/20 4:31 PM, 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?
I think so, this whole function sort of get phased out over the last
three patches, so they dont live long. Should be fine to clean out here
though.

Allison

>
> --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)
>> + 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
>>