On Tue, Oct 24, 2023 at 08:09:04AM -0700, Darrick J. Wong wrote: > > + /* > > + * Make the stable writes flag match that of the device the inode > > + * resides on when flipping the RT flag. > > + */ > > + if (S_ISREG(VFS_I(ip)->i_mode) && > > + XFS_IS_REALTIME_INODE(ip) != (fa->fsx_xflags & FS_XFLAG_REALTIME)) > > + xfs_update_stable_writes(ip); > > Hmm. Won't the masking operation here result in the if test comparing 0 > or FS_XFLAG_REALTIME to 0 or 1? > > Oh. FS_XFLAG_REALTIME == 1, so that's not an issue in this one case. > That's a bit subtle though, I'd have preferred > > XFS_IS_REALTIME_INODE(ip) != !!(fa->fsx_xflags & FS_XFLAG_REALTIME)) > > to make it more obvious that the if test isn't comparing apples to > oranges. This is all copy and pasted from a check a few lines above :) I guess I could clean it up as well or even add a rt_flag_changed local variable instead of duplicating the check. > > + /* > > + * For real-time inodes update the stable write flags to that of the RT > > + * device instead of the data device. > > + */ > > + if (S_ISREG(inode->i_mode) && XFS_IS_REALTIME_INODE(ip)) > > + xfs_update_stable_writes(ip); > > I wonder if xfs_update_stable_writes should become an empty function for > the CONFIG_XFS_RT=n case, to avoid the atomic flags update? > > (The extra code is probably not worth the microoptimization.) The compiler already eliminates the code because XFS_IS_REALTIME_INODE(ip) has a stub for CONFIG_XFS_RT=n that always returns 0.
