On Tue, Jun 07, 2022 at 02:03:08PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > I found a race involving the larp control knob, aka the debugging knob > that lets developers enable logging of extended attribute updates: > > Thread 1 Thread 2 > > echo 0 > /sys/fs/xfs/debug/larp > setxattr(REPLACE) > xfs_has_larp (returns false) > xfs_attr_set > > echo 1 > /sys/fs/xfs/debug/larp > > xfs_attr_defer_replace > xfs_attr_init_replace_state > xfs_has_larp (returns true) > xfs_attr_init_remove_state > > <oops, wrong DAS state!> > > This isn't a particularly severe problem right now because xattr logging > is only enabled when CONFIG_XFS_DEBUG=y, and developers *should* know > what they're doing. > > However, the eventual intent is that callers should be able to ask for > the assistance of the log in persisting xattr updates. This capability > might not be required for /all/ callers, which means that dynamic > control must work correctly. Once an xattr update has decided whether > or not to use logged xattrs, it needs to stay in that mode until the end > of the operation regardless of what subsequent parallel operations might > do. > > Therefore, it is an error to continue sampling xfs_globals.larp once > xfs_attr_change has made a decision about larp, and it was not correct > for me to have told Allison that ->create_intent functions can sample > the global log incompat feature bitfield to decide to elide a log item. > > Instead, create a new op flag for the xfs_da_args structure, and convert > all other callers of xfs_has_larp and xfs_sb_version_haslogxattrs within > the attr update state machine to look for the operations flag. > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> Looks fine now. Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> -- Dave Chinner david@xxxxxxxxxxxxx
