Re: [PATCH v14 14/15] xfs: Add delattr mount option

[Allison Henderson <allison.henderson@xxxxxxxxxx>]

On 1/4/21 10:46 PM, Darrick J. Wong wrote:
> On Fri, Dec 18, 2020 at 12:29:16AM -0700, Allison Henderson wrote:
> > This patch adds a mount option to enable delayed attributes. Eventually
> > this can be removed when delayed attrs becomes permanent.
> >
> > Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> > ---
> >   fs/xfs/libxfs/xfs_attr.h | 2 +-
> >   fs/xfs/xfs_mount.h       | 1 +
> >   fs/xfs/xfs_super.c       | 6 +++++-
> >   fs/xfs/xfs_xattr.c       | 2 ++
> >   4 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> > index 4838094..edd008d 100644
> > --- a/fs/xfs/libxfs/xfs_attr.h
> > +++ b/fs/xfs/libxfs/xfs_attr.h
> > @@ -30,7 +30,7 @@ struct xfs_attr_list_context;
> >   
> >   static inline bool xfs_hasdelattr(struct xfs_mount *mp)
>
> /me had a brain fart just now that ... since struct xfs_delattr_context
> is ultimately going to be absorbed into struct xfs_attr_item, we really
> should have called the control knob part of this 'logattr' instead of
> 'delattr', because that's (IMIO) a better explanation of what the mount
> option actually does for users.
That's fine, honestly I figured I'd just throw some name out there just 
to get it working initially, and if someone wants a different name, 
they'd say so.  It is a temporary option after all.  :-)

> An even better name would have been "logged attributes replayable"
> because then you could use the prefix XFS_LARP for things. :P
Yeah, I think the name scheme was something we mulled about a while ago, 
though didn't really have a solid opinion on yet.  But we did feel that 
DAS and DAC are sort of close to DA and DAX.

I am ok with LARP.  I'll probably end up mistakenly referring to it as a 
"Log Action Re-Play", but I'm fine with that.  :-)  Just as long as 
everyone else is. Names seem to be something that everyone is really 
opinionated on, and it peppers little changes all over the set, so it 
would be nice to have a semi solid consensus  :-)

Thanks for the reviews!

Allison

> Comments? :)
>
> --D
>
>
> >   {
> > -	return false;
> > +	return mp->m_flags & XFS_MOUNT_DELATTR;
> >   }
> >   
> >   /*
> > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > index dfa429b..4794f27 100644
> > --- a/fs/xfs/xfs_mount.h
> > +++ b/fs/xfs/xfs_mount.h
> > @@ -254,6 +254,7 @@ typedef struct xfs_mount {
> >   #define XFS_MOUNT_NOATTR2	(1ULL << 25)	/* disable use of attr2 format */
> >   #define XFS_MOUNT_DAX_ALWAYS	(1ULL << 26)
> >   #define XFS_MOUNT_DAX_NEVER	(1ULL << 27)
> > +#define XFS_MOUNT_DELATTR	(1ULL << 28)	/* enable delayed attributes */
> >   
> >   /*
> >    * Max and min values for mount-option defined I/O
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 813be87..72169ee 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -92,7 +92,7 @@ enum {
> >   	Opt_filestreams, Opt_quota, Opt_noquota, Opt_usrquota, Opt_grpquota,
> >   	Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
> >   	Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
> > -	Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum,
> > +	Opt_discard, Opt_nodiscard, Opt_dax, Opt_dax_enum, Opt_delattr
> >   };
> >   
> >   static const struct fs_parameter_spec xfs_fs_parameters[] = {
> > @@ -137,6 +137,7 @@ static const struct fs_parameter_spec xfs_fs_parameters[] = {
> >   	fsparam_flag("nodiscard",	Opt_nodiscard),
> >   	fsparam_flag("dax",		Opt_dax),
> >   	fsparam_enum("dax",		Opt_dax_enum, dax_param_enums),
> > +	fsparam_flag("delattr",		Opt_delattr),
> >   	{}
> >   };
> >   
> > @@ -1292,6 +1293,9 @@ xfs_fs_parse_param(
> >   		xfs_mount_set_dax_mode(mp, result.uint_32);
> >   		return 0;
> >   #endif
> > +	case Opt_delattr:
> > +		mp->m_flags |= XFS_MOUNT_DELATTR;
> > +		return 0;
> >   	/* Following mount options will be removed in September 2025 */
> >   	case Opt_ikeep:
> >   		xfs_warn(mp, "%s mount option is deprecated.", param->key);
> > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
> > index 9b0c790..8ec61df 100644
> > --- a/fs/xfs/xfs_xattr.c
> > +++ b/fs/xfs/xfs_xattr.c
> > @@ -8,6 +8,8 @@
> >   #include "xfs_shared.h"
> >   #include "xfs_format.h"
> >   #include "xfs_log_format.h"
> > +#include "xfs_trans_resv.h"
> > +#include "xfs_mount.h"
> >   #include "xfs_da_format.h"
> >   #include "xfs_inode.h"
> >   #include "xfs_da_btree.h"
> > --
> > 2.7.4