Re: [PATCH v5 06/14] xfs: Factor up trans handling in xfs_attr3_leaf_flipflags

Allison Collins
2019-12-25T20:49:36Z

On 12/24/19 5:16 AM, Christoph Hellwig wrote:
> What does "factor up" mean? Basically this moves the
> xfs_trans_roll_inode from xfs_attr3_leaf_flipflags to the callers,
> so I'd expect the subject to mention that.
Yes, that is what I meant for it to mean. I guess I've been on a few
other projects that used that terminology, so I didn't think much of it,
and this is the first time someone has indicated confusion over it. I
can certainly change the verbiage if people prefer. How about "Move
up"? "Pull up"?

>
>> + /*
>> + * Commit the flag value change and start the next trans in
>> + * series.
>> + */
>> + error = xfs_trans_roll_inode(&args->trans, args->dp);
>
> Do we really still need these comments?
I guess I brought it with the corresponding code because I wasn't sure
if people would miss it or not. It does seem to be explaining why we're
doing a roll right now. We may need to wait for after the holiday break
for more people to chime in.

>
>> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
>> index ef96971..4fffa84 100644
>> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
>> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
>> @@ -2972,10 +2972,5 @@ xfs_attr3_leaf_flipflags(
>> XFS_DA_LOGRANGE(leaf2, name_rmt, sizeof(*name_rmt)));
>> }
>>
>> - /*
>> - * Commit the flag value change and start the next trans in series.
>> - */
>> - error = xfs_trans_roll_inode(&args->trans, args->dp);
>> -
>> return error;
>
> This can become a
>
> return 0;
>
> now.
>
Sure, will change. Thanks!

Allison