Re: [PATCH v7 08/19] xfs: Refactor xfs_attr_try_sf_addname

Allison Collins
2020-02-28T18:16:06Z

On 2/28/20 12:42 AM, Chandan Rajendra wrote:
> On Sunday, February 23, 2020 7:36 AM Allison Collins wrote:
>> To help pre-simplify xfs_attr_set_args, we need to hoist transacation handling up,
>> while modularizing the adjacent code down into helpers. In this patch, hoist the
>> commit in xfs_attr_try_sf_addname up into the calling function, and also pull the
>> attr list creation down.
>>
>
> I don't see any logical errors.
>
> Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>
Alrighty, thank you!

Allison

>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>> ---
>> fs/xfs/libxfs/xfs_attr.c | 30 +++++++++++++++---------------
>> 1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index b2f0780..71298b9 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -227,8 +227,13 @@ xfs_attr_try_sf_addname(
>> struct xfs_da_args *args)
>> {
>>
>> - struct xfs_mount *mp = dp->i_mount;
>> - int error, error2;
>> + int error;
>> +
>> + /*
>> + * Build initial attribute list (if required).
>> + */
>> + if (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS)
>> + xfs_attr_shortform_create(args);
>>
>> error = xfs_attr_shortform_addname(args);
>> if (error == -ENOSPC)
>> @@ -241,12 +246,10 @@ xfs_attr_try_sf_addname(
>> if (!error && (args->name.type & ATTR_KERNOTIME) == 0)
>> xfs_trans_ichgtime(args->trans, dp, XFS_ICHGTIME_CHG);
>>
>> - if (mp->m_flags & XFS_MOUNT_WSYNC)
>> + if (dp->i_mount->m_flags & XFS_MOUNT_WSYNC)
>> xfs_trans_set_sync(args->trans);
>>
>> - error2 = xfs_trans_commit(args->trans);
>> - args->trans = NULL;
>> - return error ? error : error2;
>> + return error;
>> }
>>
>> /*
>> @@ -258,7 +261,7 @@ xfs_attr_set_args(
>> {
>> struct xfs_inode *dp = args->dp;
>> struct xfs_buf *leaf_bp = NULL;
>> - int error;
>> + int error, error2 = 0;
>>
>> /*
>> * If the attribute list is non-existent or a shortform list,
>> @@ -269,17 +272,14 @@ xfs_attr_set_args(
>> dp->i_d.di_anextents == 0)) {
>>
>> /*
>> - * Build initial attribute list (if required).
>> - */
>> - if (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS)
>> - xfs_attr_shortform_create(args);
>> -
>> - /*
>> * Try to add the attr to the attribute list in the inode.
>> */
>> error = xfs_attr_try_sf_addname(dp, args);
>> - if (error != -ENOSPC)
>> - return error;
>> + if (error != -ENOSPC) {
>> + error2 = xfs_trans_commit(args->trans);
>> + args->trans = NULL;
>> + return error ? error : error2;
>> + }
>>
>> /*
>> * It won't fit in the shortform, transform to a leaf block.
>>
>
>