| Message ID | 156652196787.2607.5728754014477374931.stgit@fedora-28 (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: mount API patch series | expand |
On Fri, Aug 23, 2019 at 08:59:27AM +0800, Ian Kent wrote: > The new mount-api uses an array of struct fs_parameter_spec for > parameter parsing, create this table populated with the xfs mount > parameters. > > The new mount-api table definition is wider than the token based > parameter table and interleaving the option description comments > between each table line is much less readable than adding them to > the end of each table entry. So add the option description comment > to each entry line even though it causes quite a few of the entries > to be longer than 80 characters. > > Signed-off-by: Ian Kent <raven@themaw.net> > --- > fs/xfs/xfs_super.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 45 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index f9450235533c..74c88b92ce22 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -38,6 +38,8 @@ > > #include <linux/magic.h> > #include <linux/parser.h> > +#include <linux/fs_context.h> > +#include <linux/fs_parser.h> > > static const struct super_operations xfs_super_operations; > struct bio_set xfs_ioend_bioset; > @@ -47,9 +49,6 @@ static struct kset *xfs_kset; /* top-level xfs sysfs dir */ > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ > #endif > > -/* > - * Table driven mount option parser. > - */ Not sure why this is comment is removed here if the associated code is staying put in this patch..? > enum { > Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize, > Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, Opt_nouuid, > @@ -109,6 +108,49 @@ static const match_table_t tokens = { > {Opt_err, NULL}, > }; > > +static const struct fs_parameter_spec xfs_param_specs[] = { > + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS log buffers */ > + fsparam_string ("logbsize", Opt_logbsize), /* size of XFS log buffers */ > + fsparam_string ("logdev", Opt_logdev), /* log device */ > + fsparam_string ("rtdev", Opt_rtdev), /* realtime I/O device */ > + fsparam_u32 ("biosize", Opt_biosize), /* log2 of preferred buffered io size */ Hmm.. it looks like the difference between numerical values as strings vs. numeric types is whether we support things like unit suffixes (i.e. "64k"). That said, Opt_biosize uses suffix_kstrtoint() similar to Opt_logbsize yet is defined here as a u32 (and Opt_allocsize falls into the same parsing code but is a string). Is that wrong or am I just missing something wrt to the type specifiers here? Brian > + fsparam_flag ("wsync", Opt_wsync), /* safe-mode nfs compatible mount */ > + fsparam_flag ("noalign", Opt_noalign), /* turn off stripe alignment */ > + fsparam_flag ("swalloc", Opt_swalloc), /* turn on stripe width allocation */ > + fsparam_u32 ("sunit", Opt_sunit), /* data volume stripe unit */ > + fsparam_u32 ("swidth", Opt_swidth), /* data volume stripe width */ > + fsparam_flag ("nouuid", Opt_nouuid), /* ignore filesystem UUID */ > + fsparam_flag_no("grpid", Opt_grpid), /* group-ID from parent directory (or not) */ > + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* group-ID from parent directory */ > + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* group-ID from current process */ > + fsparam_string ("allocsize", Opt_allocsize), /* preferred allocation size */ > + fsparam_flag ("norecovery", Opt_norecovery),/* don't run XFS recovery */ > + fsparam_flag ("inode64", Opt_inode64), /* inodes can be allocated anywhere */ > + fsparam_flag ("inode32", Opt_inode32), /* inode allocation limited to XFS_MAXINUMBER_32 */ > + fsparam_flag_no("ikeep", Opt_ikeep), /* do not free (or keep) empty inode clusters */ > + fsparam_flag_no("largeio", Opt_largeio), /* report (or do not report) large I/O sizes in stat() */ > + fsparam_flag_no("attr2", Opt_attr2), /* do (or do not) use attr2 attribute format */ > + fsparam_flag ("filestreams",Opt_filestreams), /* use filestreams allocator */ > + fsparam_flag_no("quota", Opt_quota), /* disk quotas (user) */ > + fsparam_flag ("usrquota", Opt_usrquota), /* user quota enabled */ > + fsparam_flag ("grpquota", Opt_grpquota), /* group quota enabled */ > + fsparam_flag ("prjquota", Opt_prjquota), /* project quota enabled */ > + fsparam_flag ("uquota", Opt_uquota), /* user quota (IRIX variant) */ > + fsparam_flag ("gquota", Opt_gquota), /* group quota (IRIX variant) */ > + fsparam_flag ("pquota", Opt_pquota), /* project quota (IRIX variant) */ > + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* user quota limit enforcement */ > + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* group quota limit enforcement */ > + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* project quota limit enforcement */ > + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* same as uqnoenforce */ > + fsparam_flag_no("discard", Opt_discard), /* Do (or do not) not discard unused blocks */ > + fsparam_flag ("dax", Opt_dax), /* Enable direct access to bdev pages */ > + {} > +}; > + > +static const struct fs_parameter_description xfs_fs_parameters = { > + .name = "XFS", > + .specs = xfs_param_specs, > +}; > > STATIC int > suffix_kstrtoint(const substring_t *s, unsigned int base, int *res) >
On Tue, 2019-08-27 at 08:39 -0400, Brian Foster wrote: > On Fri, Aug 23, 2019 at 08:59:27AM +0800, Ian Kent wrote: > > The new mount-api uses an array of struct fs_parameter_spec for > > parameter parsing, create this table populated with the xfs mount > > parameters. > > > > The new mount-api table definition is wider than the token based > > parameter table and interleaving the option description comments > > between each table line is much less readable than adding them to > > the end of each table entry. So add the option description comment > > to each entry line even though it causes quite a few of the entries > > to be longer than 80 characters. > > > > Signed-off-by: Ian Kent <raven@themaw.net> > > --- > > fs/xfs/xfs_super.c | 48 > > +++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 45 insertions(+), 3 deletions(-) > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > index f9450235533c..74c88b92ce22 100644 > > --- a/fs/xfs/xfs_super.c > > +++ b/fs/xfs/xfs_super.c > > @@ -38,6 +38,8 @@ > > > > #include <linux/magic.h> > > #include <linux/parser.h> > > +#include <linux/fs_context.h> > > +#include <linux/fs_parser.h> > > > > static const struct super_operations xfs_super_operations; > > struct bio_set xfs_ioend_bioset; > > @@ -47,9 +49,6 @@ static struct kset *xfs_kset; /* top- > > level xfs sysfs dir */ > > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs > > attrs */ > > #endif > > > > -/* > > - * Table driven mount option parser. > > - */ You would rather it be kept? Strictly speaking it's still a table driven parser so it could be kept. I just thought it was a useles comment since it doesn't add value in terms of understanding what's going on, IOW I think it's just noise. > > Not sure why this is comment is removed here if the associated code > is > staying put in this patch..? > > > enum { > > Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize, > > Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, > > Opt_nouuid, > > @@ -109,6 +108,49 @@ static const match_table_t tokens = { > > {Opt_err, NULL}, > > }; > > > > +static const struct fs_parameter_spec xfs_param_specs[] = { > > + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS > > log buffers */ > > + fsparam_string ("logbsize", Opt_logbsize), /* size of XFS log > > buffers */ > > + fsparam_string ("logdev", Opt_logdev), /* log device */ > > + fsparam_string ("rtdev", Opt_rtdev), /* realtime I/O > > device */ > > + fsparam_u32 ("biosize", Opt_biosize), /* log2 of > > preferred buffered io size */ > > Hmm.. it looks like the difference between numerical values as > strings > vs. numeric types is whether we support things like unit suffixes > (i.e. > "64k"). That said, Opt_biosize uses suffix_kstrtoint() similar to > Opt_logbsize yet is defined here as a u32 (and Opt_allocsize falls > into > the same parsing code but is a string). Is that wrong or am I just > missing something wrt to the type specifiers here? Dave Chinner (and IIRC Darrick) made roughly comments about the need for human unit values in the parser. I know David has some patches to implement this but I hadn't seen anything posted, he's probably over loaded with higher priority work atm. so this is probably follow up patch fodder. > > Brian > > > + fsparam_flag ("wsync", Opt_wsync), /* safe-mode nfs > > compatible mount */ > > + fsparam_flag ("noalign", Opt_noalign), /* turn off > > stripe alignment */ > > + fsparam_flag ("swalloc", Opt_swalloc), /* turn on > > stripe width allocation */ > > + fsparam_u32 ("sunit", Opt_sunit), /* data volume > > stripe unit */ > > + fsparam_u32 ("swidth", Opt_swidth), /* data volume > > stripe width */ > > + fsparam_flag ("nouuid", Opt_nouuid), /* ignore > > filesystem UUID */ > > + fsparam_flag_no("grpid", Opt_grpid), /* group-ID from > > parent directory (or not) */ > > + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* group-ID from > > parent directory */ > > + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* group-ID from > > current process */ > > + fsparam_string ("allocsize", Opt_allocsize), /* preferred > > allocation size */ > > + fsparam_flag ("norecovery", Opt_norecovery),/* don't run XFS > > recovery */ > > + fsparam_flag ("inode64", Opt_inode64), /* inodes can be > > allocated anywhere */ > > + fsparam_flag ("inode32", Opt_inode32), /* inode > > allocation limited to XFS_MAXINUMBER_32 */ > > + fsparam_flag_no("ikeep", Opt_ikeep), /* do not free (or > > keep) empty inode clusters */ > > + fsparam_flag_no("largeio", Opt_largeio), /* report (or do > > not report) large I/O sizes in stat() */ > > + fsparam_flag_no("attr2", Opt_attr2), /* do (or do not) > > use attr2 attribute format */ > > + fsparam_flag ("filestreams",Opt_filestreams), /* use > > filestreams allocator */ > > + fsparam_flag_no("quota", Opt_quota), /* disk quotas > > (user) */ > > + fsparam_flag ("usrquota", Opt_usrquota), /* user quota > > enabled */ > > + fsparam_flag ("grpquota", Opt_grpquota), /* group quota > > enabled */ > > + fsparam_flag ("prjquota", Opt_prjquota), /* project quota > > enabled */ > > + fsparam_flag ("uquota", Opt_uquota), /* user quota > > (IRIX variant) */ > > + fsparam_flag ("gquota", Opt_gquota), /* group quota > > (IRIX variant) */ > > + fsparam_flag ("pquota", Opt_pquota), /* project quota > > (IRIX variant) */ > > + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* user quota > > limit enforcement */ > > + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* group quota > > limit enforcement */ > > + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* project > > quota limit enforcement */ > > + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* same as > > uqnoenforce */ > > + fsparam_flag_no("discard", Opt_discard), /* Do (or do not) > > not discard unused blocks */ > > + fsparam_flag ("dax", Opt_dax), /* Enable > > direct access to bdev pages */ > > + {} > > +}; > > + > > +static const struct fs_parameter_description xfs_fs_parameters = { > > + .name = "XFS", > > + .specs = xfs_param_specs, > > +}; > > > > STATIC int > > suffix_kstrtoint(const substring_t *s, unsigned int base, int > > *res) > >
On Fri, Aug 30, 2019 at 06:31:58PM +0800, Ian Kent wrote: > On Tue, 2019-08-27 at 08:39 -0400, Brian Foster wrote: > > On Fri, Aug 23, 2019 at 08:59:27AM +0800, Ian Kent wrote: > > > The new mount-api uses an array of struct fs_parameter_spec for > > > parameter parsing, create this table populated with the xfs mount > > > parameters. > > > > > > The new mount-api table definition is wider than the token based > > > parameter table and interleaving the option description comments > > > between each table line is much less readable than adding them to > > > the end of each table entry. So add the option description comment > > > to each entry line even though it causes quite a few of the entries > > > to be longer than 80 characters. > > > > > > Signed-off-by: Ian Kent <raven@themaw.net> > > > --- > > > fs/xfs/xfs_super.c | 48 > > > +++++++++++++++++++++++++++++++++++++++++++++--- > > > 1 file changed, 45 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > > index f9450235533c..74c88b92ce22 100644 > > > --- a/fs/xfs/xfs_super.c > > > +++ b/fs/xfs/xfs_super.c > > > @@ -38,6 +38,8 @@ > > > > > > #include <linux/magic.h> > > > #include <linux/parser.h> > > > +#include <linux/fs_context.h> > > > +#include <linux/fs_parser.h> > > > > > > static const struct super_operations xfs_super_operations; > > > struct bio_set xfs_ioend_bioset; > > > @@ -47,9 +49,6 @@ static struct kset *xfs_kset; /* top- > > > level xfs sysfs dir */ > > > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs > > > attrs */ > > > #endif > > > > > > -/* > > > - * Table driven mount option parser. > > > - */ > > You would rather it be kept? > No, it just seems unrelated to this patch because you're leaving the associated code around. Why not remove it when the rest of the code is ripped out? > Strictly speaking it's still a table driven parser so it > could be kept. > > I just thought it was a useles comment since it doesn't add > value in terms of understanding what's going on, IOW I think > it's just noise. > > > > > Not sure why this is comment is removed here if the associated code > > is > > staying put in this patch..? > > > > > enum { > > > Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize, > > > Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, > > > Opt_nouuid, > > > @@ -109,6 +108,49 @@ static const match_table_t tokens = { > > > {Opt_err, NULL}, > > > }; > > > > > > +static const struct fs_parameter_spec xfs_param_specs[] = { > > > + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS > > > log buffers */ > > > + fsparam_string ("logbsize", Opt_logbsize), /* size of XFS log > > > buffers */ > > > + fsparam_string ("logdev", Opt_logdev), /* log device */ > > > + fsparam_string ("rtdev", Opt_rtdev), /* realtime I/O > > > device */ > > > + fsparam_u32 ("biosize", Opt_biosize), /* log2 of > > > preferred buffered io size */ > > > > Hmm.. it looks like the difference between numerical values as > > strings > > vs. numeric types is whether we support things like unit suffixes > > (i.e. > > "64k"). That said, Opt_biosize uses suffix_kstrtoint() similar to > > Opt_logbsize yet is defined here as a u32 (and Opt_allocsize falls > > into > > the same parsing code but is a string). Is that wrong or am I just > > missing something wrt to the type specifiers here? > > Dave Chinner (and IIRC Darrick) made roughly comments about the > need for human unit values in the parser. > > I know David has some patches to implement this but I hadn't > seen anything posted, he's probably over loaded with higher > priority work atm. so this is probably follow up patch fodder. > Ok, but why are we using different types for Opt_biosize and Opt_allocsize when they're backed by the same underlying field? I.e., from the current code: ... case Opt_allocsize: case Opt_biosize: if (suffix_kstrtoint(args, 10, &iosize)) return -EINVAL; iosizelog = ffs(iosize) - 1; break; ... Brian > > > > Brian > > > > > + fsparam_flag ("wsync", Opt_wsync), /* safe-mode nfs > > > compatible mount */ > > > + fsparam_flag ("noalign", Opt_noalign), /* turn off > > > stripe alignment */ > > > + fsparam_flag ("swalloc", Opt_swalloc), /* turn on > > > stripe width allocation */ > > > + fsparam_u32 ("sunit", Opt_sunit), /* data volume > > > stripe unit */ > > > + fsparam_u32 ("swidth", Opt_swidth), /* data volume > > > stripe width */ > > > + fsparam_flag ("nouuid", Opt_nouuid), /* ignore > > > filesystem UUID */ > > > + fsparam_flag_no("grpid", Opt_grpid), /* group-ID from > > > parent directory (or not) */ > > > + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* group-ID from > > > parent directory */ > > > + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* group-ID from > > > current process */ > > > + fsparam_string ("allocsize", Opt_allocsize), /* preferred > > > allocation size */ > > > + fsparam_flag ("norecovery", Opt_norecovery),/* don't run XFS > > > recovery */ > > > + fsparam_flag ("inode64", Opt_inode64), /* inodes can be > > > allocated anywhere */ > > > + fsparam_flag ("inode32", Opt_inode32), /* inode > > > allocation limited to XFS_MAXINUMBER_32 */ > > > + fsparam_flag_no("ikeep", Opt_ikeep), /* do not free (or > > > keep) empty inode clusters */ > > > + fsparam_flag_no("largeio", Opt_largeio), /* report (or do > > > not report) large I/O sizes in stat() */ > > > + fsparam_flag_no("attr2", Opt_attr2), /* do (or do not) > > > use attr2 attribute format */ > > > + fsparam_flag ("filestreams",Opt_filestreams), /* use > > > filestreams allocator */ > > > + fsparam_flag_no("quota", Opt_quota), /* disk quotas > > > (user) */ > > > + fsparam_flag ("usrquota", Opt_usrquota), /* user quota > > > enabled */ > > > + fsparam_flag ("grpquota", Opt_grpquota), /* group quota > > > enabled */ > > > + fsparam_flag ("prjquota", Opt_prjquota), /* project quota > > > enabled */ > > > + fsparam_flag ("uquota", Opt_uquota), /* user quota > > > (IRIX variant) */ > > > + fsparam_flag ("gquota", Opt_gquota), /* group quota > > > (IRIX variant) */ > > > + fsparam_flag ("pquota", Opt_pquota), /* project quota > > > (IRIX variant) */ > > > + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* user quota > > > limit enforcement */ > > > + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* group quota > > > limit enforcement */ > > > + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* project > > > quota limit enforcement */ > > > + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* same as > > > uqnoenforce */ > > > + fsparam_flag_no("discard", Opt_discard), /* Do (or do not) > > > not discard unused blocks */ > > > + fsparam_flag ("dax", Opt_dax), /* Enable > > > direct access to bdev pages */ > > > + {} > > > +}; > > > + > > > +static const struct fs_parameter_description xfs_fs_parameters = { > > > + .name = "XFS", > > > + .specs = xfs_param_specs, > > > +}; > > > > > > STATIC int > > > suffix_kstrtoint(const substring_t *s, unsigned int base, int > > > *res) > > > >
On Tue, 2019-08-27 at 08:39 -0400, Brian Foster wrote: > On Fri, Aug 23, 2019 at 08:59:27AM +0800, Ian Kent wrote: > > The new mount-api uses an array of struct fs_parameter_spec for > > parameter parsing, create this table populated with the xfs mount > > parameters. > > > > The new mount-api table definition is wider than the token based > > parameter table and interleaving the option description comments > > between each table line is much less readable than adding them to > > the end of each table entry. So add the option description comment > > to each entry line even though it causes quite a few of the entries > > to be longer than 80 characters. > > > > Signed-off-by: Ian Kent <raven@themaw.net> > > --- > > fs/xfs/xfs_super.c | 48 > > +++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 45 insertions(+), 3 deletions(-) > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > index f9450235533c..74c88b92ce22 100644 > > --- a/fs/xfs/xfs_super.c > > +++ b/fs/xfs/xfs_super.c > > @@ -38,6 +38,8 @@ > > > > #include <linux/magic.h> > > #include <linux/parser.h> > > +#include <linux/fs_context.h> > > +#include <linux/fs_parser.h> > > > > static const struct super_operations xfs_super_operations; > > struct bio_set xfs_ioend_bioset; > > @@ -47,9 +49,6 @@ static struct kset *xfs_kset; /* top- > > level xfs sysfs dir */ > > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs > > attrs */ > > #endif > > > > -/* > > - * Table driven mount option parser. > > - */ > > Not sure why this is comment is removed here if the associated code > is > staying put in this patch..? > > > enum { > > Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize, > > Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, > > Opt_nouuid, > > @@ -109,6 +108,49 @@ static const match_table_t tokens = { > > {Opt_err, NULL}, > > }; > > > > +static const struct fs_parameter_spec xfs_param_specs[] = { > > + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS > > log buffers */ > > + fsparam_string ("logbsize", Opt_logbsize), /* size of XFS log > > buffers */ > > + fsparam_string ("logdev", Opt_logdev), /* log device */ > > + fsparam_string ("rtdev", Opt_rtdev), /* realtime I/O > > device */ > > + fsparam_u32 ("biosize", Opt_biosize), /* log2 of > > preferred buffered io size */ > > Hmm.. it looks like the difference between numerical values as > strings > vs. numeric types is whether we support things like unit suffixes > (i.e. > "64k"). That said, Opt_biosize uses suffix_kstrtoint() similar to > Opt_logbsize yet is defined here as a u32 (and Opt_allocsize falls > into > the same parsing code but is a string). Is that wrong or am I just > missing something wrt to the type specifiers here? Right, the distinction appeared to be between those that use a human unit (like 64k etc.) and those that don't. Opt_biosize looked different in that it's described to be "log2 of preferred buffered io size" so I don't think it can be given as a human unit and that also appeared to be the case from the original options table. Do I need to change this somehow? > > Brian > > > + fsparam_flag ("wsync", Opt_wsync), /* safe-mode nfs > > compatible mount */ > > + fsparam_flag ("noalign", Opt_noalign), /* turn off > > stripe alignment */ > > + fsparam_flag ("swalloc", Opt_swalloc), /* turn on > > stripe width allocation */ > > + fsparam_u32 ("sunit", Opt_sunit), /* data volume > > stripe unit */ > > + fsparam_u32 ("swidth", Opt_swidth), /* data volume > > stripe width */ > > + fsparam_flag ("nouuid", Opt_nouuid), /* ignore > > filesystem UUID */ > > + fsparam_flag_no("grpid", Opt_grpid), /* group-ID from > > parent directory (or not) */ > > + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* group-ID from > > parent directory */ > > + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* group-ID from > > current process */ > > + fsparam_string ("allocsize", Opt_allocsize), /* preferred > > allocation size */ > > + fsparam_flag ("norecovery", Opt_norecovery),/* don't run XFS > > recovery */ > > + fsparam_flag ("inode64", Opt_inode64), /* inodes can be > > allocated anywhere */ > > + fsparam_flag ("inode32", Opt_inode32), /* inode > > allocation limited to XFS_MAXINUMBER_32 */ > > + fsparam_flag_no("ikeep", Opt_ikeep), /* do not free (or > > keep) empty inode clusters */ > > + fsparam_flag_no("largeio", Opt_largeio), /* report (or do > > not report) large I/O sizes in stat() */ > > + fsparam_flag_no("attr2", Opt_attr2), /* do (or do not) > > use attr2 attribute format */ > > + fsparam_flag ("filestreams",Opt_filestreams), /* use > > filestreams allocator */ > > + fsparam_flag_no("quota", Opt_quota), /* disk quotas > > (user) */ > > + fsparam_flag ("usrquota", Opt_usrquota), /* user quota > > enabled */ > > + fsparam_flag ("grpquota", Opt_grpquota), /* group quota > > enabled */ > > + fsparam_flag ("prjquota", Opt_prjquota), /* project quota > > enabled */ > > + fsparam_flag ("uquota", Opt_uquota), /* user quota > > (IRIX variant) */ > > + fsparam_flag ("gquota", Opt_gquota), /* group quota > > (IRIX variant) */ > > + fsparam_flag ("pquota", Opt_pquota), /* project quota > > (IRIX variant) */ > > + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* user quota > > limit enforcement */ > > + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* group quota > > limit enforcement */ > > + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* project > > quota limit enforcement */ > > + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* same as > > uqnoenforce */ > > + fsparam_flag_no("discard", Opt_discard), /* Do (or do not) > > not discard unused blocks */ > > + fsparam_flag ("dax", Opt_dax), /* Enable > > direct access to bdev pages */ > > + {} > > +}; > > + > > +static const struct fs_parameter_description xfs_fs_parameters = { > > + .name = "XFS", > > + .specs = xfs_param_specs, > > +}; > > > > STATIC int > > suffix_kstrtoint(const substring_t *s, unsigned int base, int > > *res) > >
On Tue, Sep 17, 2019 at 11:13:20AM +0800, Ian Kent wrote: > On Tue, 2019-08-27 at 08:39 -0400, Brian Foster wrote: > > On Fri, Aug 23, 2019 at 08:59:27AM +0800, Ian Kent wrote: > > > The new mount-api uses an array of struct fs_parameter_spec for > > > parameter parsing, create this table populated with the xfs mount > > > parameters. > > > > > > The new mount-api table definition is wider than the token based > > > parameter table and interleaving the option description comments > > > between each table line is much less readable than adding them to > > > the end of each table entry. So add the option description comment > > > to each entry line even though it causes quite a few of the entries > > > to be longer than 80 characters. > > > > > > Signed-off-by: Ian Kent <raven@themaw.net> > > > --- > > > fs/xfs/xfs_super.c | 48 > > > +++++++++++++++++++++++++++++++++++++++++++++--- > > > 1 file changed, 45 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > > index f9450235533c..74c88b92ce22 100644 > > > --- a/fs/xfs/xfs_super.c > > > +++ b/fs/xfs/xfs_super.c > > > @@ -38,6 +38,8 @@ > > > > > > #include <linux/magic.h> > > > #include <linux/parser.h> > > > +#include <linux/fs_context.h> > > > +#include <linux/fs_parser.h> > > > > > > static const struct super_operations xfs_super_operations; > > > struct bio_set xfs_ioend_bioset; > > > @@ -47,9 +49,6 @@ static struct kset *xfs_kset; /* top- > > > level xfs sysfs dir */ > > > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs > > > attrs */ > > > #endif > > > > > > -/* > > > - * Table driven mount option parser. > > > - */ > > > > Not sure why this is comment is removed here if the associated code > > is > > staying put in this patch..? > > > > > enum { > > > Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize, > > > Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, > > > Opt_nouuid, > > > @@ -109,6 +108,49 @@ static const match_table_t tokens = { > > > {Opt_err, NULL}, > > > }; > > > > > > +static const struct fs_parameter_spec xfs_param_specs[] = { > > > + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS > > > log buffers */ > > > + fsparam_string ("logbsize", Opt_logbsize), /* size of XFS log > > > buffers */ > > > + fsparam_string ("logdev", Opt_logdev), /* log device */ > > > + fsparam_string ("rtdev", Opt_rtdev), /* realtime I/O > > > device */ > > > + fsparam_u32 ("biosize", Opt_biosize), /* log2 of > > > preferred buffered io size */ > > > > Hmm.. it looks like the difference between numerical values as > > strings > > vs. numeric types is whether we support things like unit suffixes > > (i.e. > > "64k"). That said, Opt_biosize uses suffix_kstrtoint() similar to > > Opt_logbsize yet is defined here as a u32 (and Opt_allocsize falls > > into > > the same parsing code but is a string). Is that wrong or am I just > > missing something wrt to the type specifiers here? > > Right, the distinction appeared to be between those that use a human > unit (like 64k etc.) and those that don't. > > Opt_biosize looked different in that it's described to be "log2 of > preferred buffered io size" so I don't think it can be given as a > human unit and that also appeared to be the case from the original > options table. > > Do I need to change this somehow? > I'm not sure. Can you respond to the question in my most recent reply[1] to this patch please? Brian [1] https://lore.kernel.org/linux-xfs/20190830115611.GA25927@bfoster/ > > > > Brian > > > > > + fsparam_flag ("wsync", Opt_wsync), /* safe-mode nfs > > > compatible mount */ > > > + fsparam_flag ("noalign", Opt_noalign), /* turn off > > > stripe alignment */ > > > + fsparam_flag ("swalloc", Opt_swalloc), /* turn on > > > stripe width allocation */ > > > + fsparam_u32 ("sunit", Opt_sunit), /* data volume > > > stripe unit */ > > > + fsparam_u32 ("swidth", Opt_swidth), /* data volume > > > stripe width */ > > > + fsparam_flag ("nouuid", Opt_nouuid), /* ignore > > > filesystem UUID */ > > > + fsparam_flag_no("grpid", Opt_grpid), /* group-ID from > > > parent directory (or not) */ > > > + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* group-ID from > > > parent directory */ > > > + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* group-ID from > > > current process */ > > > + fsparam_string ("allocsize", Opt_allocsize), /* preferred > > > allocation size */ > > > + fsparam_flag ("norecovery", Opt_norecovery),/* don't run XFS > > > recovery */ > > > + fsparam_flag ("inode64", Opt_inode64), /* inodes can be > > > allocated anywhere */ > > > + fsparam_flag ("inode32", Opt_inode32), /* inode > > > allocation limited to XFS_MAXINUMBER_32 */ > > > + fsparam_flag_no("ikeep", Opt_ikeep), /* do not free (or > > > keep) empty inode clusters */ > > > + fsparam_flag_no("largeio", Opt_largeio), /* report (or do > > > not report) large I/O sizes in stat() */ > > > + fsparam_flag_no("attr2", Opt_attr2), /* do (or do not) > > > use attr2 attribute format */ > > > + fsparam_flag ("filestreams",Opt_filestreams), /* use > > > filestreams allocator */ > > > + fsparam_flag_no("quota", Opt_quota), /* disk quotas > > > (user) */ > > > + fsparam_flag ("usrquota", Opt_usrquota), /* user quota > > > enabled */ > > > + fsparam_flag ("grpquota", Opt_grpquota), /* group quota > > > enabled */ > > > + fsparam_flag ("prjquota", Opt_prjquota), /* project quota > > > enabled */ > > > + fsparam_flag ("uquota", Opt_uquota), /* user quota > > > (IRIX variant) */ > > > + fsparam_flag ("gquota", Opt_gquota), /* group quota > > > (IRIX variant) */ > > > + fsparam_flag ("pquota", Opt_pquota), /* project quota > > > (IRIX variant) */ > > > + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* user quota > > > limit enforcement */ > > > + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* group quota > > > limit enforcement */ > > > + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* project > > > quota limit enforcement */ > > > + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* same as > > > uqnoenforce */ > > > + fsparam_flag_no("discard", Opt_discard), /* Do (or do not) > > > not discard unused blocks */ > > > + fsparam_flag ("dax", Opt_dax), /* Enable > > > direct access to bdev pages */ > > > + {} > > > +}; > > > + > > > +static const struct fs_parameter_description xfs_fs_parameters = { > > > + .name = "XFS", > > > + .specs = xfs_param_specs, > > > +}; > > > > > > STATIC int > > > suffix_kstrtoint(const substring_t *s, unsigned int base, int > > > *res) > > > >
On Tue, 2019-09-17 at 08:10 -0400, Brian Foster wrote: > On Tue, Sep 17, 2019 at 11:13:20AM +0800, Ian Kent wrote: > > On Tue, 2019-08-27 at 08:39 -0400, Brian Foster wrote: > > > On Fri, Aug 23, 2019 at 08:59:27AM +0800, Ian Kent wrote: > > > > The new mount-api uses an array of struct fs_parameter_spec for > > > > parameter parsing, create this table populated with the xfs > > > > mount > > > > parameters. > > > > > > > > The new mount-api table definition is wider than the token > > > > based > > > > parameter table and interleaving the option description > > > > comments > > > > between each table line is much less readable than adding them > > > > to > > > > the end of each table entry. So add the option description > > > > comment > > > > to each entry line even though it causes quite a few of the > > > > entries > > > > to be longer than 80 characters. > > > > > > > > Signed-off-by: Ian Kent <raven@themaw.net> > > > > --- > > > > fs/xfs/xfs_super.c | 48 > > > > +++++++++++++++++++++++++++++++++++++++++++++--- > > > > 1 file changed, 45 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > > > index f9450235533c..74c88b92ce22 100644 > > > > --- a/fs/xfs/xfs_super.c > > > > +++ b/fs/xfs/xfs_super.c > > > > @@ -38,6 +38,8 @@ > > > > > > > > #include <linux/magic.h> > > > > #include <linux/parser.h> > > > > +#include <linux/fs_context.h> > > > > +#include <linux/fs_parser.h> > > > > > > > > static const struct super_operations xfs_super_operations; > > > > struct bio_set xfs_ioend_bioset; > > > > @@ -47,9 +49,6 @@ static struct kset *xfs_kset; /* top- > > > > level xfs sysfs dir */ > > > > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs > > > > attrs */ > > > > #endif > > > > > > > > -/* > > > > - * Table driven mount option parser. > > > > - */ > > > > > > Not sure why this is comment is removed here if the associated > > > code > > > is > > > staying put in this patch..? > > > > > > > enum { > > > > Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, > > > > Opt_biosize, > > > > Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, > > > > Opt_swidth, > > > > Opt_nouuid, > > > > @@ -109,6 +108,49 @@ static const match_table_t tokens = { > > > > {Opt_err, NULL}, > > > > }; > > > > > > > > +static const struct fs_parameter_spec xfs_param_specs[] = { > > > > + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS > > > > log buffers */ > > > > + fsparam_string ("logbsize", Opt_logbsize), /* size of XFS > > > > log > > > > buffers */ > > > > + fsparam_string ("logdev", Opt_logdev), /* log device > > > > */ > > > > + fsparam_string ("rtdev", Opt_rtdev), /* realtime I/O > > > > device */ > > > > + fsparam_u32 ("biosize", Opt_biosize), /* log2 of > > > > preferred buffered io size */ > > > > > > Hmm.. it looks like the difference between numerical values as > > > strings > > > vs. numeric types is whether we support things like unit suffixes > > > (i.e. > > > "64k"). That said, Opt_biosize uses suffix_kstrtoint() similar to > > > Opt_logbsize yet is defined here as a u32 (and Opt_allocsize > > > falls > > > into > > > the same parsing code but is a string). Is that wrong or am I > > > just > > > missing something wrt to the type specifiers here? > > > > Right, the distinction appeared to be between those that use a > > human > > unit (like 64k etc.) and those that don't. > > > > Opt_biosize looked different in that it's described to be "log2 of > > preferred buffered io size" so I don't think it can be given as a > > human unit and that also appeared to be the case from the original > > options table. > > > > Do I need to change this somehow? > > > > I'm not sure. Can you respond to the question in my most recent > reply[1] > to this patch please? > > Brian > > [1] https://lore.kernel.org/linux-xfs/20190830115611.GA25927@bfoster/ I see, yes, they are different in the unchanged source. It seems to me that the handling of these should be changed since it looks like Opt_allocsize can have a human unit value while Opt_biosize, given it's listed as a log2 value, it doesn't make sense to call suffix_kstrtoint() on it. Maybe that should be corrected as a pre-patch in the series or forwarded separately ... although I can see how this has gone unnoticed. > > > > Brian > > > > > > > + fsparam_flag ("wsync", Opt_wsync), /* safe-mode nfs > > > > compatible mount */ > > > > + fsparam_flag ("noalign", Opt_noalign), /* turn off > > > > stripe alignment */ > > > > + fsparam_flag ("swalloc", Opt_swalloc), /* turn on > > > > stripe width allocation */ > > > > + fsparam_u32 ("sunit", Opt_sunit), /* data volume > > > > stripe unit */ > > > > + fsparam_u32 ("swidth", Opt_swidth), /* data volume > > > > stripe width */ > > > > + fsparam_flag ("nouuid", Opt_nouuid), /* ignore > > > > filesystem UUID */ > > > > + fsparam_flag_no("grpid", Opt_grpid), /* group-ID > > > > from > > > > parent directory (or not) */ > > > > + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* group-ID from > > > > parent directory */ > > > > + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* group-ID from > > > > current process */ > > > > + fsparam_string ("allocsize", Opt_allocsize), /* preferred > > > > allocation size */ > > > > + fsparam_flag ("norecovery", Opt_norecovery),/* don't run XFS > > > > recovery */ > > > > + fsparam_flag ("inode64", Opt_inode64), /* inodes can be > > > > allocated anywhere */ > > > > + fsparam_flag ("inode32", Opt_inode32), /* inode > > > > allocation limited to XFS_MAXINUMBER_32 */ > > > > + fsparam_flag_no("ikeep", Opt_ikeep), /* do not free > > > > (or > > > > keep) empty inode clusters */ > > > > + fsparam_flag_no("largeio", Opt_largeio), /* report (or > > > > do > > > > not report) large I/O sizes in stat() */ > > > > + fsparam_flag_no("attr2", Opt_attr2), /* do (or do > > > > not) > > > > use attr2 attribute format */ > > > > + fsparam_flag ("filestreams",Opt_filestreams), /* use > > > > filestreams allocator */ > > > > + fsparam_flag_no("quota", Opt_quota), /* disk quotas > > > > (user) */ > > > > + fsparam_flag ("usrquota", Opt_usrquota), /* user quota > > > > enabled */ > > > > + fsparam_flag ("grpquota", Opt_grpquota), /* group quota > > > > enabled */ > > > > + fsparam_flag ("prjquota", Opt_prjquota), /* project quota > > > > enabled */ > > > > + fsparam_flag ("uquota", Opt_uquota), /* user quota > > > > (IRIX variant) */ > > > > + fsparam_flag ("gquota", Opt_gquota), /* group quota > > > > (IRIX variant) */ > > > > + fsparam_flag ("pquota", Opt_pquota), /* project quota > > > > (IRIX variant) */ > > > > + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* user quota > > > > limit enforcement */ > > > > + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* group quota > > > > limit enforcement */ > > > > + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* project > > > > quota limit enforcement */ > > > > + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* same as > > > > uqnoenforce */ > > > > + fsparam_flag_no("discard", Opt_discard), /* Do (or do > > > > not) > > > > not discard unused blocks */ > > > > + fsparam_flag ("dax", Opt_dax), /* Enable > > > > direct access to bdev pages */ > > > > + {} > > > > +}; > > > > + > > > > +static const struct fs_parameter_description xfs_fs_parameters > > > > = { > > > > + .name = "XFS", > > > > + .specs = xfs_param_specs, > > > > +}; > > > > > > > > STATIC int > > > > suffix_kstrtoint(const substring_t *s, unsigned int base, int > > > > *res) > > > >
On Wed, 2019-09-18 at 11:43 +0800, Ian Kent wrote: > On Tue, 2019-09-17 at 08:10 -0400, Brian Foster wrote: > > On Tue, Sep 17, 2019 at 11:13:20AM +0800, Ian Kent wrote: > > > On Tue, 2019-08-27 at 08:39 -0400, Brian Foster wrote: > > > > On Fri, Aug 23, 2019 at 08:59:27AM +0800, Ian Kent wrote: > > > > > The new mount-api uses an array of struct fs_parameter_spec > > > > > for > > > > > parameter parsing, create this table populated with the xfs > > > > > mount > > > > > parameters. > > > > > > > > > > The new mount-api table definition is wider than the token > > > > > based > > > > > parameter table and interleaving the option description > > > > > comments > > > > > between each table line is much less readable than adding > > > > > them > > > > > to > > > > > the end of each table entry. So add the option description > > > > > comment > > > > > to each entry line even though it causes quite a few of the > > > > > entries > > > > > to be longer than 80 characters. > > > > > > > > > > Signed-off-by: Ian Kent <raven@themaw.net> > > > > > --- > > > > > fs/xfs/xfs_super.c | 48 > > > > > +++++++++++++++++++++++++++++++++++++++++++++--- > > > > > 1 file changed, 45 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > > > > > index f9450235533c..74c88b92ce22 100644 > > > > > --- a/fs/xfs/xfs_super.c > > > > > +++ b/fs/xfs/xfs_super.c > > > > > @@ -38,6 +38,8 @@ > > > > > > > > > > #include <linux/magic.h> > > > > > #include <linux/parser.h> > > > > > +#include <linux/fs_context.h> > > > > > +#include <linux/fs_parser.h> > > > > > > > > > > static const struct super_operations xfs_super_operations; > > > > > struct bio_set xfs_ioend_bioset; > > > > > @@ -47,9 +49,6 @@ static struct kset *xfs_kset; > > > > > /* top- > > > > > level xfs sysfs dir */ > > > > > static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs > > > > > attrs */ > > > > > #endif > > > > > > > > > > -/* > > > > > - * Table driven mount option parser. > > > > > - */ > > > > > > > > Not sure why this is comment is removed here if the associated > > > > code > > > > is > > > > staying put in this patch..? > > > > > > > > > enum { > > > > > Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, > > > > > Opt_biosize, > > > > > Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, > > > > > Opt_swidth, > > > > > Opt_nouuid, > > > > > @@ -109,6 +108,49 @@ static const match_table_t tokens = { > > > > > {Opt_err, NULL}, > > > > > }; > > > > > > > > > > +static const struct fs_parameter_spec xfs_param_specs[] = { > > > > > + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS > > > > > log buffers */ > > > > > + fsparam_string ("logbsize", Opt_logbsize), /* size of > > > > > XFS > > > > > log > > > > > buffers */ > > > > > + fsparam_string ("logdev", Opt_logdev), /* log device > > > > > */ > > > > > + fsparam_string ("rtdev", Opt_rtdev), /* realtime > > > > > I/O > > > > > device */ > > > > > + fsparam_u32 ("biosize", Opt_biosize), /* log2 of > > > > > preferred buffered io size */ > > > > > > > > Hmm.. it looks like the difference between numerical values as > > > > strings > > > > vs. numeric types is whether we support things like unit > > > > suffixes > > > > (i.e. > > > > "64k"). That said, Opt_biosize uses suffix_kstrtoint() similar > > > > to > > > > Opt_logbsize yet is defined here as a u32 (and Opt_allocsize > > > > falls > > > > into > > > > the same parsing code but is a string). Is that wrong or am I > > > > just > > > > missing something wrt to the type specifiers here? > > > > > > Right, the distinction appeared to be between those that use a > > > human > > > unit (like 64k etc.) and those that don't. > > > > > > Opt_biosize looked different in that it's described to be "log2 > > > of > > > preferred buffered io size" so I don't think it can be given as a > > > human unit and that also appeared to be the case from the > > > original > > > options table. > > > > > > Do I need to change this somehow? > > > > > > > I'm not sure. Can you respond to the question in my most recent > > reply[1] > > to this patch please? > > > > Brian > > > > [1] > > https://lore.kernel.org/linux-xfs/20190830115611.GA25927@bfoster/ > > I see, yes, they are different in the unchanged source. > > It seems to me that the handling of these should be changed since > it looks like Opt_allocsize can have a human unit value while > Opt_biosize, given it's listed as a log2 value, it doesn't make > sense to call suffix_kstrtoint() on it. > > Maybe that should be corrected as a pre-patch in the series or > forwarded separately ... although I can see how this has gone > unnoticed. Ha, and surely there needs to be a conversion of some sort if they really are different unit values ... Darrick? > > > > > Brian > > > > > > > > > + fsparam_flag ("wsync", Opt_wsync), /* safe- > > > > > mode nfs > > > > > compatible mount */ > > > > > + fsparam_flag ("noalign", Opt_noalign), /* turn > > > > > off > > > > > stripe alignment */ > > > > > + fsparam_flag ("swalloc", Opt_swalloc), /* turn > > > > > on > > > > > stripe width allocation */ > > > > > + fsparam_u32 ("sunit", Opt_sunit), /* data volume > > > > > stripe unit */ > > > > > + fsparam_u32 ("swidth", Opt_swidth), /* data volume > > > > > stripe width */ > > > > > + fsparam_flag ("nouuid", Opt_nouuid), /* > > > > > ignore > > > > > filesystem UUID */ > > > > > + fsparam_flag_no("grpid", Opt_grpid), /* group-ID > > > > > from > > > > > parent directory (or not) */ > > > > > + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* > > > > > group-ID from > > > > > parent directory */ > > > > > + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* > > > > > group-ID from > > > > > current process */ > > > > > + fsparam_string ("allocsize", Opt_allocsize), /* preferred > > > > > allocation size */ > > > > > + fsparam_flag ("norecovery", Opt_norecovery),/* don't > > > > > run XFS > > > > > recovery */ > > > > > + fsparam_flag ("inode64", Opt_inode64), /* > > > > > inodes can be > > > > > allocated anywhere */ > > > > > + fsparam_flag ("inode32", Opt_inode32), /* inode > > > > > allocation limited to XFS_MAXINUMBER_32 */ > > > > > + fsparam_flag_no("ikeep", Opt_ikeep), /* do not > > > > > free > > > > > (or > > > > > keep) empty inode clusters */ > > > > > + fsparam_flag_no("largeio", Opt_largeio), /* report (or > > > > > do > > > > > not report) large I/O sizes in stat() */ > > > > > + fsparam_flag_no("attr2", Opt_attr2), /* do (or do > > > > > not) > > > > > use attr2 attribute format */ > > > > > + fsparam_flag ("filestreams",Opt_filestreams), /* use > > > > > filestreams allocator */ > > > > > + fsparam_flag_no("quota", Opt_quota), /* disk > > > > > quotas > > > > > (user) */ > > > > > + fsparam_flag ("usrquota", Opt_usrquota), /* user > > > > > quota > > > > > enabled */ > > > > > + fsparam_flag ("grpquota", Opt_grpquota), /* group > > > > > quota > > > > > enabled */ > > > > > + fsparam_flag ("prjquota", Opt_prjquota), /* > > > > > project quota > > > > > enabled */ > > > > > + fsparam_flag ("uquota", Opt_uquota), /* user > > > > > quota > > > > > (IRIX variant) */ > > > > > + fsparam_flag ("gquota", Opt_gquota), /* group > > > > > quota > > > > > (IRIX variant) */ > > > > > + fsparam_flag ("pquota", Opt_pquota), /* > > > > > project quota > > > > > (IRIX variant) */ > > > > > + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* > > > > > user quota > > > > > limit enforcement */ > > > > > + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* > > > > > group quota > > > > > limit enforcement */ > > > > > + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* > > > > > project > > > > > quota limit enforcement */ > > > > > + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* > > > > > same as > > > > > uqnoenforce */ > > > > > + fsparam_flag_no("discard", Opt_discard), /* Do (or do > > > > > not) > > > > > not discard unused blocks */ > > > > > + fsparam_flag ("dax", Opt_dax), /* > > > > > Enable > > > > > direct access to bdev pages */ > > > > > + {} > > > > > +}; > > > > > + > > > > > +static const struct fs_parameter_description > > > > > xfs_fs_parameters > > > > > = { > > > > > + .name = "XFS", > > > > > + .specs = xfs_param_specs, > > > > > +}; > > > > > > > > > > STATIC int > > > > > suffix_kstrtoint(const substring_t *s, unsigned int base, > > > > > int > > > > > *res) > > > > >
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index f9450235533c..74c88b92ce22 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -38,6 +38,8 @@ #include <linux/magic.h> #include <linux/parser.h> +#include <linux/fs_context.h> +#include <linux/fs_parser.h> static const struct super_operations xfs_super_operations; struct bio_set xfs_ioend_bioset; @@ -47,9 +49,6 @@ static struct kset *xfs_kset; /* top-level xfs sysfs dir */ static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */ #endif -/* - * Table driven mount option parser. - */ enum { Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize, Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, Opt_nouuid, @@ -109,6 +108,49 @@ static const match_table_t tokens = { {Opt_err, NULL}, }; +static const struct fs_parameter_spec xfs_param_specs[] = { + fsparam_u32 ("logbufs", Opt_logbufs), /* number of XFS log buffers */ + fsparam_string ("logbsize", Opt_logbsize), /* size of XFS log buffers */ + fsparam_string ("logdev", Opt_logdev), /* log device */ + fsparam_string ("rtdev", Opt_rtdev), /* realtime I/O device */ + fsparam_u32 ("biosize", Opt_biosize), /* log2 of preferred buffered io size */ + fsparam_flag ("wsync", Opt_wsync), /* safe-mode nfs compatible mount */ + fsparam_flag ("noalign", Opt_noalign), /* turn off stripe alignment */ + fsparam_flag ("swalloc", Opt_swalloc), /* turn on stripe width allocation */ + fsparam_u32 ("sunit", Opt_sunit), /* data volume stripe unit */ + fsparam_u32 ("swidth", Opt_swidth), /* data volume stripe width */ + fsparam_flag ("nouuid", Opt_nouuid), /* ignore filesystem UUID */ + fsparam_flag_no("grpid", Opt_grpid), /* group-ID from parent directory (or not) */ + fsparam_flag ("bsdgroups", Opt_bsdgroups), /* group-ID from parent directory */ + fsparam_flag ("sysvgroups", Opt_sysvgroups),/* group-ID from current process */ + fsparam_string ("allocsize", Opt_allocsize), /* preferred allocation size */ + fsparam_flag ("norecovery", Opt_norecovery),/* don't run XFS recovery */ + fsparam_flag ("inode64", Opt_inode64), /* inodes can be allocated anywhere */ + fsparam_flag ("inode32", Opt_inode32), /* inode allocation limited to XFS_MAXINUMBER_32 */ + fsparam_flag_no("ikeep", Opt_ikeep), /* do not free (or keep) empty inode clusters */ + fsparam_flag_no("largeio", Opt_largeio), /* report (or do not report) large I/O sizes in stat() */ + fsparam_flag_no("attr2", Opt_attr2), /* do (or do not) use attr2 attribute format */ + fsparam_flag ("filestreams",Opt_filestreams), /* use filestreams allocator */ + fsparam_flag_no("quota", Opt_quota), /* disk quotas (user) */ + fsparam_flag ("usrquota", Opt_usrquota), /* user quota enabled */ + fsparam_flag ("grpquota", Opt_grpquota), /* group quota enabled */ + fsparam_flag ("prjquota", Opt_prjquota), /* project quota enabled */ + fsparam_flag ("uquota", Opt_uquota), /* user quota (IRIX variant) */ + fsparam_flag ("gquota", Opt_gquota), /* group quota (IRIX variant) */ + fsparam_flag ("pquota", Opt_pquota), /* project quota (IRIX variant) */ + fsparam_flag ("uqnoenforce",Opt_uqnoenforce), /* user quota limit enforcement */ + fsparam_flag ("gqnoenforce",Opt_gqnoenforce), /* group quota limit enforcement */ + fsparam_flag ("pqnoenforce",Opt_pqnoenforce), /* project quota limit enforcement */ + fsparam_flag ("qnoenforce", Opt_qnoenforce), /* same as uqnoenforce */ + fsparam_flag_no("discard", Opt_discard), /* Do (or do not) not discard unused blocks */ + fsparam_flag ("dax", Opt_dax), /* Enable direct access to bdev pages */ + {} +}; + +static const struct fs_parameter_description xfs_fs_parameters = { + .name = "XFS", + .specs = xfs_param_specs, +}; STATIC int suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)
The new mount-api uses an array of struct fs_parameter_spec for parameter parsing, create this table populated with the xfs mount parameters. The new mount-api table definition is wider than the token based parameter table and interleaving the option description comments between each table line is much less readable than adding them to the end of each table entry. So add the option description comment to each entry line even though it causes quite a few of the entries to be longer than 80 characters. Signed-off-by: Ian Kent <raven@themaw.net> --- fs/xfs/xfs_super.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-)