On Fri, 2022-06-17 at 07:54 +1000, Dave Chinner wrote: > On Sat, Jun 11, 2022 at 02:41:59AM -0700, Allison Henderson wrote: > > Renames that generate parent pointer updates will need to 2 extra > > defer > > operations. One for the rmap update and another for the parent > > pointer > > update > > Not sure I follow this - defer operation counts are something > tracked in the transaction reservations, whilst this is changing the > number of inodes that are joined and held across defer operations. > > These rmap updates already occur on the directory inodes in a rename > (when the dir update changes the dir shape), so I'm guessing that > you are now talking about changing parent attrs for the child inodes > may require attr fork shape changes (hence rmap updates) due to the > deferred parent pointer xattr update? > > If so, this should be placed in the series before the modifications > to the rename operation is modified to join 4 ops to it, preferably > at the start of the series.... I see, sure, I can move this patch down to the beginning of the set > > > Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx> > > --- > > fs/xfs/libxfs/xfs_defer.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h > > index 114a3a4930a3..0c2a6e537016 100644 > > --- a/fs/xfs/libxfs/xfs_defer.h > > +++ b/fs/xfs/libxfs/xfs_defer.h > > @@ -70,7 +70,7 @@ extern const struct xfs_defer_op_type > > xfs_attr_defer_type; > > /* > > * Deferred operation item relogging limits. > > */ > > -#define XFS_DEFER_OPS_NR_INODES 2 /* join up to two inodes */ > > +#define XFS_DEFER_OPS_NR_INODES 4 /* join up to four inodes > > */ > > The comment is not useful - it should desvribe what operation > requires 4 inodes to be joined. e.g. > > /* > * Rename w/ parent pointers requires 4 indoes with defered ops to > * be joined to the transaction. > */ Sure, will update > > Then, if we are changing the maximum number of inodes that are > joined to a deferred operation, then we need to also update the > locking code such as in xfs_defer_ops_continue() that has to order > locking of multiple inodes correctly. Ok, I see it, I will take a look at updating that > > Also, rename can lock and modify 5 inodes, not 4, so the 4 inodes > that get joined here need to be clearly documented somewhere. Ok, I think its src dir, target dir, src inode, target inode, and then wip. Do we want the documenting in xfs_defer_ops_continue? Or just the commit description? > Also, > xfs_sort_for_rename() that orders all the inodes in rename into > correct locking order in an array, and xfs_lock_inodes() that does > the locking of the inodes in the array. Yes, I see it. You want a comment in xfs_defer_ops_continue referring to the order? Thanks! Allison > > Cheers, > > Dave.
