Re: [PATCH v11 14/25] xfs: Remove xfs_trans_roll in xfs_attr_node_removename

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

On 7/21/20 4:38 PM, Darrick J. Wong wrote:
> On Mon, Jul 20, 2020 at 05:15:55PM -0700, Allison Collins wrote:
>> A transaction roll is not necessary immediately after setting the
>> INCOMPLETE flag when removing a node xattr entry with remote value
>> blocks. The remote block invalidation that immediately follows setting
>> the flag is an in-core only change. The next step after that is to start
>> unmapping the remote blocks from the attr fork, but the xattr remove
>> transaction reservation includes reservation for full tree splits of the
>> dabtree and bmap tree. The remote block unmap code will roll the
>> transaction as extents are unmapped and freed.
>>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> Reviewed-by: Brian Foster <bfoster@redhat.com>
>
> Urrrk. The analysis is correct here, but whoooee was it hard to find.
I know, its a lot of explaining for what looks like such a small change,
but better to have it than not I think
>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Thank you!

Allison

>
> --D
>
>> ---
>> fs/xfs/libxfs/xfs_attr.c | 4 ----
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 1a78023..f1becca 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1148,10 +1148,6 @@ xfs_attr_node_removename(
>> if (error)
>> goto out;
>>
>> - error = xfs_trans_roll_inode(&args->trans, args->dp);
>> - if (error)
>> - goto out;
>> -
>> error = xfs_attr_rmtval_invalidate(args);
>> if (error)
>> return error;
>> --
>> 2.7.4
>>