On Mon, Apr 20, 2020 at 05:15:08PM +0530, Chandan Rajendra wrote:
> On Saturday, April 4, 2020 3:42 AM Allison Collins wrote:
...
> > Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_attr.c | 384 +++++++++++++++++++++++++++-------------
> > fs/xfs/libxfs/xfs_attr.h | 16 ++
> > fs/xfs/libxfs/xfs_attr_leaf.c | 1 +
> > fs/xfs/libxfs/xfs_attr_remote.c | 111 +++++++-----
> > fs/xfs/libxfs/xfs_attr_remote.h | 4 +
> > fs/xfs/xfs_attr_inactive.c | 1 +
> > fs/xfs/xfs_trace.h | 1 -
> > 7 files changed, 351 insertions(+), 167 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> > index f700976..c160b7a 100644
> > --- a/fs/xfs/libxfs/xfs_attr.c
> > +++ b/fs/xfs/libxfs/xfs_attr.c
...
> > @@ -765,22 +873,25 @@ xfs_attr_leaf_addname(
> > error = xfs_attr3_leaf_flipflags(args);
> > if (error)
> > return error;
> > - /*
> > - * Commit the flag value change and start the next trans in
> > - * series.
> > - */
> > - error = xfs_trans_roll_inode(&args->trans, args->dp);
> > - if (error)
> > - return error;
> > -
> > + dac->dela_state = XFS_DAS_FLIP_LFLAG;
> > + return -EAGAIN;
> > +das_flip_flag:
> > /*
> > * Dismantle the "old" attribute/value pair by removing
> > * a "remote" value (if it exists).
> > */
> > xfs_attr_restore_rmt_blk(args);
> >
> > + xfs_attr_rmtval_invalidate(args);
> > +das_rm_lblk:
> > if (args->rmtblkno) {
> > - error = xfs_attr_rmtval_remove(args);
> > + error = __xfs_attr_rmtval_remove(args);
> > +
> > + if (error == -EAGAIN) {
> > + dac->dela_state = XFS_DAS_RM_LBLK;
>
> Similar to what I had observed in the patch "Add delay ready attr remove
> routines",
>
> Shouldn't XFS_DAC_DEFER_FINISH be set in dac->flags?
> __xfs_attr_rmtval_remove() calls __xfs_bunmapi() which would
> have added items to the deferred list.
>
Just note that transaction rolls don't currently finish deferred ops. So
from the perspective of preserving current behavior it might make sense
to set the flag here if there was an explicit xfs_defer_finish() that's
been factored out, but not so if it was just a transaction roll.
Brian
> --
> chandan
>
>
>
