Re: [PATCH v6 13/16] xfs: Add helper function xfs_attr_rmtval_unmap

Allison Collins
2020-01-22T05:23:12Z

On 1/21/20 4:24 PM, Darrick J. Wong wrote:
> On Sat, Jan 18, 2020 at 03:50:32PM -0700, Allison Collins wrote:
>> This function is similar to xfs_attr_rmtval_remove, but adapted to
>> return. EAGAIN for new transactions. We will use this later when we
>
> "..to return -EAGAIN"?
yees, I noticed that after I sent it... will fix :-)
>
>> introduce delayed attributes
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> ---
>> fs/xfs/libxfs/xfs_attr_remote.c | 27 +++++++++++++++++++++++++++
>> fs/xfs/libxfs/xfs_attr_remote.h | 1 +
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
>> index 9b4fa2a..f2ee0b8 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.c
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
>> @@ -684,3 +684,30 @@ xfs_attr_rmtval_remove(
>> }
>> return 0;
>> }
>> +
>> +/*
>> + * Unmap value blocks for this attr. This is similar to
>> + * xfs_attr_rmtval_remove, but adapted to to return EAGAIN for new transactions
note to self: fix too many to's

>
> Do you have to scan for and invalidate any incore buffers for the remote
> value? Or will that be performed by another step? Hard to tell because
> this function doesn't have any callers.
Yes, that happens in xfs_attr_rmtval_invalidate. Right after we mark
incomplete, and before we fall into the bunmapi loop I mentioned in the
last patch. Basically this function becomes the inside of that loop.

>
>> + */
>> +int
>> +xfs_attr_rmtval_unmap(
>> + struct xfs_da_args *args)
>> +{
>> + int error, done;
>> +
>> + /*
>> + * Unmap value blocks for this attr. This is similar to
>> + * xfs_attr_rmtval_remove, but open coded here to return EAGAIN
>> + * for new transactions
>> + */
>> + error = xfs_bunmapi(args->trans, args->dp,
>> + args->rmtblkno, args->rmtblkcnt,
>> + XFS_BMAPI_ATTRFORK, 1, &done);
>> + if (error)
>> + return error;
>> +
>> + if (!done)
>> + return -EAGAIN;
>
> return done ? 0 : -EAGAIN; ?
I believe Christoph asked for the return 0; I am not particular about
it as long a people are in agreement. If people feel strongly one way
or another please chime in.

Allison
>
> --D
>

>> +
>> + return 0;
>> +}
>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h
>> index 85f2593..7ab3770 100644
>> --- a/fs/xfs/libxfs/xfs_attr_remote.h
>> +++ b/fs/xfs/libxfs/xfs_attr_remote.h
>> @@ -12,4 +12,5 @@ int xfs_attr_rmtval_get(struct xfs_da_args *args);
>> int xfs_attr_rmtval_set(struct xfs_da_args *args);
>> int xfs_attr_rmtval_remove(struct xfs_da_args *args);
>> int xfs_attr_rmtval_invalidate(struct xfs_da_args *args);
>> +int xfs_attr_rmtval_unmap(struct xfs_da_args *args);
>> #endif /* __XFS_ATTR_REMOTE_H__ */
>> --
>> 2.7.4
>>