Re: [PATCH v5 08/14] xfs: Factor up xfs_attr_try_sf_addname

Allison Collins
2019-12-14T06:59:09Z

On 12/13/19 7:15 AM, Brian Foster wrote:
> On Wed, Dec 11, 2019 at 09:15:07PM -0700, Allison Collins wrote:
>> New delayed attribute routines cannot handle transactions. We
>> can factor up the commit, but there is little left in this
>> function other than some error handling and an ichgtime. So
>> hoist all of xfs_attr_try_sf_addname up at this time. We will
>> remove all the commits in this set.
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> ---
>> fs/xfs/libxfs/xfs_attr.c | 54 ++++++++++++++++++++----------------------------
>> 1 file changed, 22 insertions(+), 32 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 36f6a43..9c78e0d 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
> ...
>> @@ -258,7 +230,7 @@ xfs_attr_set_args(
>> {
>> struct xfs_inode *dp = args->dp;
>> struct xfs_buf *leaf_bp = NULL;
>> - int error;
>> + int error, error2 = 0;;
>
> Extra semicolon on the above line.
>
>>
>> /*
>> * If the attribute list is non-existent or a shortform list,
>> @@ -277,9 +249,27 @@ xfs_attr_set_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;
>> +
>> + error = xfs_attr_shortform_addname(args);
>> +
>> + /* Should only be 0, -EEXIST or ENOSPC */
>> + if (error != -ENOSPC) {
>> + /*
>> + * Commit the shortform mods, and we're done.
>> + * NOTE: this is also the error path (EEXIST, etc).
>> + */
>> + if (!error && (args->name.type & ATTR_KERNOTIME) == 0)
>> + xfs_trans_ichgtime(args->trans, dp,
>> + XFS_ICHGTIME_CHG);
>> +
>> + 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;
>> + }
>> +
>
> And extra whitespace here. With those nits fixed:
>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
>
Ok, will do. Thank you!

Allison

>>
>> /*
>> * It won't fit in the shortform, transform to a leaf block.
>> --
>> 2.7.4
>>
>