* [PATCH v3 00/16] xfs: mount API patch series
@ 2019-09-20 9:55 Ian Kent
2019-09-20 9:55 ` [PATCH v3 01/16] vfs: Create fs_context-aware mount_bdev() replacement Ian Kent
` (15 more replies)
0 siblings, 16 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:55 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
This patch series add support to xfs for the new kernel mount API
as described in the LWN article at https://lwn.net/Articles/780267/.
In the article there's a lengthy description of the reasons for
adopting the API and problems expected to be resolved by using it.
The series has been applied to the repository located at
git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git, and built and
some simple tests run on it.
Other things that continue to cause me concern:
- Message logging.
There is error logging done in the VFS by the mount-api code, some
is VFS specific while some is file system specific. This can lead
to duplicated and often inconsistent logging.
The mount-api feature of saving error message text to the mount
context for later retrival by fsopen()/fsconfig()/fsmount() users
is the reason the mount-api log macros are present. But, at the
moment (last time I looked), these macros will either log the
error message or save it to the mount context, there's not yet
a way to modify this behaviour which can lead to messages,
possibly needed for debug purposes, being lost. There's also
the pr_xxx() log functions (not a problem for xfs AFAICS) that
aren't aware of the mount context.
Consequently I've tried to keep logging as it is in xfs and
also tried to avoid duplicate log messages, deferring mount
context messaging until more work has been done on mount-api
convesions so that further discussions can take place.
- That large comment about mount option handling.
I'm not sure what to do about that large comment. On one hand
I think there's useful information there and don't really want
to para-phrase it (largely) out of existence, not knowing of
a sensible place to put it so I can refer to it instead.
Changes for v3:
- fix struct xfs_fs_context initialization in xfs_parseargs().
- move call to xfs_validate_params() below label "done".
- if allocation of xfs_mount fails return ENOMEM immediately.
- remove erroneous kfree(mp) in xfs_fill_super().
- move the removal of xfs_fs_remount() and xfs_test_remount_options()
to the switch to mount api patch.
- retain original usage of distinct <option>, no<option> usage.
- fix line length and a white space problem in xfs_parseargs().
- defer introduction of struct fs_context_operations until mount
api implementation.
- don't use a new line for the void parameter of xfs_mount_alloc().
- check for -ENOPARAM in xfs_parse_param() to report invalid options
using the options switch (to avoid double entry log messages).
- remove obolete mount option biosize.
- try and make comment in xfs_fc_free() more understandable.
Changes for v2:
- changed .name to uppercase in fs_parameter_description to ensure
consistent error log messages between the vfs parser and the xfs
parameter parser.
- clearify comment above xfs_parse_param() about when possibly
allocated mp->m_logname or mp->m_rtname are freed.
- factor out xfs_remount_rw() and xfs_remount_ro() from xfs_remount().
- changed xfs_mount_alloc() to not set super block in xfs_mount so it
can be re-used when switching to the mount-api.
- fixed don't check for NULL when calling kfree() in xfs_fc_free().
- refactored xfs_parseargs() in an attempt to highlight the code
that actually changes in converting to use the new mount api.
- dropped xfs-mount-api-rename-xfs_fill_super.patch, it didn't seem
necessary.
- move comment about remount difficulties above xfs_reconfigure()
and increase line length to try and make the comment managable.
Al Viro has sent a pull request to Linus for the patch containing
the vfs_get_block_super() just recently so there will be conflicts
if this series is merged without dropping that patch.
---
David Howells (1):
vfs: Create fs_context-aware mount_bdev() replacement
Ian Kent (15):
xfs: remove very old mount option
xfs: mount-api - add fs parameter description
xfs: mount-api - refactor suffix_kstrtoint()
xfs: mount-api - refactor xfs_parseags()
xfs: mount-api - make xfs_parse_param() take context .parse_param() args
xfs: mount-api - move xfs_parseargs() validation to a helper
xfs: mount-api - refactor xfs_fs_fill_super()
xfs: mount-api - add xfs_get_tree()
xfs: mount-api - add xfs_remount_rw() helper
xfs: mount-api - add xfs_remount_ro() helper
xfs: mount api - add xfs_reconfigure()
xfs: mount-api - add xfs_fc_free()
xfs: mount-api - dont set sb in xfs_mount_alloc()
xfs: mount-api - switch to new mount-api
xfs: mount-api - remove legacy mount functions
fs/fs_context.c | 2
fs/super.c | 111 +++++
fs/xfs/xfs_super.c | 960 ++++++++++++++++++++++++--------------------
include/linux/fs_context.h | 9
4 files changed, 642 insertions(+), 440 deletions(-)
--
Ian
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 01/16] vfs: Create fs_context-aware mount_bdev() replacement
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
@ 2019-09-20 9:55 ` Ian Kent
2019-09-20 9:55 ` [PATCH v3 02/16] xfs: remove very old mount option Ian Kent
` (14 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:55 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
From: David Howells <dhowells@redhat.com>
Create a function, vfs_get_block_super(), that is fs_context-aware and a
replacement for mount_bdev(). It caches the block device pointer and file
open mode in the fs_context struct so that this information can be passed
into sget_fc()'s test and set functions.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: linux-block@vger.kernel.org
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/fs_context.c | 2 +
fs/super.c | 111 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs_context.h | 9 ++++
3 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/fs/fs_context.c b/fs/fs_context.c
index 103643c68e3f..270ecae32216 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -501,6 +501,8 @@ void put_fs_context(struct fs_context *fc)
if (fc->need_free && fc->ops && fc->ops->free)
fc->ops->free(fc);
+ if (fc->dev_destructor)
+ fc->dev_destructor(fc);
security_free_mnt_opts(&fc->security);
put_net(fc->net_ns);
diff --git a/fs/super.c b/fs/super.c
index 113c58f19425..95cf8a2d5b25 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1215,6 +1215,110 @@ int get_tree_single(struct fs_context *fc,
EXPORT_SYMBOL(get_tree_single);
#ifdef CONFIG_BLOCK
+static void fc_bdev_destructor(struct fs_context *fc)
+{
+ if (fc->bdev) {
+ blkdev_put(fc->bdev, fc->bdev_mode);
+ fc->bdev = NULL;
+ }
+}
+
+static int set_bdev_super_fc(struct super_block *s, struct fs_context *fc)
+{
+ s->s_mode = fc->bdev_mode;
+ s->s_bdev = fc->bdev;
+ s->s_dev = s->s_bdev->bd_dev;
+ s->s_bdi = bdi_get(s->s_bdev->bd_bdi);
+ fc->bdev = NULL;
+ return 0;
+}
+
+static int test_bdev_super_fc(struct super_block *s, struct fs_context *fc)
+{
+ return s->s_bdev == fc->bdev;
+}
+
+/**
+ * vfs_get_block_super - Get a superblock based on a single block device
+ * @fc: The filesystem context holding the parameters
+ * @keying: How to distinguish superblocks
+ * @fill_super: Helper to initialise a new superblock
+ */
+int vfs_get_block_super(struct fs_context *fc,
+ int (*fill_super)(struct super_block *,
+ struct fs_context *))
+{
+ struct block_device *bdev;
+ struct super_block *s;
+ int error = 0;
+
+ fc->bdev_mode = FMODE_READ | FMODE_EXCL;
+ if (!(fc->sb_flags & SB_RDONLY))
+ fc->bdev_mode |= FMODE_WRITE;
+
+ if (!fc->source)
+ return invalf(fc, "No source specified");
+
+ bdev = blkdev_get_by_path(fc->source, fc->bdev_mode, fc->fs_type);
+ if (IS_ERR(bdev)) {
+ errorf(fc, "%s: Can't open blockdev", fc->source);
+ return PTR_ERR(bdev);
+ }
+
+ fc->dev_destructor = fc_bdev_destructor;
+ fc->bdev = bdev;
+
+ /* Once the superblock is inserted into the list by sget_fc(), s_umount
+ * will protect the lockfs code from trying to start a snapshot while
+ * we are mounting
+ */
+ mutex_lock(&bdev->bd_fsfreeze_mutex);
+ if (bdev->bd_fsfreeze_count > 0) {
+ mutex_unlock(&bdev->bd_fsfreeze_mutex);
+ warnf(fc, "%pg: Can't mount, blockdev is frozen", bdev);
+ return -EBUSY;
+ }
+
+ fc->sb_flags |= SB_NOSEC;
+ s = sget_fc(fc, test_bdev_super_fc, set_bdev_super_fc);
+ mutex_unlock(&bdev->bd_fsfreeze_mutex);
+ if (IS_ERR(s))
+ return PTR_ERR(s);
+
+ if (s->s_root) {
+ /* Don't summarily change the RO/RW state. */
+ if ((fc->sb_flags ^ s->s_flags) & SB_RDONLY) {
+ warnf(fc, "%pg: Can't mount, would change RO state", bdev);
+ error = -EBUSY;
+ goto error_sb;
+ }
+
+ /* Leave fc->bdev to fc_bdev_destructor() to clean up to avoid
+ * locking conflicts.
+ */
+ } else {
+ snprintf(s->s_id, sizeof(s->s_id), "%pg", bdev);
+ sb_set_blocksize(s, block_size(bdev));
+ error = fill_super(s, fc);
+ if (error)
+ goto error_sb;
+
+ s->s_flags |= SB_ACTIVE;
+ bdev->bd_super = s;
+ }
+
+ BUG_ON(fc->root);
+ fc->root = dget(s->s_root);
+ return 0;
+
+error_sb:
+ deactivate_locked_super(s);
+ /* Leave fc->bdev to fc_bdev_destructor() to clean up */
+ return error;
+}
+EXPORT_SYMBOL(vfs_get_block_super);
+
+
static int set_bdev_super(struct super_block *s, void *data)
{
s->s_bdev = data;
@@ -1414,8 +1518,13 @@ int vfs_get_tree(struct fs_context *fc)
* on the superblock.
*/
error = fc->ops->get_tree(fc);
- if (error < 0)
+ if (error < 0) {
+ if (fc->dev_destructor) {
+ fc->dev_destructor(fc);
+ fc->dev_destructor = NULL;
+ }
return error;
+ }
if (!fc->root) {
pr_err("Filesystem %s get_tree() didn't set fc->root\n",
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 7c6fe3d47fa6..ed5b4349671e 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -88,6 +88,9 @@ struct fs_context {
struct mutex uapi_mutex; /* Userspace access mutex */
struct file_system_type *fs_type;
void *fs_private; /* The filesystem's context */
+ union {
+ struct block_device *bdev; /* The backing blockdev (if applicable) */
+ };
struct dentry *root; /* The root and superblock */
struct user_namespace *user_ns; /* The user namespace for this mount */
struct net *net_ns; /* The network namespace for this mount */
@@ -97,6 +100,7 @@ struct fs_context {
const char *subtype; /* The subtype to set on the superblock */
void *security; /* Linux S&M options */
void *s_fs_info; /* Proposed s_fs_info */
+ fmode_t bdev_mode; /* File open mode for bdev */
unsigned int sb_flags; /* Proposed superblock flags (SB_*) */
unsigned int sb_flags_mask; /* Superblock flags that were changed */
unsigned int s_iflags; /* OR'd with sb->s_iflags */
@@ -105,6 +109,7 @@ struct fs_context {
enum fs_context_phase phase:8; /* The phase the context is in */
bool need_free:1; /* Need to call ops->free() */
bool global:1; /* Goes into &init_user_ns */
+ void (*dev_destructor)(struct fs_context *fc); /* For block or mtd */
};
struct fs_context_operations {
@@ -154,6 +159,10 @@ extern int get_tree_single(struct fs_context *fc,
int (*fill_super)(struct super_block *sb,
struct fs_context *fc));
+extern int vfs_get_block_super(struct fs_context *fc,
+ int (*fill_super)(struct super_block *sb,
+ struct fs_context *fc));
+
extern const struct file_operations fscontext_fops;
/*
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 02/16] xfs: remove very old mount option
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
2019-09-20 9:55 ` [PATCH v3 01/16] vfs: Create fs_context-aware mount_bdev() replacement Ian Kent
@ 2019-09-20 9:55 ` Ian Kent
2019-09-24 10:45 ` Brian Foster
2019-09-20 9:55 ` [PATCH v3 03/16] xfs: mount-api - add fs parameter description Ian Kent
` (13 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:55 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
It appears the biosize mount option hasn't been documented as
a vilid option since 2005.
So remove it.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f9450235533c..1010097354a6 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -51,7 +51,7 @@ static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */
* Table driven mount option parser.
*/
enum {
- Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize,
+ Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev,
Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, Opt_nouuid,
Opt_grpid, Opt_nogrpid, Opt_bsdgroups, Opt_sysvgroups,
Opt_allocsize, Opt_norecovery, Opt_inode64, Opt_inode32, Opt_ikeep,
@@ -67,7 +67,6 @@ static const match_table_t tokens = {
{Opt_logbsize, "logbsize=%s"}, /* size of XFS log buffers */
{Opt_logdev, "logdev=%s"}, /* log device */
{Opt_rtdev, "rtdev=%s"}, /* realtime I/O device */
- {Opt_biosize, "biosize=%u"}, /* log2 of preferred buffered io size */
{Opt_wsync, "wsync"}, /* safe-mode nfs compatible mount */
{Opt_noalign, "noalign"}, /* turn off stripe alignment */
{Opt_swalloc, "swalloc"}, /* turn on stripe width allocation */
@@ -229,7 +228,6 @@ xfs_parseargs(
return -ENOMEM;
break;
case Opt_allocsize:
- case Opt_biosize:
if (suffix_kstrtoint(args, 10, &iosize))
return -EINVAL;
iosizelog = ffs(iosize) - 1;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 03/16] xfs: mount-api - add fs parameter description
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
2019-09-20 9:55 ` [PATCH v3 01/16] vfs: Create fs_context-aware mount_bdev() replacement Ian Kent
2019-09-20 9:55 ` [PATCH v3 02/16] xfs: remove very old mount option Ian Kent
@ 2019-09-20 9:55 ` Ian Kent
2019-09-24 10:45 ` Brian Foster
2019-09-20 9:55 ` [PATCH v3 04/16] xfs: mount-api - refactor suffix_kstrtoint() Ian Kent
` (12 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:55 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
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 | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 1010097354a6..9c1ce3d70c08 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;
@@ -108,6 +110,54 @@ 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_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 ("grpid", Opt_grpid), /* group-ID from parent directory */
+ fsparam_flag ("nogrpid", Opt_nogrpid), /* no group-ID from parent directory */
+ 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 ("ikeep", Opt_ikeep), /* do not free empty inode clusters */
+ fsparam_flag ("noikeep", Opt_noikeep), /* free empty inode clusters */
+ fsparam_flag ("largeio", Opt_largeio), /* report large I/O sizes in stat() */
+ fsparam_flag ("nolargeio", Opt_nolargeio), /* do not report large I/O sizes in stat() */
+ fsparam_flag ("attr2", Opt_attr2), /* do use attr2 attribute format */
+ fsparam_flag ("noattr2", Opt_noattr2), /* do not use attr2 attribute format */
+ fsparam_flag ("filestreams",Opt_filestreams), /* use filestreams allocator */
+ fsparam_flag ("quota", Opt_quota), /* disk quotas (user) */
+ fsparam_flag ("noquota", Opt_noquota), /* no quotas */
+ 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 ("discard", Opt_discard), /* Discard unused blocks */
+ fsparam_flag ("nodiscard", Opt_nodiscard), /* Do 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)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 04/16] xfs: mount-api - refactor suffix_kstrtoint()
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (2 preceding siblings ...)
2019-09-20 9:55 ` [PATCH v3 03/16] xfs: mount-api - add fs parameter description Ian Kent
@ 2019-09-20 9:55 ` Ian Kent
2019-09-24 10:46 ` Brian Foster
2019-09-20 9:55 ` [PATCH v3 05/16] xfs: mount-api - refactor xfs_parseags() Ian Kent
` (11 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:55 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
The mount-api doesn't have a "human unit" parse type yet so
the options that have values like "10k" etc. still need to
be converted by the fs.
But the value comes to the fs as a string (not a substring_t
type) so there's a need to change the conversion function to
take a character string instead.
After refactoring xfs_parseargs() and changing it to use
xfs_parse_param() match_kstrtoint() will no longer be used
and will be removed.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 9c1ce3d70c08..6a16750b1314 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -160,13 +160,13 @@ static const struct fs_parameter_description xfs_fs_parameters = {
};
STATIC int
-suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)
+suffix_kstrtoint(const char *s, unsigned int base, int *res)
{
int last, shift_left_factor = 0, _res;
char *value;
int ret = 0;
- value = match_strdup(s);
+ value = kstrdup(s, GFP_KERNEL);
if (!value)
return -ENOMEM;
@@ -191,6 +191,20 @@ suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)
return ret;
}
+STATIC int
+match_kstrtoint(const substring_t *s, unsigned int base, int *res)
+{
+ const char *value;
+ int ret;
+
+ value = match_strdup(s);
+ if (!value)
+ return -ENOMEM;
+ ret = suffix_kstrtoint(value, base, res);
+ kfree(value);
+ return ret;
+}
+
/*
* This function fills in xfs_mount_t fields based on mount args.
* Note: the superblock has _not_ yet been read in.
@@ -262,7 +276,7 @@ xfs_parseargs(
return -EINVAL;
break;
case Opt_logbsize:
- if (suffix_kstrtoint(args, 10, &mp->m_logbsize))
+ if (match_kstrtoint(args, 10, &mp->m_logbsize))
return -EINVAL;
break;
case Opt_logdev:
@@ -278,7 +292,7 @@ xfs_parseargs(
return -ENOMEM;
break;
case Opt_allocsize:
- if (suffix_kstrtoint(args, 10, &iosize))
+ if (match_kstrtoint(args, 10, &iosize))
return -EINVAL;
iosizelog = ffs(iosize) - 1;
break;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 05/16] xfs: mount-api - refactor xfs_parseags()
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (3 preceding siblings ...)
2019-09-20 9:55 ` [PATCH v3 04/16] xfs: mount-api - refactor suffix_kstrtoint() Ian Kent
@ 2019-09-20 9:55 ` Ian Kent
2019-09-24 10:46 ` Brian Foster
2019-09-20 9:56 ` [PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args Ian Kent
` (10 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:55 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Refactor xfs_parseags(), move the entire token case block to a
separate function in an attempt to highlight the code that
actually changes in converting to use the new mount api.
The only changes are what's needed to communicate the variables
dsunit, dswidth and iosizelog back to xfs_parseags().
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 290 ++++++++++++++++++++++++++++------------------------
1 file changed, 155 insertions(+), 135 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 6a16750b1314..b04aebab69ab 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -205,6 +205,156 @@ match_kstrtoint(const substring_t *s, unsigned int base, int *res)
return ret;
}
+STATIC int
+xfs_parse_param(
+ int token,
+ char *p,
+ substring_t *args,
+ struct xfs_mount *mp,
+ int *dsunit,
+ int *dswidth,
+ uint8_t *iosizelog)
+{
+ int iosize = 0;
+
+ switch (token) {
+ case Opt_logbufs:
+ if (match_int(args, &mp->m_logbufs))
+ return -EINVAL;
+ break;
+ case Opt_logbsize:
+ if (match_kstrtoint(args, 10, &mp->m_logbsize))
+ return -EINVAL;
+ break;
+ case Opt_logdev:
+ kfree(mp->m_logname);
+ mp->m_logname = match_strdup(args);
+ if (!mp->m_logname)
+ return -ENOMEM;
+ break;
+ case Opt_rtdev:
+ kfree(mp->m_rtname);
+ mp->m_rtname = match_strdup(args);
+ if (!mp->m_rtname)
+ return -ENOMEM;
+ break;
+ case Opt_allocsize:
+ if (match_kstrtoint(args, 10, &iosize))
+ return -EINVAL;
+ *iosizelog = ffs(iosize) - 1;
+ break;
+ case Opt_grpid:
+ case Opt_bsdgroups:
+ mp->m_flags |= XFS_MOUNT_GRPID;
+ break;
+ case Opt_nogrpid:
+ case Opt_sysvgroups:
+ mp->m_flags &= ~XFS_MOUNT_GRPID;
+ break;
+ case Opt_wsync:
+ mp->m_flags |= XFS_MOUNT_WSYNC;
+ break;
+ case Opt_norecovery:
+ mp->m_flags |= XFS_MOUNT_NORECOVERY;
+ break;
+ case Opt_noalign:
+ mp->m_flags |= XFS_MOUNT_NOALIGN;
+ break;
+ case Opt_swalloc:
+ mp->m_flags |= XFS_MOUNT_SWALLOC;
+ break;
+ case Opt_sunit:
+ if (match_int(args, dsunit))
+ return -EINVAL;
+ break;
+ case Opt_swidth:
+ if (match_int(args, dswidth))
+ return -EINVAL;
+ break;
+ case Opt_inode32:
+ mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+ break;
+ case Opt_inode64:
+ mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
+ break;
+ case Opt_nouuid:
+ mp->m_flags |= XFS_MOUNT_NOUUID;
+ break;
+ case Opt_ikeep:
+ mp->m_flags |= XFS_MOUNT_IKEEP;
+ break;
+ case Opt_noikeep:
+ mp->m_flags &= ~XFS_MOUNT_IKEEP;
+ break;
+ case Opt_largeio:
+ mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
+ break;
+ case Opt_nolargeio:
+ mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
+ break;
+ case Opt_attr2:
+ mp->m_flags |= XFS_MOUNT_ATTR2;
+ break;
+ case Opt_noattr2:
+ mp->m_flags &= ~XFS_MOUNT_ATTR2;
+ mp->m_flags |= XFS_MOUNT_NOATTR2;
+ break;
+ case Opt_filestreams:
+ mp->m_flags |= XFS_MOUNT_FILESTREAMS;
+ break;
+ case Opt_noquota:
+ mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
+ mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
+ mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
+ break;
+ case Opt_quota:
+ case Opt_uquota:
+ case Opt_usrquota:
+ mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
+ XFS_UQUOTA_ENFD);
+ break;
+ case Opt_qnoenforce:
+ case Opt_uqnoenforce:
+ mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
+ mp->m_qflags &= ~XFS_UQUOTA_ENFD;
+ break;
+ case Opt_pquota:
+ case Opt_prjquota:
+ mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
+ XFS_PQUOTA_ENFD);
+ break;
+ case Opt_pqnoenforce:
+ mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
+ mp->m_qflags &= ~XFS_PQUOTA_ENFD;
+ break;
+ case Opt_gquota:
+ case Opt_grpquota:
+ mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
+ XFS_GQUOTA_ENFD);
+ break;
+ case Opt_gqnoenforce:
+ mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
+ mp->m_qflags &= ~XFS_GQUOTA_ENFD;
+ break;
+ case Opt_discard:
+ mp->m_flags |= XFS_MOUNT_DISCARD;
+ break;
+ case Opt_nodiscard:
+ mp->m_flags &= ~XFS_MOUNT_DISCARD;
+ break;
+#ifdef CONFIG_FS_DAX
+ case Opt_dax:
+ mp->m_flags |= XFS_MOUNT_DAX;
+ break;
+#endif
+ default:
+ xfs_warn(mp, "unknown mount option [%s].", p);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/*
* This function fills in xfs_mount_t fields based on mount args.
* Note: the superblock has _not_ yet been read in.
@@ -226,7 +376,6 @@ xfs_parseargs(
substring_t args[MAX_OPT_ARGS];
int dsunit = 0;
int dswidth = 0;
- int iosize = 0;
uint8_t iosizelog = 0;
/*
@@ -265,145 +414,16 @@ xfs_parseargs(
while ((p = strsep(&options, ",")) != NULL) {
int token;
+ int ret;
if (!*p)
continue;
token = match_token(p, tokens, args);
- switch (token) {
- case Opt_logbufs:
- if (match_int(args, &mp->m_logbufs))
- return -EINVAL;
- break;
- case Opt_logbsize:
- if (match_kstrtoint(args, 10, &mp->m_logbsize))
- return -EINVAL;
- break;
- case Opt_logdev:
- kfree(mp->m_logname);
- mp->m_logname = match_strdup(args);
- if (!mp->m_logname)
- return -ENOMEM;
- break;
- case Opt_rtdev:
- kfree(mp->m_rtname);
- mp->m_rtname = match_strdup(args);
- if (!mp->m_rtname)
- return -ENOMEM;
- break;
- case Opt_allocsize:
- if (match_kstrtoint(args, 10, &iosize))
- return -EINVAL;
- iosizelog = ffs(iosize) - 1;
- break;
- case Opt_grpid:
- case Opt_bsdgroups:
- mp->m_flags |= XFS_MOUNT_GRPID;
- break;
- case Opt_nogrpid:
- case Opt_sysvgroups:
- mp->m_flags &= ~XFS_MOUNT_GRPID;
- break;
- case Opt_wsync:
- mp->m_flags |= XFS_MOUNT_WSYNC;
- break;
- case Opt_norecovery:
- mp->m_flags |= XFS_MOUNT_NORECOVERY;
- break;
- case Opt_noalign:
- mp->m_flags |= XFS_MOUNT_NOALIGN;
- break;
- case Opt_swalloc:
- mp->m_flags |= XFS_MOUNT_SWALLOC;
- break;
- case Opt_sunit:
- if (match_int(args, &dsunit))
- return -EINVAL;
- break;
- case Opt_swidth:
- if (match_int(args, &dswidth))
- return -EINVAL;
- break;
- case Opt_inode32:
- mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
- break;
- case Opt_inode64:
- mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
- break;
- case Opt_nouuid:
- mp->m_flags |= XFS_MOUNT_NOUUID;
- break;
- case Opt_ikeep:
- mp->m_flags |= XFS_MOUNT_IKEEP;
- break;
- case Opt_noikeep:
- mp->m_flags &= ~XFS_MOUNT_IKEEP;
- break;
- case Opt_largeio:
- mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
- break;
- case Opt_nolargeio:
- mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
- break;
- case Opt_attr2:
- mp->m_flags |= XFS_MOUNT_ATTR2;
- break;
- case Opt_noattr2:
- mp->m_flags &= ~XFS_MOUNT_ATTR2;
- mp->m_flags |= XFS_MOUNT_NOATTR2;
- break;
- case Opt_filestreams:
- mp->m_flags |= XFS_MOUNT_FILESTREAMS;
- break;
- case Opt_noquota:
- mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
- mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
- mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
- break;
- case Opt_quota:
- case Opt_uquota:
- case Opt_usrquota:
- mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
- XFS_UQUOTA_ENFD);
- break;
- case Opt_qnoenforce:
- case Opt_uqnoenforce:
- mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
- mp->m_qflags &= ~XFS_UQUOTA_ENFD;
- break;
- case Opt_pquota:
- case Opt_prjquota:
- mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
- XFS_PQUOTA_ENFD);
- break;
- case Opt_pqnoenforce:
- mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
- mp->m_qflags &= ~XFS_PQUOTA_ENFD;
- break;
- case Opt_gquota:
- case Opt_grpquota:
- mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
- XFS_GQUOTA_ENFD);
- break;
- case Opt_gqnoenforce:
- mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
- mp->m_qflags &= ~XFS_GQUOTA_ENFD;
- break;
- case Opt_discard:
- mp->m_flags |= XFS_MOUNT_DISCARD;
- break;
- case Opt_nodiscard:
- mp->m_flags &= ~XFS_MOUNT_DISCARD;
- break;
-#ifdef CONFIG_FS_DAX
- case Opt_dax:
- mp->m_flags |= XFS_MOUNT_DAX;
- break;
-#endif
- default:
- xfs_warn(mp, "unknown mount option [%s].", p);
- return -EINVAL;
- }
+ ret = xfs_parse_param(token, p, args, mp,
+ &dsunit, &dswidth, &iosizelog);
+ if (ret)
+ return ret;
}
/*
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (4 preceding siblings ...)
2019-09-20 9:55 ` [PATCH v3 05/16] xfs: mount-api - refactor xfs_parseags() Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-24 10:54 ` Brian Foster
2019-09-20 9:56 ` [PATCH v3 07/16] xfs: mount-api - move xfs_parseargs() validation to a helper Ian Kent
` (9 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Make xfs_parse_param() take arguments of the fs context operation
.parse_param() in preparation for switching to use the file system
mount context for mount.
The function fc_parse() only uses the file system context (fc here)
when calling log macros warnf() and invalf() which in turn check
only the fc->log field to determine if the message should be saved
to a context buffer (for later retrival by userspace) or logged
using printk().
Also the temporary function match_kstrtoint() is now unused, remove it.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 135 ++++++++++++++++++++++++++++++----------------------
1 file changed, 79 insertions(+), 56 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index b04aebab69ab..041ab8b52a7d 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -191,57 +191,60 @@ suffix_kstrtoint(const char *s, unsigned int base, int *res)
return ret;
}
-STATIC int
-match_kstrtoint(const substring_t *s, unsigned int base, int *res)
-{
- const char *value;
- int ret;
-
- value = match_strdup(s);
- if (!value)
- return -ENOMEM;
- ret = suffix_kstrtoint(value, base, res);
- kfree(value);
- return ret;
-}
+struct xfs_fs_context {
+ int dsunit;
+ int dswidth;
+ uint8_t iosizelog;
+};
STATIC int
xfs_parse_param(
- int token,
- char *p,
- substring_t *args,
- struct xfs_mount *mp,
- int *dsunit,
- int *dswidth,
- uint8_t *iosizelog)
+ struct fs_context *fc,
+ struct fs_parameter *param)
{
+ struct xfs_fs_context *ctx = fc->fs_private;
+ struct xfs_mount *mp = fc->s_fs_info;
+ struct fs_parse_result result;
int iosize = 0;
+ int opt;
- switch (token) {
+ opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
+ if (opt < 0) {
+ /*
+ * If fs_parse() returns -ENOPARAM and the parameter
+ * is "source" the VFS needs to handle this option
+ * in order to boot otherwise use the default case
+ * below to handle invalid options.
+ */
+ if (opt != -ENOPARAM ||
+ strcmp(param->key, "source") == 0)
+ return opt;
+ }
+
+ switch (opt) {
case Opt_logbufs:
- if (match_int(args, &mp->m_logbufs))
- return -EINVAL;
+ mp->m_logbufs = result.uint_32;
break;
case Opt_logbsize:
- if (match_kstrtoint(args, 10, &mp->m_logbsize))
+ if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
return -EINVAL;
break;
case Opt_logdev:
kfree(mp->m_logname);
- mp->m_logname = match_strdup(args);
+ mp->m_logname = kstrdup(param->string, GFP_KERNEL);
if (!mp->m_logname)
return -ENOMEM;
break;
case Opt_rtdev:
kfree(mp->m_rtname);
- mp->m_rtname = match_strdup(args);
+ mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
if (!mp->m_rtname)
return -ENOMEM;
break;
case Opt_allocsize:
- if (match_kstrtoint(args, 10, &iosize))
+ if (suffix_kstrtoint(param->string, 10, &iosize))
return -EINVAL;
- *iosizelog = ffs(iosize) - 1;
+ ctx->iosizelog = ffs(iosize) - 1;
break;
case Opt_grpid:
case Opt_bsdgroups:
@@ -264,12 +267,10 @@ xfs_parse_param(
mp->m_flags |= XFS_MOUNT_SWALLOC;
break;
case Opt_sunit:
- if (match_int(args, dsunit))
- return -EINVAL;
+ ctx->dsunit = result.uint_32;
break;
case Opt_swidth:
- if (match_int(args, dswidth))
- return -EINVAL;
+ ctx->dswidth = result.uint_32;
break;
case Opt_inode32:
mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
@@ -348,7 +349,7 @@ xfs_parse_param(
break;
#endif
default:
- xfs_warn(mp, "unknown mount option [%s].", p);
+ xfs_warn(mp, "unknown mount option [%s].", param->key);
return -EINVAL;
}
@@ -373,10 +374,13 @@ xfs_parseargs(
{
const struct super_block *sb = mp->m_super;
char *p;
- substring_t args[MAX_OPT_ARGS];
- int dsunit = 0;
- int dswidth = 0;
- uint8_t iosizelog = 0;
+ struct fs_context fc;
+ struct xfs_fs_context context;
+
+ memset(&fc, 0, sizeof(fc));
+ memset(&context, 0, sizeof(context));
+ fc.fs_private = &context;
+ fc.s_fs_info = mp;
/*
* set up the mount name first so all the errors will refer to the
@@ -413,16 +417,34 @@ xfs_parseargs(
goto done;
while ((p = strsep(&options, ",")) != NULL) {
- int token;
- int ret;
+ struct fs_parameter param;
+ char *value;
+ int ret;
if (!*p)
continue;
- token = match_token(p, tokens, args);
- ret = xfs_parse_param(token, p, args, mp,
- &dsunit, &dswidth, &iosizelog);
- if (ret)
+ param.key = p;
+ param.type = fs_value_is_string;
+ param.string = NULL;
+ param.size = 0;
+
+ value = strchr(p, '=');
+ if (value) {
+ *value++ = 0;
+ param.size = strlen(value);
+ if (param.size > 0) {
+ param.string = kmemdup_nul(value,
+ param.size,
+ GFP_KERNEL);
+ if (!param.string)
+ return -ENOMEM;
+ }
+ }
+
+ ret = xfs_parse_param(&fc, ¶m);
+ kfree(param.string);
+ if (ret < 0)
return ret;
}
@@ -435,7 +457,8 @@ xfs_parseargs(
return -EINVAL;
}
- if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (dsunit || dswidth)) {
+ if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
+ (ctx->dsunit || ctx->dswidth)) {
xfs_warn(mp,
"sunit and swidth options incompatible with the noalign option");
return -EINVAL;
@@ -448,28 +471,28 @@ xfs_parseargs(
}
#endif
- if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
+ if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
xfs_warn(mp, "sunit and swidth must be specified together");
return -EINVAL;
}
- if (dsunit && (dswidth % dsunit != 0)) {
+ if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
xfs_warn(mp,
"stripe width (%d) must be a multiple of the stripe unit (%d)",
- dswidth, dsunit);
+ ctx->dswidth, ctx->dsunit);
return -EINVAL;
}
done:
- if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
+ if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
/*
* At this point the superblock has not been read
* in, therefore we do not know the block size.
* Before the mount call ends we will convert
* these to FSBs.
*/
- mp->m_dalign = dsunit;
- mp->m_swidth = dswidth;
+ mp->m_dalign = ctx->dsunit;
+ mp->m_swidth = ctx->dswidth;
}
if (mp->m_logbufs != -1 &&
@@ -491,18 +514,18 @@ xfs_parseargs(
return -EINVAL;
}
- if (iosizelog) {
- if (iosizelog > XFS_MAX_IO_LOG ||
- iosizelog < XFS_MIN_IO_LOG) {
+ if (ctx->iosizelog) {
+ if (ctx->iosizelog > XFS_MAX_IO_LOG ||
+ ctx->iosizelog < XFS_MIN_IO_LOG) {
xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
- iosizelog, XFS_MIN_IO_LOG,
+ ctx->iosizelog, XFS_MIN_IO_LOG,
XFS_MAX_IO_LOG);
return -EINVAL;
}
mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
- mp->m_readio_log = iosizelog;
- mp->m_writeio_log = iosizelog;
+ mp->m_readio_log = ctx->iosizelog;
+ mp->m_writeio_log = ctx->iosizelog;
}
return 0;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 07/16] xfs: mount-api - move xfs_parseargs() validation to a helper
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (5 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-24 10:56 ` Brian Foster
2019-09-20 9:56 ` [PATCH v3 08/16] xfs: mount-api - refactor xfs_fs_fill_super() Ian Kent
` (8 subsequent siblings)
15 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Move the validation code of xfs_parseargs() into a helper for later
use within the mount context methods.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 148 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 94 insertions(+), 54 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 041ab8b52a7d..5cb9a9fd1a15 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -356,6 +356,97 @@ xfs_parse_param(
return 0;
}
+STATIC int
+xfs_validate_params(
+ struct xfs_mount *mp,
+ struct xfs_fs_context *ctx,
+ bool nooptions)
+{
+ if (nooptions)
+ goto noopts;
+
+ /*
+ * no recovery flag requires a read-only mount
+ */
+ if ((mp->m_flags & XFS_MOUNT_NORECOVERY) &&
+ !(mp->m_flags & XFS_MOUNT_RDONLY)) {
+ xfs_warn(mp, "no-recovery mounts must be read-only.");
+ return -EINVAL;
+ }
+
+ if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (ctx->dsunit || ctx->dswidth)) {
+ xfs_warn(mp,
+ "sunit and swidth options incompatible with the noalign option");
+ return -EINVAL;
+ }
+
+#ifndef CONFIG_XFS_QUOTA
+ if (XFS_IS_QUOTA_RUNNING(mp)) {
+ xfs_warn(mp, "quota support not available in this kernel.");
+ return -EINVAL;
+ }
+#endif
+
+ if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
+ xfs_warn(mp, "sunit and swidth must be specified together");
+ return -EINVAL;
+ }
+
+ if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
+ xfs_warn(mp,
+ "stripe width (%d) must be a multiple of the stripe unit (%d)",
+ ctx->dswidth, ctx->dsunit);
+ return -EINVAL;
+ }
+
+noopts:
+ if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
+ /*
+ * At this point the superblock has not been read
+ * in, therefore we do not know the block size.
+ * Before the mount call ends we will convert
+ * these to FSBs.
+ */
+ mp->m_dalign = ctx->dsunit;
+ mp->m_swidth = ctx->dswidth;
+ }
+
+ if (mp->m_logbufs != -1 &&
+ mp->m_logbufs != 0 &&
+ (mp->m_logbufs < XLOG_MIN_ICLOGS ||
+ mp->m_logbufs > XLOG_MAX_ICLOGS)) {
+ xfs_warn(mp, "invalid logbufs value: %d [not %d-%d]",
+ mp->m_logbufs, XLOG_MIN_ICLOGS, XLOG_MAX_ICLOGS);
+ return -EINVAL;
+ }
+ if (mp->m_logbsize != -1 &&
+ mp->m_logbsize != 0 &&
+ (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
+ mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
+ !is_power_of_2(mp->m_logbsize))) {
+ xfs_warn(mp,
+ "invalid logbufsize: %d [not 16k,32k,64k,128k or 256k]",
+ mp->m_logbsize);
+ return -EINVAL;
+ }
+
+ if (ctx->iosizelog) {
+ if (ctx->iosizelog > XFS_MAX_IO_LOG ||
+ ctx->iosizelog < XFS_MIN_IO_LOG) {
+ xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
+ ctx->iosizelog, XFS_MIN_IO_LOG,
+ XFS_MAX_IO_LOG);
+ return -EINVAL;
+ }
+
+ mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
+ mp->m_readio_log = ctx->iosizelog;
+ mp->m_writeio_log = ctx->iosizelog;
+ }
+
+ return 0;
+}
+
/*
* This function fills in xfs_mount_t fields based on mount args.
* Note: the superblock has _not_ yet been read in.
@@ -445,16 +536,7 @@ xfs_parseargs(
ret = xfs_parse_param(&fc, ¶m);
kfree(param.string);
if (ret < 0)
- return ret;
- }
-
- /*
- * no recovery flag requires a read-only mount
- */
- if ((mp->m_flags & XFS_MOUNT_NORECOVERY) &&
- !(mp->m_flags & XFS_MOUNT_RDONLY)) {
- xfs_warn(mp, "no-recovery mounts must be read-only.");
- return -EINVAL;
+ goto done;
}
if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
@@ -484,51 +566,9 @@ xfs_parseargs(
}
done:
- if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
- /*
- * At this point the superblock has not been read
- * in, therefore we do not know the block size.
- * Before the mount call ends we will convert
- * these to FSBs.
- */
- mp->m_dalign = ctx->dsunit;
- mp->m_swidth = ctx->dswidth;
- }
-
- if (mp->m_logbufs != -1 &&
- mp->m_logbufs != 0 &&
- (mp->m_logbufs < XLOG_MIN_ICLOGS ||
- mp->m_logbufs > XLOG_MAX_ICLOGS)) {
- xfs_warn(mp, "invalid logbufs value: %d [not %d-%d]",
- mp->m_logbufs, XLOG_MIN_ICLOGS, XLOG_MAX_ICLOGS);
- return -EINVAL;
- }
- if (mp->m_logbsize != -1 &&
- mp->m_logbsize != 0 &&
- (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
- mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
- !is_power_of_2(mp->m_logbsize))) {
- xfs_warn(mp,
- "invalid logbufsize: %d [not 16k,32k,64k,128k or 256k]",
- mp->m_logbsize);
- return -EINVAL;
- }
+ ret = xfs_validate_params(mp, &context, false);
- if (ctx->iosizelog) {
- if (ctx->iosizelog > XFS_MAX_IO_LOG ||
- ctx->iosizelog < XFS_MIN_IO_LOG) {
- xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
- ctx->iosizelog, XFS_MIN_IO_LOG,
- XFS_MAX_IO_LOG);
- return -EINVAL;
- }
-
- mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
- mp->m_readio_log = ctx->iosizelog;
- mp->m_writeio_log = ctx->iosizelog;
- }
-
- return 0;
+ return ret;
}
struct proc_xfs_info {
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 08/16] xfs: mount-api - refactor xfs_fs_fill_super()
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (6 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 07/16] xfs: mount-api - move xfs_parseargs() validation to a helper Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 09/16] xfs: mount-api - add xfs_get_tree() Ian Kent
` (7 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Much of the code in xfs_fs_fill_super() will be used by the fill super
function of the new mount-api.
So refactor the common code into a helper in an attempt to show what's
actually changed.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 64 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 5cb9a9fd1a15..9cf5c85d1341 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1718,27 +1718,14 @@ xfs_mount_alloc(
STATIC int
-xfs_fs_fill_super(
- struct super_block *sb,
- void *data,
+__xfs_fs_fill_super(
+ struct xfs_mount *mp,
int silent)
{
+ struct super_block *sb = mp->m_super;
struct inode *root;
- struct xfs_mount *mp = NULL;
- int flags = 0, error = -ENOMEM;
-
- /*
- * allocate mp and do all low-level struct initializations before we
- * attach it to the super
- */
- mp = xfs_mount_alloc(sb);
- if (!mp)
- goto out;
- sb->s_fs_info = mp;
-
- error = xfs_parseargs(mp, (char *)data);
- if (error)
- goto out_free_fsname;
+ int flags = 0;
+ int error;
sb_min_blocksize(sb, BBSIZE);
sb->s_xattr = xfs_xattr_handlers;
@@ -1765,7 +1752,7 @@ xfs_fs_fill_super(
error = xfs_open_devices(mp);
if (error)
- goto out_free_fsname;
+ goto out;
error = xfs_init_mount_workqueues(mp);
if (error)
@@ -1900,10 +1887,6 @@ xfs_fs_fill_super(
xfs_destroy_mount_workqueues(mp);
out_close_devices:
xfs_close_devices(mp);
- out_free_fsname:
- sb->s_fs_info = NULL;
- xfs_free_fsname(mp);
- kfree(mp);
out:
return error;
@@ -1913,6 +1896,41 @@ xfs_fs_fill_super(
goto out_free_sb;
}
+STATIC int
+xfs_fs_fill_super(
+ struct super_block *sb,
+ void *data,
+ int silent)
+{
+ struct xfs_mount *mp = NULL;
+ int error = -ENOMEM;
+
+ /*
+ * allocate mp and do all low-level struct initializations before we
+ * attach it to the super
+ */
+ mp = xfs_mount_alloc(sb);
+ if (!mp)
+ return -ENOMEM;
+ sb->s_fs_info = mp;
+
+ error = xfs_parseargs(mp, (char *)data);
+ if (error)
+ goto out_free_fsname;
+
+ error = __xfs_fs_fill_super(mp, silent);
+ if (error)
+ goto out_free_fsname;
+
+ return 0;
+
+ out_free_fsname:
+ sb->s_fs_info = NULL;
+ xfs_free_fsname(mp);
+ kfree(mp);
+ return error;
+}
+
STATIC void
xfs_fs_put_super(
struct super_block *sb)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 09/16] xfs: mount-api - add xfs_get_tree()
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (7 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 08/16] xfs: mount-api - refactor xfs_fs_fill_super() Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 10/16] xfs: mount-api - add xfs_remount_rw() helper Ian Kent
` (6 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Add the fs_context_operations method .get_tree that validates
mount options and fills the super block as previously done
by the file_system_type .mount method.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 9cf5c85d1341..3a4c724de1cc 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1931,6 +1931,51 @@ xfs_fs_fill_super(
return error;
}
+STATIC int
+xfs_fill_super(
+ struct super_block *sb,
+ struct fs_context *fc)
+{
+ struct xfs_fs_context *ctx = fc->fs_private;
+ struct xfs_mount *mp = sb->s_fs_info;
+ int silent = fc->sb_flags & SB_SILENT;
+ int error = -ENOMEM;
+
+ mp->m_super = sb;
+
+ /*
+ * set up the mount name first so all the errors will refer to the
+ * correct device.
+ */
+ mp->m_fsname = kstrndup(sb->s_id, MAXNAMELEN, GFP_KERNEL);
+ if (!mp->m_fsname)
+ return -ENOMEM;
+ mp->m_fsname_len = strlen(mp->m_fsname) + 1;
+
+ error = xfs_validate_params(mp, ctx, false);
+ if (error)
+ goto out_free_fsname;
+
+ error = __xfs_fs_fill_super(mp, silent);
+ if (error)
+ goto out_free_fsname;
+
+ return 0;
+
+ out_free_fsname:
+ sb->s_fs_info = NULL;
+ xfs_free_fsname(mp);
+
+ return error;
+}
+
+STATIC int
+xfs_get_tree(
+ struct fs_context *fc)
+{
+ return vfs_get_block_super(fc, xfs_fill_super);
+}
+
STATIC void
xfs_fs_put_super(
struct super_block *sb)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 10/16] xfs: mount-api - add xfs_remount_rw() helper
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (8 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 09/16] xfs: mount-api - add xfs_get_tree() Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 11/16] xfs: mount-api - add xfs_remount_ro() helper Ian Kent
` (5 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Factor the remount read write code into a helper to simplify the
subsequent change from the super block method .remount_fs to the
mount-api fs_context_operations method .reconfigure.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 115 +++++++++++++++++++++++++++++-----------------------
1 file changed, 64 insertions(+), 51 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 3a4c724de1cc..b037d933e622 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1369,6 +1369,68 @@ xfs_test_remount_options(
return error;
}
+STATIC int
+xfs_remount_rw(
+ struct xfs_mount *mp)
+{
+ xfs_sb_t *sbp = &mp->m_sb;
+ int error;
+
+ if (mp->m_flags & XFS_MOUNT_NORECOVERY) {
+ xfs_warn(mp,
+ "ro->rw transition prohibited on norecovery mount");
+ return -EINVAL;
+ }
+
+ if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+ xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
+ xfs_warn(mp,
+ "ro->rw transition prohibited on unknown (0x%x) ro-compat filesystem",
+ (sbp->sb_features_ro_compat &
+ XFS_SB_FEAT_RO_COMPAT_UNKNOWN));
+ return -EINVAL;
+ }
+
+ mp->m_flags &= ~XFS_MOUNT_RDONLY;
+
+ /*
+ * If this is the first remount to writeable state we
+ * might have some superblock changes to update.
+ */
+ if (mp->m_update_sb) {
+ error = xfs_sync_sb(mp, false);
+ if (error) {
+ xfs_warn(mp, "failed to write sb changes");
+ return error;
+ }
+ mp->m_update_sb = false;
+ }
+
+ /*
+ * Fill out the reserve pool if it is empty. Use the stashed
+ * value if it is non-zero, otherwise go with the default.
+ */
+ xfs_restore_resvblks(mp);
+ xfs_log_work_queue(mp);
+
+ /* Recover any CoW blocks that never got remapped. */
+ error = xfs_reflink_recover_cow(mp);
+ if (error) {
+ xfs_err(mp,
+ "Error %d recovering leftover CoW allocations.", error);
+ xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+ return error;
+ }
+ xfs_start_block_reaping(mp);
+
+ /* Create the per-AG metadata reservation pool .*/
+ error = xfs_fs_reserve_ag_blocks(mp);
+ if (error && error != -ENOSPC)
+ return error;
+
+ return 0;
+}
+
STATIC int
xfs_fs_remount(
struct super_block *sb,
@@ -1432,57 +1494,8 @@ xfs_fs_remount(
/* ro -> rw */
if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(*flags & SB_RDONLY)) {
- if (mp->m_flags & XFS_MOUNT_NORECOVERY) {
- xfs_warn(mp,
- "ro->rw transition prohibited on norecovery mount");
- return -EINVAL;
- }
-
- if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
- xfs_sb_has_ro_compat_feature(sbp,
- XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
- xfs_warn(mp,
-"ro->rw transition prohibited on unknown (0x%x) ro-compat filesystem",
- (sbp->sb_features_ro_compat &
- XFS_SB_FEAT_RO_COMPAT_UNKNOWN));
- return -EINVAL;
- }
-
- mp->m_flags &= ~XFS_MOUNT_RDONLY;
-
- /*
- * If this is the first remount to writeable state we
- * might have some superblock changes to update.
- */
- if (mp->m_update_sb) {
- error = xfs_sync_sb(mp, false);
- if (error) {
- xfs_warn(mp, "failed to write sb changes");
- return error;
- }
- mp->m_update_sb = false;
- }
-
- /*
- * Fill out the reserve pool if it is empty. Use the stashed
- * value if it is non-zero, otherwise go with the default.
- */
- xfs_restore_resvblks(mp);
- xfs_log_work_queue(mp);
-
- /* Recover any CoW blocks that never got remapped. */
- error = xfs_reflink_recover_cow(mp);
- if (error) {
- xfs_err(mp,
- "Error %d recovering leftover CoW allocations.", error);
- xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
- return error;
- }
- xfs_start_block_reaping(mp);
-
- /* Create the per-AG metadata reservation pool .*/
- error = xfs_fs_reserve_ag_blocks(mp);
- if (error && error != -ENOSPC)
+ error = xfs_remount_rw(mp);
+ if (error)
return error;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 11/16] xfs: mount-api - add xfs_remount_ro() helper
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (9 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 10/16] xfs: mount-api - add xfs_remount_rw() helper Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 12/16] xfs: mount api - add xfs_reconfigure() Ian Kent
` (4 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Factor the remount read only code into a helper to simplify the
subsequent change from the super block method .remount_fs to the
mount-api fs_context_operations method .reconfigure.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 73 +++++++++++++++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 30 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index b037d933e622..ef28a2891091 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1431,6 +1431,47 @@ xfs_remount_rw(
return 0;
}
+STATIC int
+xfs_remount_ro(
+ struct xfs_mount *mp)
+{
+ int error;
+
+ /*
+ * Cancel background eofb scanning so it cannot race with the
+ * final log force+buftarg wait and deadlock the remount.
+ */
+ xfs_stop_block_reaping(mp);
+
+ /* Get rid of any leftover CoW reservations... */
+ error = xfs_icache_free_cowblocks(mp, NULL);
+ if (error) {
+ xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+ return error;
+ }
+
+ /* Free the per-AG metadata reservation pool. */
+ error = xfs_fs_unreserve_ag_blocks(mp);
+ if (error) {
+ xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+ return error;
+ }
+
+ /*
+ * Before we sync the metadata, we need to free up the reserve
+ * block pool so that the used block count in the superblock on
+ * disk is correct at the end of the remount. Stash the current
+ * reserve pool size so that if we get remounted rw, we can
+ * return it to the same size.
+ */
+ xfs_save_resvblks(mp);
+
+ xfs_quiesce_attr(mp);
+ mp->m_flags |= XFS_MOUNT_RDONLY;
+
+ return 0;
+}
+
STATIC int
xfs_fs_remount(
struct super_block *sb,
@@ -1501,37 +1542,9 @@ xfs_fs_remount(
/* rw -> ro */
if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (*flags & SB_RDONLY)) {
- /*
- * Cancel background eofb scanning so it cannot race with the
- * final log force+buftarg wait and deadlock the remount.
- */
- xfs_stop_block_reaping(mp);
-
- /* Get rid of any leftover CoW reservations... */
- error = xfs_icache_free_cowblocks(mp, NULL);
- if (error) {
- xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
- return error;
- }
-
- /* Free the per-AG metadata reservation pool. */
- error = xfs_fs_unreserve_ag_blocks(mp);
- if (error) {
- xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
+ error = xfs_remount_ro(mp);
+ if (error)
return error;
- }
-
- /*
- * Before we sync the metadata, we need to free up the reserve
- * block pool so that the used block count in the superblock on
- * disk is correct at the end of the remount. Stash the current
- * reserve pool size so that if we get remounted rw, we can
- * return it to the same size.
- */
- xfs_save_resvblks(mp);
-
- xfs_quiesce_attr(mp);
- mp->m_flags |= XFS_MOUNT_RDONLY;
}
return 0;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 12/16] xfs: mount api - add xfs_reconfigure()
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (10 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 11/16] xfs: mount-api - add xfs_remount_ro() helper Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 13/16] xfs: mount-api - add xfs_fc_free() Ian Kent
` (3 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Add the fs_context_operations method .reconfigure that performs
remount validation as previously done by the super_operations
.remount_fs method.
An attempt has also been made to update the comment about options
handling problems with mount(8) to reflect the current situation.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index ef28a2891091..06b382290354 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1550,6 +1550,89 @@ xfs_fs_remount(
return 0;
}
+/*
+ * There have been problems in the past with options passed from mount(8).
+ *
+ * The problem being that options passed by mount(8) in the case where only
+ * the the mount point path is given would consist of the existing fstab
+ * options with the options from mtab for the current mount merged in and
+ * the options given on the command line last. But the result couldn't be
+ * relied upon to accurately reflect the current mount options so that
+ * rejecting options that can't be changed on reconfigure could erronously
+ * cause mount failure.
+ *
+ * The mount-api uses a legacy mount options handler in the VFS to handle
+ * mount(8) so these options will continue to be passed. Even if mount(8)
+ * is updated to use fsopen()/fsconfig()/fsmount() it's likely to continue
+ * to set the existing options so options problems with reconfigure could
+ * continue.
+ *
+ * For the longest time mtab locking was a problem and this could have been
+ * one possible cause. It's also possible there could have been options
+ * order problems.
+ *
+ * That has changed now as mtab is a link to the proc file system mount
+ * table so mtab options should be always accurate.
+ *
+ * Consulting the util-linux maintainer (Karel Zak) he is confident that,
+ * in this case, the options passed by mount(8) will be those of the current
+ * mount and the options order should be a correct merge of fstab and mtab
+ * options, and new options given on the command line.
+ *
+ * So, in theory, it should be possible to compare incoming options and
+ * return an error for options that differ from the current mount and can't
+ * be changed on reconfigure to prevent users from believing they might have
+ * changed mount options using remount which can't be changed.
+ *
+ * But for now continue to return success for every reconfigure request, and
+ * silently ignore all options that can't actually be changed.
+ */
+STATIC int
+xfs_reconfigure(
+ struct fs_context *fc)
+{
+ struct xfs_fs_context *ctx = fc->fs_private;
+ struct xfs_mount *mp = XFS_M(fc->root->d_sb);
+ struct xfs_mount *new_mp = fc->s_fs_info;
+ xfs_sb_t *sbp = &mp->m_sb;
+ int flags = fc->sb_flags;
+ int error;
+
+ error = xfs_validate_params(new_mp, ctx, false);
+ if (error)
+ return error;
+
+ /* inode32 -> inode64 */
+ if ((mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
+ !(new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
+ mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
+ mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
+ }
+
+ /* inode64 -> inode32 */
+ if (!(mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
+ (new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
+ mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+ mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
+ }
+
+ /* ro -> rw */
+ if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(flags & SB_RDONLY)) {
+ error = xfs_remount_rw(mp);
+ if (error)
+ return error;
+ }
+
+ /* rw -> ro */
+ if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
+ error = xfs_remount_ro(mp);
+ if (error)
+ return error;
+ }
+
+ return 0;
+}
+
/*
* Second stage of a freeze. The data is already frozen so we only
* need to take care of the metadata. Once that's done sync the superblock
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 13/16] xfs: mount-api - add xfs_fc_free()
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (11 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 12/16] xfs: mount api - add xfs_reconfigure() Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 14/16] xfs: mount-api - dont set sb in xfs_mount_alloc() Ian Kent
` (2 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Add the fs_context_operations method .free that performs fs
context cleanup on context release.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 06b382290354..edb15bb98dde 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -2155,6 +2155,32 @@ static const struct super_operations xfs_super_operations = {
.free_cached_objects = xfs_fs_free_cached_objects,
};
+static void xfs_fc_free(struct fs_context *fc)
+{
+ struct xfs_fs_context *ctx = fc->fs_private;
+ struct xfs_mount *mp = fc->s_fs_info;
+
+ /*
+ * When the mount context is initialized the private
+ * struct xfs_mount info (mp) is allocated and stored in
+ * the fs context along with the struct xfs_fs_context
+ * (ctx) mount context working working storage.
+ *
+ * On success the mount info struct, mp, is moved into
+ * private info super block field ->s_fs_info of the
+ * newly allocated super block. But if an error occurs
+ * before this happens it's the responsibility of the fs
+ * context to release the mount info struct in addition
+ * to the mount context working storage.
+ */
+ if (mp) {
+ kfree(mp->m_logname);
+ kfree(mp->m_rtname);
+ kfree(mp);
+ }
+ kfree(ctx);
+}
+
static struct file_system_type xfs_fs_type = {
.owner = THIS_MODULE,
.name = "xfs",
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 14/16] xfs: mount-api - dont set sb in xfs_mount_alloc()
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (12 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 13/16] xfs: mount-api - add xfs_fc_free() Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 15/16] xfs: mount-api - switch to new mount-api Ian Kent
2019-09-20 9:56 ` [PATCH v3 16/16] xfs: mount-api - remove legacy mount functions Ian Kent
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
When changing to use the new mount api the super block won't be
available when the xfs_mount info struct is allocated so move
setting the super block in xfs_mount to xfs_fs_fill_super().
Also change xfs_mount_alloc() decalaration static -> STATIC.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index edb15bb98dde..eb5dc70658ab 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1794,9 +1794,8 @@ xfs_destroy_percpu_counters(
percpu_counter_destroy(&mp->m_delalloc_blks);
}
-static struct xfs_mount *
-xfs_mount_alloc(
- struct super_block *sb)
+STATIC struct xfs_mount *
+xfs_mount_alloc(void)
{
struct xfs_mount *mp;
@@ -1804,7 +1803,6 @@ xfs_mount_alloc(
if (!mp)
return NULL;
- mp->m_super = sb;
spin_lock_init(&mp->m_sb_lock);
spin_lock_init(&mp->m_agirotor_lock);
INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC);
@@ -2018,9 +2016,10 @@ xfs_fs_fill_super(
* allocate mp and do all low-level struct initializations before we
* attach it to the super
*/
- mp = xfs_mount_alloc(sb);
+ mp = xfs_mount_alloc();
if (!mp)
return -ENOMEM;
+ mp->m_super = sb;
sb->s_fs_info = mp;
error = xfs_parseargs(mp, (char *)data);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 15/16] xfs: mount-api - switch to new mount-api
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (13 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 14/16] xfs: mount-api - dont set sb in xfs_mount_alloc() Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 16/16] xfs: mount-api - remove legacy mount functions Ian Kent
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
The infrastructure needed to use the new mount api is now
in place, switch over to use it.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 156 +++++++++++++++++++---------------------------------
1 file changed, 56 insertions(+), 100 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index eb5dc70658ab..e0c8e9d8a2f6 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1349,26 +1349,6 @@ xfs_quiesce_attr(
xfs_log_quiesce(mp);
}
-STATIC int
-xfs_test_remount_options(
- struct super_block *sb,
- char *options)
-{
- int error = 0;
- struct xfs_mount *tmp_mp;
-
- tmp_mp = kmem_zalloc(sizeof(*tmp_mp), KM_MAYFAIL);
- if (!tmp_mp)
- return -ENOMEM;
-
- tmp_mp->m_super = sb;
- error = xfs_parseargs(tmp_mp, options);
- xfs_free_fsname(tmp_mp);
- kmem_free(tmp_mp);
-
- return error;
-}
-
STATIC int
xfs_remount_rw(
struct xfs_mount *mp)
@@ -1472,84 +1452,6 @@ xfs_remount_ro(
return 0;
}
-STATIC int
-xfs_fs_remount(
- struct super_block *sb,
- int *flags,
- char *options)
-{
- struct xfs_mount *mp = XFS_M(sb);
- xfs_sb_t *sbp = &mp->m_sb;
- substring_t args[MAX_OPT_ARGS];
- char *p;
- int error;
-
- /* First, check for complete junk; i.e. invalid options */
- error = xfs_test_remount_options(sb, options);
- if (error)
- return error;
-
- sync_filesystem(sb);
- while ((p = strsep(&options, ",")) != NULL) {
- int token;
-
- if (!*p)
- continue;
-
- token = match_token(p, tokens, args);
- switch (token) {
- case Opt_inode64:
- mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
- mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
- break;
- case Opt_inode32:
- mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
- mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
- break;
- default:
- /*
- * Logically we would return an error here to prevent
- * users from believing they might have changed
- * mount options using remount which can't be changed.
- *
- * But unfortunately mount(8) adds all options from
- * mtab and fstab to the mount arguments in some cases
- * so we can't blindly reject options, but have to
- * check for each specified option if it actually
- * differs from the currently set option and only
- * reject it if that's the case.
- *
- * Until that is implemented we return success for
- * every remount request, and silently ignore all
- * options that we can't actually change.
- */
-#if 0
- xfs_info(mp,
- "mount option \"%s\" not supported for remount", p);
- return -EINVAL;
-#else
- break;
-#endif
- }
- }
-
- /* ro -> rw */
- if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(*flags & SB_RDONLY)) {
- error = xfs_remount_rw(mp);
- if (error)
- return error;
- }
-
- /* rw -> ro */
- if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (*flags & SB_RDONLY)) {
- error = xfs_remount_ro(mp);
- if (error)
- return error;
- }
-
- return 0;
-}
-
/*
* There have been problems in the past with options passed from mount(8).
*
@@ -2148,7 +2050,6 @@ static const struct super_operations xfs_super_operations = {
.freeze_fs = xfs_fs_freeze,
.unfreeze_fs = xfs_fs_unfreeze,
.statfs = xfs_fs_statfs,
- .remount_fs = xfs_fs_remount,
.show_options = xfs_fs_show_options,
.nr_cached_objects = xfs_fs_nr_cached_objects,
.free_cached_objects = xfs_fs_free_cached_objects,
@@ -2180,10 +2081,65 @@ static void xfs_fc_free(struct fs_context *fc)
kfree(ctx);
}
+static const struct fs_context_operations xfs_context_ops = {
+ .parse_param = xfs_parse_param,
+ .get_tree = xfs_get_tree,
+ .reconfigure = xfs_reconfigure,
+ .free = xfs_fc_free,
+};
+
+/*
+ * Set up the filesystem mount context.
+ */
+int xfs_init_fs_context(struct fs_context *fc)
+{
+ struct xfs_fs_context *ctx;
+ struct xfs_mount *mp;
+
+ ctx = kzalloc(sizeof(struct xfs_fs_context), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ mp = xfs_mount_alloc();
+ if (!mp) {
+ kfree(ctx);
+ return -ENOMEM;
+ }
+
+ /*
+ * Set some default flags that could be cleared by the mount option
+ * parsing.
+ */
+ mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
+
+ /*
+ * These can be overridden by the mount option parsing.
+ */
+ mp->m_logbufs = -1;
+ mp->m_logbsize = -1;
+
+ /*
+ * Copy binary VFS mount flags we are interested in.
+ */
+ if (fc->sb_flags & SB_RDONLY)
+ mp->m_flags |= XFS_MOUNT_RDONLY;
+ if (fc->sb_flags & SB_DIRSYNC)
+ mp->m_flags |= XFS_MOUNT_DIRSYNC;
+ if (fc->sb_flags & SB_SYNCHRONOUS)
+ mp->m_flags |= XFS_MOUNT_WSYNC;
+
+ fc->fs_private = ctx;
+ fc->s_fs_info = mp;
+ fc->ops = &xfs_context_ops;
+
+ return 0;
+}
+
static struct file_system_type xfs_fs_type = {
.owner = THIS_MODULE,
.name = "xfs",
- .mount = xfs_fs_mount,
+ .init_fs_context = xfs_init_fs_context,
+ .parameters = &xfs_fs_parameters,
.kill_sb = kill_block_super,
.fs_flags = FS_REQUIRES_DEV,
};
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 16/16] xfs: mount-api - remove legacy mount functions
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
` (14 preceding siblings ...)
2019-09-20 9:56 ` [PATCH v3 15/16] xfs: mount-api - switch to new mount-api Ian Kent
@ 2019-09-20 9:56 ` Ian Kent
15 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-20 9:56 UTC (permalink / raw)
To: linux-xfs; +Cc: David Howells, Dave Chinner, Al Viro, Eric Sandeen
Now that the new mount api is being used the old mount functions
and parsing table can be removed.
Signed-off-by: Ian Kent <raven@themaw.net>
---
fs/xfs/xfs_super.c | 218 ----------------------------------------------------
1 file changed, 1 insertion(+), 217 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index e0c8e9d8a2f6..9124e0c9f1eb 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -61,53 +61,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_err,
-};
-
-static const match_table_t tokens = {
- {Opt_logbufs, "logbufs=%u"}, /* number of XFS log buffers */
- {Opt_logbsize, "logbsize=%s"}, /* size of XFS log buffers */
- {Opt_logdev, "logdev=%s"}, /* log device */
- {Opt_rtdev, "rtdev=%s"}, /* realtime I/O device */
- {Opt_wsync, "wsync"}, /* safe-mode nfs compatible mount */
- {Opt_noalign, "noalign"}, /* turn off stripe alignment */
- {Opt_swalloc, "swalloc"}, /* turn on stripe width allocation */
- {Opt_sunit, "sunit=%u"}, /* data volume stripe unit */
- {Opt_swidth, "swidth=%u"}, /* data volume stripe width */
- {Opt_nouuid, "nouuid"}, /* ignore filesystem UUID */
- {Opt_grpid, "grpid"}, /* group-ID from parent directory */
- {Opt_nogrpid, "nogrpid"}, /* group-ID from current process */
- {Opt_bsdgroups, "bsdgroups"}, /* group-ID from parent directory */
- {Opt_sysvgroups,"sysvgroups"}, /* group-ID from current process */
- {Opt_allocsize, "allocsize=%s"},/* preferred allocation size */
- {Opt_norecovery,"norecovery"}, /* don't run XFS recovery */
- {Opt_inode64, "inode64"}, /* inodes can be allocated anywhere */
- {Opt_inode32, "inode32"}, /* inode allocation limited to
- * XFS_MAXINUMBER_32 */
- {Opt_ikeep, "ikeep"}, /* do not free empty inode clusters */
- {Opt_noikeep, "noikeep"}, /* free empty inode clusters */
- {Opt_largeio, "largeio"}, /* report large I/O sizes in stat() */
- {Opt_nolargeio, "nolargeio"}, /* do not report large I/O sizes
- * in stat(). */
- {Opt_attr2, "attr2"}, /* do use attr2 attribute format */
- {Opt_noattr2, "noattr2"}, /* do not use attr2 attribute format */
- {Opt_filestreams,"filestreams"},/* use filestreams allocator */
- {Opt_quota, "quota"}, /* disk quotas (user) */
- {Opt_noquota, "noquota"}, /* no quotas */
- {Opt_usrquota, "usrquota"}, /* user quota enabled */
- {Opt_grpquota, "grpquota"}, /* group quota enabled */
- {Opt_prjquota, "prjquota"}, /* project quota enabled */
- {Opt_uquota, "uquota"}, /* user quota (IRIX variant) */
- {Opt_gquota, "gquota"}, /* group quota (IRIX variant) */
- {Opt_pquota, "pquota"}, /* project quota (IRIX variant) */
- {Opt_uqnoenforce,"uqnoenforce"},/* user quota limit enforcement */
- {Opt_gqnoenforce,"gqnoenforce"},/* group quota limit enforcement */
- {Opt_pqnoenforce,"pqnoenforce"},/* project quota limit enforcement */
- {Opt_qnoenforce, "qnoenforce"}, /* same as uqnoenforce */
- {Opt_discard, "discard"}, /* Discard unused blocks */
- {Opt_nodiscard, "nodiscard"}, /* Do not discard unused blocks */
- {Opt_dax, "dax"}, /* Enable direct access to bdev pages */
- {Opt_err, NULL},
+ Opt_discard, Opt_nodiscard, Opt_dax,
};
static const struct fs_parameter_spec xfs_param_specs[] = {
@@ -447,130 +401,6 @@ xfs_validate_params(
return 0;
}
-/*
- * This function fills in xfs_mount_t fields based on mount args.
- * Note: the superblock has _not_ yet been read in.
- *
- * Note that this function leaks the various device name allocations on
- * failure. The caller takes care of them.
- *
- * *sb is const because this is also used to test options on the remount
- * path, and we don't want this to have any side effects at remount time.
- * Today this function does not change *sb, but just to future-proof...
- */
-STATIC int
-xfs_parseargs(
- struct xfs_mount *mp,
- char *options)
-{
- const struct super_block *sb = mp->m_super;
- char *p;
- struct fs_context fc;
- struct xfs_fs_context context;
-
- memset(&fc, 0, sizeof(fc));
- memset(&context, 0, sizeof(context));
- fc.fs_private = &context;
- fc.s_fs_info = mp;
-
- /*
- * set up the mount name first so all the errors will refer to the
- * correct device.
- */
- mp->m_fsname = kstrndup(sb->s_id, MAXNAMELEN, GFP_KERNEL);
- if (!mp->m_fsname)
- return -ENOMEM;
- mp->m_fsname_len = strlen(mp->m_fsname) + 1;
-
- /*
- * Copy binary VFS mount flags we are interested in.
- */
- if (sb_rdonly(sb))
- mp->m_flags |= XFS_MOUNT_RDONLY;
- if (sb->s_flags & SB_DIRSYNC)
- mp->m_flags |= XFS_MOUNT_DIRSYNC;
- if (sb->s_flags & SB_SYNCHRONOUS)
- mp->m_flags |= XFS_MOUNT_WSYNC;
-
- /*
- * Set some default flags that could be cleared by the mount option
- * parsing.
- */
- mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
-
- /*
- * These can be overridden by the mount option parsing.
- */
- mp->m_logbufs = -1;
- mp->m_logbsize = -1;
-
- if (!options)
- goto done;
-
- while ((p = strsep(&options, ",")) != NULL) {
- struct fs_parameter param;
- char *value;
- int ret;
-
- if (!*p)
- continue;
-
- param.key = p;
- param.type = fs_value_is_string;
- param.string = NULL;
- param.size = 0;
-
- value = strchr(p, '=');
- if (value) {
- *value++ = 0;
- param.size = strlen(value);
- if (param.size > 0) {
- param.string = kmemdup_nul(value,
- param.size,
- GFP_KERNEL);
- if (!param.string)
- return -ENOMEM;
- }
- }
-
- ret = xfs_parse_param(&fc, ¶m);
- kfree(param.string);
- if (ret < 0)
- goto done;
- }
-
- if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
- (ctx->dsunit || ctx->dswidth)) {
- xfs_warn(mp,
- "sunit and swidth options incompatible with the noalign option");
- return -EINVAL;
- }
-
-#ifndef CONFIG_XFS_QUOTA
- if (XFS_IS_QUOTA_RUNNING(mp)) {
- xfs_warn(mp, "quota support not available in this kernel.");
- return -EINVAL;
- }
-#endif
-
- if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
- xfs_warn(mp, "sunit and swidth must be specified together");
- return -EINVAL;
- }
-
- if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
- xfs_warn(mp,
- "stripe width (%d) must be a multiple of the stripe unit (%d)",
- ctx->dswidth, ctx->dsunit);
- return -EINVAL;
- }
-
-done:
- ret = xfs_validate_params(mp, &context, false);
-
- return ret;
-}
-
struct proc_xfs_info {
uint64_t flag;
char *str;
@@ -1905,42 +1735,6 @@ __xfs_fs_fill_super(
goto out_free_sb;
}
-STATIC int
-xfs_fs_fill_super(
- struct super_block *sb,
- void *data,
- int silent)
-{
- struct xfs_mount *mp = NULL;
- int error = -ENOMEM;
-
- /*
- * allocate mp and do all low-level struct initializations before we
- * attach it to the super
- */
- mp = xfs_mount_alloc();
- if (!mp)
- return -ENOMEM;
- mp->m_super = sb;
- sb->s_fs_info = mp;
-
- error = xfs_parseargs(mp, (char *)data);
- if (error)
- goto out_free_fsname;
-
- error = __xfs_fs_fill_super(mp, silent);
- if (error)
- goto out_free_fsname;
-
- return 0;
-
- out_free_fsname:
- sb->s_fs_info = NULL;
- xfs_free_fsname(mp);
- kfree(mp);
- return error;
-}
-
STATIC int
xfs_fill_super(
struct super_block *sb,
@@ -2011,16 +1805,6 @@ xfs_fs_put_super(
kfree(mp);
}
-STATIC struct dentry *
-xfs_fs_mount(
- struct file_system_type *fs_type,
- int flags,
- const char *dev_name,
- void *data)
-{
- return mount_bdev(fs_type, flags, dev_name, data, xfs_fs_fill_super);
-}
-
static long
xfs_fs_nr_cached_objects(
struct super_block *sb,
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 02/16] xfs: remove very old mount option
2019-09-20 9:55 ` [PATCH v3 02/16] xfs: remove very old mount option Ian Kent
@ 2019-09-24 10:45 ` Brian Foster
0 siblings, 0 replies; 25+ messages in thread
From: Brian Foster @ 2019-09-24 10:45 UTC (permalink / raw)
To: Ian Kent; +Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Fri, Sep 20, 2019 at 05:55:43PM +0800, Ian Kent wrote:
> It appears the biosize mount option hasn't been documented as
> a vilid option since 2005.
>
> So remove it.
>
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
Works for me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_super.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index f9450235533c..1010097354a6 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -51,7 +51,7 @@ static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */
> * Table driven mount option parser.
> */
> enum {
> - Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize,
> + Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev,
> Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, Opt_nouuid,
> Opt_grpid, Opt_nogrpid, Opt_bsdgroups, Opt_sysvgroups,
> Opt_allocsize, Opt_norecovery, Opt_inode64, Opt_inode32, Opt_ikeep,
> @@ -67,7 +67,6 @@ static const match_table_t tokens = {
> {Opt_logbsize, "logbsize=%s"}, /* size of XFS log buffers */
> {Opt_logdev, "logdev=%s"}, /* log device */
> {Opt_rtdev, "rtdev=%s"}, /* realtime I/O device */
> - {Opt_biosize, "biosize=%u"}, /* log2 of preferred buffered io size */
> {Opt_wsync, "wsync"}, /* safe-mode nfs compatible mount */
> {Opt_noalign, "noalign"}, /* turn off stripe alignment */
> {Opt_swalloc, "swalloc"}, /* turn on stripe width allocation */
> @@ -229,7 +228,6 @@ xfs_parseargs(
> return -ENOMEM;
> break;
> case Opt_allocsize:
> - case Opt_biosize:
> if (suffix_kstrtoint(args, 10, &iosize))
> return -EINVAL;
> iosizelog = ffs(iosize) - 1;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 03/16] xfs: mount-api - add fs parameter description
2019-09-20 9:55 ` [PATCH v3 03/16] xfs: mount-api - add fs parameter description Ian Kent
@ 2019-09-24 10:45 ` Brian Foster
0 siblings, 0 replies; 25+ messages in thread
From: Brian Foster @ 2019-09-24 10:45 UTC (permalink / raw)
To: Ian Kent; +Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Fri, Sep 20, 2019 at 05:55:49PM +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>
> ---
So it looks like we've replaced the fsparam_flag_no usage with explicit
option declarations and the biosize thing is removed. Seems fine to me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_super.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 1010097354a6..9c1ce3d70c08 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;
> @@ -108,6 +110,54 @@ 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_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 ("grpid", Opt_grpid), /* group-ID from parent directory */
> + fsparam_flag ("nogrpid", Opt_nogrpid), /* no group-ID from parent directory */
> + 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 ("ikeep", Opt_ikeep), /* do not free empty inode clusters */
> + fsparam_flag ("noikeep", Opt_noikeep), /* free empty inode clusters */
> + fsparam_flag ("largeio", Opt_largeio), /* report large I/O sizes in stat() */
> + fsparam_flag ("nolargeio", Opt_nolargeio), /* do not report large I/O sizes in stat() */
> + fsparam_flag ("attr2", Opt_attr2), /* do use attr2 attribute format */
> + fsparam_flag ("noattr2", Opt_noattr2), /* do not use attr2 attribute format */
> + fsparam_flag ("filestreams",Opt_filestreams), /* use filestreams allocator */
> + fsparam_flag ("quota", Opt_quota), /* disk quotas (user) */
> + fsparam_flag ("noquota", Opt_noquota), /* no quotas */
> + 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 ("discard", Opt_discard), /* Discard unused blocks */
> + fsparam_flag ("nodiscard", Opt_nodiscard), /* Do 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)
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 04/16] xfs: mount-api - refactor suffix_kstrtoint()
2019-09-20 9:55 ` [PATCH v3 04/16] xfs: mount-api - refactor suffix_kstrtoint() Ian Kent
@ 2019-09-24 10:46 ` Brian Foster
0 siblings, 0 replies; 25+ messages in thread
From: Brian Foster @ 2019-09-24 10:46 UTC (permalink / raw)
To: Ian Kent; +Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Fri, Sep 20, 2019 at 05:55:54PM +0800, Ian Kent wrote:
> The mount-api doesn't have a "human unit" parse type yet so
> the options that have values like "10k" etc. still need to
> be converted by the fs.
>
> But the value comes to the fs as a string (not a substring_t
> type) so there's a need to change the conversion function to
> take a character string instead.
>
> After refactoring xfs_parseargs() and changing it to use
> xfs_parse_param() match_kstrtoint() will no longer be used
> and will be removed.
>
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
Seems fine since this function goes away shortly anyways:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_super.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 9c1ce3d70c08..6a16750b1314 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -160,13 +160,13 @@ static const struct fs_parameter_description xfs_fs_parameters = {
> };
>
> STATIC int
> -suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)
> +suffix_kstrtoint(const char *s, unsigned int base, int *res)
> {
> int last, shift_left_factor = 0, _res;
> char *value;
> int ret = 0;
>
> - value = match_strdup(s);
> + value = kstrdup(s, GFP_KERNEL);
> if (!value)
> return -ENOMEM;
>
> @@ -191,6 +191,20 @@ suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)
> return ret;
> }
>
> +STATIC int
> +match_kstrtoint(const substring_t *s, unsigned int base, int *res)
> +{
> + const char *value;
> + int ret;
> +
> + value = match_strdup(s);
> + if (!value)
> + return -ENOMEM;
> + ret = suffix_kstrtoint(value, base, res);
> + kfree(value);
> + return ret;
> +}
> +
> /*
> * This function fills in xfs_mount_t fields based on mount args.
> * Note: the superblock has _not_ yet been read in.
> @@ -262,7 +276,7 @@ xfs_parseargs(
> return -EINVAL;
> break;
> case Opt_logbsize:
> - if (suffix_kstrtoint(args, 10, &mp->m_logbsize))
> + if (match_kstrtoint(args, 10, &mp->m_logbsize))
> return -EINVAL;
> break;
> case Opt_logdev:
> @@ -278,7 +292,7 @@ xfs_parseargs(
> return -ENOMEM;
> break;
> case Opt_allocsize:
> - if (suffix_kstrtoint(args, 10, &iosize))
> + if (match_kstrtoint(args, 10, &iosize))
> return -EINVAL;
> iosizelog = ffs(iosize) - 1;
> break;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 05/16] xfs: mount-api - refactor xfs_parseags()
2019-09-20 9:55 ` [PATCH v3 05/16] xfs: mount-api - refactor xfs_parseags() Ian Kent
@ 2019-09-24 10:46 ` Brian Foster
0 siblings, 0 replies; 25+ messages in thread
From: Brian Foster @ 2019-09-24 10:46 UTC (permalink / raw)
To: Ian Kent; +Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Fri, Sep 20, 2019 at 05:55:59PM +0800, Ian Kent wrote:
> Refactor xfs_parseags(), move the entire token case block to a
> separate function in an attempt to highlight the code that
> actually changes in converting to use the new mount api.
>
> The only changes are what's needed to communicate the variables
> dsunit, dswidth and iosizelog back to xfs_parseags().
>
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
No changes from the previous version except for the lack of the bio size
options. There's probably no need to drop the r-b tag for that:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/xfs_super.c | 290 ++++++++++++++++++++++++++++------------------------
> 1 file changed, 155 insertions(+), 135 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 6a16750b1314..b04aebab69ab 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -205,6 +205,156 @@ match_kstrtoint(const substring_t *s, unsigned int base, int *res)
> return ret;
> }
>
> +STATIC int
> +xfs_parse_param(
> + int token,
> + char *p,
> + substring_t *args,
> + struct xfs_mount *mp,
> + int *dsunit,
> + int *dswidth,
> + uint8_t *iosizelog)
> +{
> + int iosize = 0;
> +
> + switch (token) {
> + case Opt_logbufs:
> + if (match_int(args, &mp->m_logbufs))
> + return -EINVAL;
> + break;
> + case Opt_logbsize:
> + if (match_kstrtoint(args, 10, &mp->m_logbsize))
> + return -EINVAL;
> + break;
> + case Opt_logdev:
> + kfree(mp->m_logname);
> + mp->m_logname = match_strdup(args);
> + if (!mp->m_logname)
> + return -ENOMEM;
> + break;
> + case Opt_rtdev:
> + kfree(mp->m_rtname);
> + mp->m_rtname = match_strdup(args);
> + if (!mp->m_rtname)
> + return -ENOMEM;
> + break;
> + case Opt_allocsize:
> + if (match_kstrtoint(args, 10, &iosize))
> + return -EINVAL;
> + *iosizelog = ffs(iosize) - 1;
> + break;
> + case Opt_grpid:
> + case Opt_bsdgroups:
> + mp->m_flags |= XFS_MOUNT_GRPID;
> + break;
> + case Opt_nogrpid:
> + case Opt_sysvgroups:
> + mp->m_flags &= ~XFS_MOUNT_GRPID;
> + break;
> + case Opt_wsync:
> + mp->m_flags |= XFS_MOUNT_WSYNC;
> + break;
> + case Opt_norecovery:
> + mp->m_flags |= XFS_MOUNT_NORECOVERY;
> + break;
> + case Opt_noalign:
> + mp->m_flags |= XFS_MOUNT_NOALIGN;
> + break;
> + case Opt_swalloc:
> + mp->m_flags |= XFS_MOUNT_SWALLOC;
> + break;
> + case Opt_sunit:
> + if (match_int(args, dsunit))
> + return -EINVAL;
> + break;
> + case Opt_swidth:
> + if (match_int(args, dswidth))
> + return -EINVAL;
> + break;
> + case Opt_inode32:
> + mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> + break;
> + case Opt_inode64:
> + mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> + break;
> + case Opt_nouuid:
> + mp->m_flags |= XFS_MOUNT_NOUUID;
> + break;
> + case Opt_ikeep:
> + mp->m_flags |= XFS_MOUNT_IKEEP;
> + break;
> + case Opt_noikeep:
> + mp->m_flags &= ~XFS_MOUNT_IKEEP;
> + break;
> + case Opt_largeio:
> + mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
> + break;
> + case Opt_nolargeio:
> + mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> + break;
> + case Opt_attr2:
> + mp->m_flags |= XFS_MOUNT_ATTR2;
> + break;
> + case Opt_noattr2:
> + mp->m_flags &= ~XFS_MOUNT_ATTR2;
> + mp->m_flags |= XFS_MOUNT_NOATTR2;
> + break;
> + case Opt_filestreams:
> + mp->m_flags |= XFS_MOUNT_FILESTREAMS;
> + break;
> + case Opt_noquota:
> + mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> + mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> + mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> + break;
> + case Opt_quota:
> + case Opt_uquota:
> + case Opt_usrquota:
> + mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> + XFS_UQUOTA_ENFD);
> + break;
> + case Opt_qnoenforce:
> + case Opt_uqnoenforce:
> + mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> + mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> + break;
> + case Opt_pquota:
> + case Opt_prjquota:
> + mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> + XFS_PQUOTA_ENFD);
> + break;
> + case Opt_pqnoenforce:
> + mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> + mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> + break;
> + case Opt_gquota:
> + case Opt_grpquota:
> + mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> + XFS_GQUOTA_ENFD);
> + break;
> + case Opt_gqnoenforce:
> + mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> + mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> + break;
> + case Opt_discard:
> + mp->m_flags |= XFS_MOUNT_DISCARD;
> + break;
> + case Opt_nodiscard:
> + mp->m_flags &= ~XFS_MOUNT_DISCARD;
> + break;
> +#ifdef CONFIG_FS_DAX
> + case Opt_dax:
> + mp->m_flags |= XFS_MOUNT_DAX;
> + break;
> +#endif
> + default:
> + xfs_warn(mp, "unknown mount option [%s].", p);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> /*
> * This function fills in xfs_mount_t fields based on mount args.
> * Note: the superblock has _not_ yet been read in.
> @@ -226,7 +376,6 @@ xfs_parseargs(
> substring_t args[MAX_OPT_ARGS];
> int dsunit = 0;
> int dswidth = 0;
> - int iosize = 0;
> uint8_t iosizelog = 0;
>
> /*
> @@ -265,145 +414,16 @@ xfs_parseargs(
>
> while ((p = strsep(&options, ",")) != NULL) {
> int token;
> + int ret;
>
> if (!*p)
> continue;
>
> token = match_token(p, tokens, args);
> - switch (token) {
> - case Opt_logbufs:
> - if (match_int(args, &mp->m_logbufs))
> - return -EINVAL;
> - break;
> - case Opt_logbsize:
> - if (match_kstrtoint(args, 10, &mp->m_logbsize))
> - return -EINVAL;
> - break;
> - case Opt_logdev:
> - kfree(mp->m_logname);
> - mp->m_logname = match_strdup(args);
> - if (!mp->m_logname)
> - return -ENOMEM;
> - break;
> - case Opt_rtdev:
> - kfree(mp->m_rtname);
> - mp->m_rtname = match_strdup(args);
> - if (!mp->m_rtname)
> - return -ENOMEM;
> - break;
> - case Opt_allocsize:
> - if (match_kstrtoint(args, 10, &iosize))
> - return -EINVAL;
> - iosizelog = ffs(iosize) - 1;
> - break;
> - case Opt_grpid:
> - case Opt_bsdgroups:
> - mp->m_flags |= XFS_MOUNT_GRPID;
> - break;
> - case Opt_nogrpid:
> - case Opt_sysvgroups:
> - mp->m_flags &= ~XFS_MOUNT_GRPID;
> - break;
> - case Opt_wsync:
> - mp->m_flags |= XFS_MOUNT_WSYNC;
> - break;
> - case Opt_norecovery:
> - mp->m_flags |= XFS_MOUNT_NORECOVERY;
> - break;
> - case Opt_noalign:
> - mp->m_flags |= XFS_MOUNT_NOALIGN;
> - break;
> - case Opt_swalloc:
> - mp->m_flags |= XFS_MOUNT_SWALLOC;
> - break;
> - case Opt_sunit:
> - if (match_int(args, &dsunit))
> - return -EINVAL;
> - break;
> - case Opt_swidth:
> - if (match_int(args, &dswidth))
> - return -EINVAL;
> - break;
> - case Opt_inode32:
> - mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> - break;
> - case Opt_inode64:
> - mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> - break;
> - case Opt_nouuid:
> - mp->m_flags |= XFS_MOUNT_NOUUID;
> - break;
> - case Opt_ikeep:
> - mp->m_flags |= XFS_MOUNT_IKEEP;
> - break;
> - case Opt_noikeep:
> - mp->m_flags &= ~XFS_MOUNT_IKEEP;
> - break;
> - case Opt_largeio:
> - mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
> - break;
> - case Opt_nolargeio:
> - mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> - break;
> - case Opt_attr2:
> - mp->m_flags |= XFS_MOUNT_ATTR2;
> - break;
> - case Opt_noattr2:
> - mp->m_flags &= ~XFS_MOUNT_ATTR2;
> - mp->m_flags |= XFS_MOUNT_NOATTR2;
> - break;
> - case Opt_filestreams:
> - mp->m_flags |= XFS_MOUNT_FILESTREAMS;
> - break;
> - case Opt_noquota:
> - mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> - mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> - mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> - break;
> - case Opt_quota:
> - case Opt_uquota:
> - case Opt_usrquota:
> - mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> - XFS_UQUOTA_ENFD);
> - break;
> - case Opt_qnoenforce:
> - case Opt_uqnoenforce:
> - mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> - mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> - break;
> - case Opt_pquota:
> - case Opt_prjquota:
> - mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> - XFS_PQUOTA_ENFD);
> - break;
> - case Opt_pqnoenforce:
> - mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> - mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> - break;
> - case Opt_gquota:
> - case Opt_grpquota:
> - mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> - XFS_GQUOTA_ENFD);
> - break;
> - case Opt_gqnoenforce:
> - mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> - mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> - break;
> - case Opt_discard:
> - mp->m_flags |= XFS_MOUNT_DISCARD;
> - break;
> - case Opt_nodiscard:
> - mp->m_flags &= ~XFS_MOUNT_DISCARD;
> - break;
> -#ifdef CONFIG_FS_DAX
> - case Opt_dax:
> - mp->m_flags |= XFS_MOUNT_DAX;
> - break;
> -#endif
> - default:
> - xfs_warn(mp, "unknown mount option [%s].", p);
> - return -EINVAL;
> - }
> + ret = xfs_parse_param(token, p, args, mp,
> + &dsunit, &dswidth, &iosizelog);
> + if (ret)
> + return ret;
> }
>
> /*
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args
2019-09-20 9:56 ` [PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args Ian Kent
@ 2019-09-24 10:54 ` Brian Foster
2019-09-24 11:00 ` Ian Kent
0 siblings, 1 reply; 25+ messages in thread
From: Brian Foster @ 2019-09-24 10:54 UTC (permalink / raw)
To: Ian Kent; +Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Fri, Sep 20, 2019 at 05:56:04PM +0800, Ian Kent wrote:
> Make xfs_parse_param() take arguments of the fs context operation
> .parse_param() in preparation for switching to use the file system
> mount context for mount.
>
> The function fc_parse() only uses the file system context (fc here)
> when calling log macros warnf() and invalf() which in turn check
> only the fc->log field to determine if the message should be saved
> to a context buffer (for later retrival by userspace) or logged
> using printk().
>
> Also the temporary function match_kstrtoint() is now unused, remove it.
>
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
This fails to compile:
$ make -j 8 M=fs/xfs
CC [M] fs/xfs/xfs_super.o
fs/xfs/xfs_super.c: In function ‘xfs_parseargs’:
fs/xfs/xfs_super.c:461:7: error: ‘ctx’ undeclared (first use in this function)
(ctx->dsunit || ctx->dswidth)) {
^~~
fs/xfs/xfs_super.c:461:7: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [scripts/Makefile.build:280: fs/xfs/xfs_super.o] Error 1
make: *** [Makefile:1624: _module_fs/xfs] Error 2
> fs/xfs/xfs_super.c | 135 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 79 insertions(+), 56 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index b04aebab69ab..041ab8b52a7d 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -191,57 +191,60 @@ suffix_kstrtoint(const char *s, unsigned int base, int *res)
> return ret;
> }
>
...
> STATIC int
> xfs_parse_param(
...
>
> - switch (token) {
> + opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> + if (opt < 0) {
> + /*
> + * If fs_parse() returns -ENOPARAM and the parameter
> + * is "source" the VFS needs to handle this option
> + * in order to boot otherwise use the default case
> + * below to handle invalid options.
> + */
> + if (opt != -ENOPARAM ||
> + strcmp(param->key, "source") == 0)
> + return opt;
Why is this not something that is handled in core mount-api code? Every
filesystem needs this logic in order to be a rootfs..?
Brian
> + }
> +
> + switch (opt) {
> case Opt_logbufs:
> - if (match_int(args, &mp->m_logbufs))
> - return -EINVAL;
> + mp->m_logbufs = result.uint_32;
> break;
> case Opt_logbsize:
> - if (match_kstrtoint(args, 10, &mp->m_logbsize))
> + if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
> return -EINVAL;
> break;
> case Opt_logdev:
> kfree(mp->m_logname);
> - mp->m_logname = match_strdup(args);
> + mp->m_logname = kstrdup(param->string, GFP_KERNEL);
> if (!mp->m_logname)
> return -ENOMEM;
> break;
> case Opt_rtdev:
> kfree(mp->m_rtname);
> - mp->m_rtname = match_strdup(args);
> + mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
> if (!mp->m_rtname)
> return -ENOMEM;
> break;
> case Opt_allocsize:
> - if (match_kstrtoint(args, 10, &iosize))
> + if (suffix_kstrtoint(param->string, 10, &iosize))
> return -EINVAL;
> - *iosizelog = ffs(iosize) - 1;
> + ctx->iosizelog = ffs(iosize) - 1;
> break;
> case Opt_grpid:
> case Opt_bsdgroups:
> @@ -264,12 +267,10 @@ xfs_parse_param(
> mp->m_flags |= XFS_MOUNT_SWALLOC;
> break;
> case Opt_sunit:
> - if (match_int(args, dsunit))
> - return -EINVAL;
> + ctx->dsunit = result.uint_32;
> break;
> case Opt_swidth:
> - if (match_int(args, dswidth))
> - return -EINVAL;
> + ctx->dswidth = result.uint_32;
> break;
> case Opt_inode32:
> mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> @@ -348,7 +349,7 @@ xfs_parse_param(
> break;
> #endif
> default:
> - xfs_warn(mp, "unknown mount option [%s].", p);
> + xfs_warn(mp, "unknown mount option [%s].", param->key);
> return -EINVAL;
> }
>
> @@ -373,10 +374,13 @@ xfs_parseargs(
> {
> const struct super_block *sb = mp->m_super;
> char *p;
> - substring_t args[MAX_OPT_ARGS];
> - int dsunit = 0;
> - int dswidth = 0;
> - uint8_t iosizelog = 0;
> + struct fs_context fc;
> + struct xfs_fs_context context;
> +
> + memset(&fc, 0, sizeof(fc));
> + memset(&context, 0, sizeof(context));
> + fc.fs_private = &context;
> + fc.s_fs_info = mp;
>
> /*
> * set up the mount name first so all the errors will refer to the
> @@ -413,16 +417,34 @@ xfs_parseargs(
> goto done;
>
> while ((p = strsep(&options, ",")) != NULL) {
> - int token;
> - int ret;
> + struct fs_parameter param;
> + char *value;
> + int ret;
>
> if (!*p)
> continue;
>
> - token = match_token(p, tokens, args);
> - ret = xfs_parse_param(token, p, args, mp,
> - &dsunit, &dswidth, &iosizelog);
> - if (ret)
> + param.key = p;
> + param.type = fs_value_is_string;
> + param.string = NULL;
> + param.size = 0;
> +
> + value = strchr(p, '=');
> + if (value) {
> + *value++ = 0;
> + param.size = strlen(value);
> + if (param.size > 0) {
> + param.string = kmemdup_nul(value,
> + param.size,
> + GFP_KERNEL);
> + if (!param.string)
> + return -ENOMEM;
> + }
> + }
> +
> + ret = xfs_parse_param(&fc, ¶m);
> + kfree(param.string);
> + if (ret < 0)
> return ret;
> }
>
> @@ -435,7 +457,8 @@ xfs_parseargs(
> return -EINVAL;
> }
>
> - if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (dsunit || dswidth)) {
> + if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
> + (ctx->dsunit || ctx->dswidth)) {
> xfs_warn(mp,
> "sunit and swidth options incompatible with the noalign option");
> return -EINVAL;
> @@ -448,28 +471,28 @@ xfs_parseargs(
> }
> #endif
>
> - if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
> + if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
> xfs_warn(mp, "sunit and swidth must be specified together");
> return -EINVAL;
> }
>
> - if (dsunit && (dswidth % dsunit != 0)) {
> + if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
> xfs_warn(mp,
> "stripe width (%d) must be a multiple of the stripe unit (%d)",
> - dswidth, dsunit);
> + ctx->dswidth, ctx->dsunit);
> return -EINVAL;
> }
>
> done:
> - if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> + if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> /*
> * At this point the superblock has not been read
> * in, therefore we do not know the block size.
> * Before the mount call ends we will convert
> * these to FSBs.
> */
> - mp->m_dalign = dsunit;
> - mp->m_swidth = dswidth;
> + mp->m_dalign = ctx->dsunit;
> + mp->m_swidth = ctx->dswidth;
> }
>
> if (mp->m_logbufs != -1 &&
> @@ -491,18 +514,18 @@ xfs_parseargs(
> return -EINVAL;
> }
>
> - if (iosizelog) {
> - if (iosizelog > XFS_MAX_IO_LOG ||
> - iosizelog < XFS_MIN_IO_LOG) {
> + if (ctx->iosizelog) {
> + if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> + ctx->iosizelog < XFS_MIN_IO_LOG) {
> xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
> - iosizelog, XFS_MIN_IO_LOG,
> + ctx->iosizelog, XFS_MIN_IO_LOG,
> XFS_MAX_IO_LOG);
> return -EINVAL;
> }
>
> mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> - mp->m_readio_log = iosizelog;
> - mp->m_writeio_log = iosizelog;
> + mp->m_readio_log = ctx->iosizelog;
> + mp->m_writeio_log = ctx->iosizelog;
> }
>
> return 0;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 07/16] xfs: mount-api - move xfs_parseargs() validation to a helper
2019-09-20 9:56 ` [PATCH v3 07/16] xfs: mount-api - move xfs_parseargs() validation to a helper Ian Kent
@ 2019-09-24 10:56 ` Brian Foster
2019-09-24 11:41 ` Ian Kent
0 siblings, 1 reply; 25+ messages in thread
From: Brian Foster @ 2019-09-24 10:56 UTC (permalink / raw)
To: Ian Kent; +Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Fri, Sep 20, 2019 at 05:56:09PM +0800, Ian Kent wrote:
> Move the validation code of xfs_parseargs() into a helper for later
> use within the mount context methods.
>
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
More compile issues:
$ make -j 8 M=fs/xfs
CC [M] fs/xfs/xfs_super.o
...
fs/xfs/xfs_super.c:543:7: note: each undeclared identifier is reported only once for each function it appears in
fs/xfs/xfs_super.c:569:2: error: ‘ret’ undeclared (first use in this function); did you mean ‘net’?
ret = xfs_validate_params(mp, &context, false);
^~~
net
fs/xfs/xfs_super.c:572:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
make[1]: *** [scripts/Makefile.build:280: fs/xfs/xfs_super.o] Error 1
make: *** [Makefile:1624: _module_fs/xfs] Error 2
I'll probably stop here for now since clearly this and the previous
patch need some tweaks and I'd rather not review around compile errors.
Brian
> fs/xfs/xfs_super.c | 148 +++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 94 insertions(+), 54 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 041ab8b52a7d..5cb9a9fd1a15 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -356,6 +356,97 @@ xfs_parse_param(
> return 0;
> }
>
> +STATIC int
> +xfs_validate_params(
> + struct xfs_mount *mp,
> + struct xfs_fs_context *ctx,
> + bool nooptions)
> +{
> + if (nooptions)
> + goto noopts;
> +
> + /*
> + * no recovery flag requires a read-only mount
> + */
> + if ((mp->m_flags & XFS_MOUNT_NORECOVERY) &&
> + !(mp->m_flags & XFS_MOUNT_RDONLY)) {
> + xfs_warn(mp, "no-recovery mounts must be read-only.");
> + return -EINVAL;
> + }
> +
> + if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (ctx->dsunit || ctx->dswidth)) {
> + xfs_warn(mp,
> + "sunit and swidth options incompatible with the noalign option");
> + return -EINVAL;
> + }
> +
> +#ifndef CONFIG_XFS_QUOTA
> + if (XFS_IS_QUOTA_RUNNING(mp)) {
> + xfs_warn(mp, "quota support not available in this kernel.");
> + return -EINVAL;
> + }
> +#endif
> +
> + if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
> + xfs_warn(mp, "sunit and swidth must be specified together");
> + return -EINVAL;
> + }
> +
> + if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
> + xfs_warn(mp,
> + "stripe width (%d) must be a multiple of the stripe unit (%d)",
> + ctx->dswidth, ctx->dsunit);
> + return -EINVAL;
> + }
> +
> +noopts:
> + if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> + /*
> + * At this point the superblock has not been read
> + * in, therefore we do not know the block size.
> + * Before the mount call ends we will convert
> + * these to FSBs.
> + */
> + mp->m_dalign = ctx->dsunit;
> + mp->m_swidth = ctx->dswidth;
> + }
> +
> + if (mp->m_logbufs != -1 &&
> + mp->m_logbufs != 0 &&
> + (mp->m_logbufs < XLOG_MIN_ICLOGS ||
> + mp->m_logbufs > XLOG_MAX_ICLOGS)) {
> + xfs_warn(mp, "invalid logbufs value: %d [not %d-%d]",
> + mp->m_logbufs, XLOG_MIN_ICLOGS, XLOG_MAX_ICLOGS);
> + return -EINVAL;
> + }
> + if (mp->m_logbsize != -1 &&
> + mp->m_logbsize != 0 &&
> + (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
> + mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
> + !is_power_of_2(mp->m_logbsize))) {
> + xfs_warn(mp,
> + "invalid logbufsize: %d [not 16k,32k,64k,128k or 256k]",
> + mp->m_logbsize);
> + return -EINVAL;
> + }
> +
> + if (ctx->iosizelog) {
> + if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> + ctx->iosizelog < XFS_MIN_IO_LOG) {
> + xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
> + ctx->iosizelog, XFS_MIN_IO_LOG,
> + XFS_MAX_IO_LOG);
> + return -EINVAL;
> + }
> +
> + mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> + mp->m_readio_log = ctx->iosizelog;
> + mp->m_writeio_log = ctx->iosizelog;
> + }
> +
> + return 0;
> +}
> +
> /*
> * This function fills in xfs_mount_t fields based on mount args.
> * Note: the superblock has _not_ yet been read in.
> @@ -445,16 +536,7 @@ xfs_parseargs(
> ret = xfs_parse_param(&fc, ¶m);
> kfree(param.string);
> if (ret < 0)
> - return ret;
> - }
> -
> - /*
> - * no recovery flag requires a read-only mount
> - */
> - if ((mp->m_flags & XFS_MOUNT_NORECOVERY) &&
> - !(mp->m_flags & XFS_MOUNT_RDONLY)) {
> - xfs_warn(mp, "no-recovery mounts must be read-only.");
> - return -EINVAL;
> + goto done;
> }
>
> if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
> @@ -484,51 +566,9 @@ xfs_parseargs(
> }
>
> done:
> - if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> - /*
> - * At this point the superblock has not been read
> - * in, therefore we do not know the block size.
> - * Before the mount call ends we will convert
> - * these to FSBs.
> - */
> - mp->m_dalign = ctx->dsunit;
> - mp->m_swidth = ctx->dswidth;
> - }
> -
> - if (mp->m_logbufs != -1 &&
> - mp->m_logbufs != 0 &&
> - (mp->m_logbufs < XLOG_MIN_ICLOGS ||
> - mp->m_logbufs > XLOG_MAX_ICLOGS)) {
> - xfs_warn(mp, "invalid logbufs value: %d [not %d-%d]",
> - mp->m_logbufs, XLOG_MIN_ICLOGS, XLOG_MAX_ICLOGS);
> - return -EINVAL;
> - }
> - if (mp->m_logbsize != -1 &&
> - mp->m_logbsize != 0 &&
> - (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
> - mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
> - !is_power_of_2(mp->m_logbsize))) {
> - xfs_warn(mp,
> - "invalid logbufsize: %d [not 16k,32k,64k,128k or 256k]",
> - mp->m_logbsize);
> - return -EINVAL;
> - }
> + ret = xfs_validate_params(mp, &context, false);
>
> - if (ctx->iosizelog) {
> - if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> - ctx->iosizelog < XFS_MIN_IO_LOG) {
> - xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
> - ctx->iosizelog, XFS_MIN_IO_LOG,
> - XFS_MAX_IO_LOG);
> - return -EINVAL;
> - }
> -
> - mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> - mp->m_readio_log = ctx->iosizelog;
> - mp->m_writeio_log = ctx->iosizelog;
> - }
> -
> - return 0;
> + return ret;
> }
>
> struct proc_xfs_info {
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args
2019-09-24 10:54 ` Brian Foster
@ 2019-09-24 11:00 ` Ian Kent
0 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-24 11:00 UTC (permalink / raw)
To: Brian Foster
Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Tue, 2019-09-24 at 06:54 -0400, Brian Foster wrote:
> On Fri, Sep 20, 2019 at 05:56:04PM +0800, Ian Kent wrote:
> > Make xfs_parse_param() take arguments of the fs context operation
> > .parse_param() in preparation for switching to use the file system
> > mount context for mount.
> >
> > The function fc_parse() only uses the file system context (fc here)
> > when calling log macros warnf() and invalf() which in turn check
> > only the fc->log field to determine if the message should be saved
> > to a context buffer (for later retrival by userspace) or logged
> > using printk().
> >
> > Also the temporary function match_kstrtoint() is now unused, remove
> > it.
> >
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > ---
>
> This fails to compile:
>
> $ make -j 8 M=fs/xfs
> CC [M] fs/xfs/xfs_super.o
> fs/xfs/xfs_super.c: In function ‘xfs_parseargs’:
> fs/xfs/xfs_super.c:461:7: error: ‘ctx’ undeclared (first use in this
> function)
> (ctx->dsunit || ctx->dswidth)) {
> ^~~
> fs/xfs/xfs_super.c:461:7: note: each undeclared identifier is
> reported only once for each function it appears in
> make[1]: *** [scripts/Makefile.build:280: fs/xfs/xfs_super.o] Error 1
> make: *** [Makefile:1624: _module_fs/xfs] Error 2
No!
Arrgh, I must have mixed up my patch series.
This was a mistake I fixed when I went through and compiled with
each patch just before posting.
Looks like I'll need to post a v4 then with this fixed, *sigh*.
>
> > fs/xfs/xfs_super.c | 135 ++++++++++++++++++++++++++++++--------
> > --------------
> > 1 file changed, 79 insertions(+), 56 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index b04aebab69ab..041ab8b52a7d 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -191,57 +191,60 @@ suffix_kstrtoint(const char *s, unsigned int
> > base, int *res)
> > return ret;
> > }
> >
> ...
> > STATIC int
> > xfs_parse_param(
> ...
> >
> > - switch (token) {
> > + opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> > + if (opt < 0) {
> > + /*
> > + * If fs_parse() returns -ENOPARAM and the parameter
> > + * is "source" the VFS needs to handle this option
> > + * in order to boot otherwise use the default case
> > + * below to handle invalid options.
> > + */
> > + if (opt != -ENOPARAM ||
> > + strcmp(param->key, "source") == 0)
> > + return opt;
>
> Why is this not something that is handled in core mount-api code?
> Every
> filesystem needs this logic in order to be a rootfs..?
>
> Brian
>
> > + }
> > +
> > + switch (opt) {
> > case Opt_logbufs:
> > - if (match_int(args, &mp->m_logbufs))
> > - return -EINVAL;
> > + mp->m_logbufs = result.uint_32;
> > break;
> > case Opt_logbsize:
> > - if (match_kstrtoint(args, 10, &mp->m_logbsize))
> > + if (suffix_kstrtoint(param->string, 10, &mp-
> > >m_logbsize))
> > return -EINVAL;
> > break;
> > case Opt_logdev:
> > kfree(mp->m_logname);
> > - mp->m_logname = match_strdup(args);
> > + mp->m_logname = kstrdup(param->string, GFP_KERNEL);
> > if (!mp->m_logname)
> > return -ENOMEM;
> > break;
> > case Opt_rtdev:
> > kfree(mp->m_rtname);
> > - mp->m_rtname = match_strdup(args);
> > + mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
> > if (!mp->m_rtname)
> > return -ENOMEM;
> > break;
> > case Opt_allocsize:
> > - if (match_kstrtoint(args, 10, &iosize))
> > + if (suffix_kstrtoint(param->string, 10, &iosize))
> > return -EINVAL;
> > - *iosizelog = ffs(iosize) - 1;
> > + ctx->iosizelog = ffs(iosize) - 1;
> > break;
> > case Opt_grpid:
> > case Opt_bsdgroups:
> > @@ -264,12 +267,10 @@ xfs_parse_param(
> > mp->m_flags |= XFS_MOUNT_SWALLOC;
> > break;
> > case Opt_sunit:
> > - if (match_int(args, dsunit))
> > - return -EINVAL;
> > + ctx->dsunit = result.uint_32;
> > break;
> > case Opt_swidth:
> > - if (match_int(args, dswidth))
> > - return -EINVAL;
> > + ctx->dswidth = result.uint_32;
> > break;
> > case Opt_inode32:
> > mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> > @@ -348,7 +349,7 @@ xfs_parse_param(
> > break;
> > #endif
> > default:
> > - xfs_warn(mp, "unknown mount option [%s].", p);
> > + xfs_warn(mp, "unknown mount option [%s].", param->key);
> > return -EINVAL;
> > }
> >
> > @@ -373,10 +374,13 @@ xfs_parseargs(
> > {
> > const struct super_block *sb = mp->m_super;
> > char *p;
> > - substring_t args[MAX_OPT_ARGS];
> > - int dsunit = 0;
> > - int dswidth = 0;
> > - uint8_t iosizelog = 0;
> > + struct fs_context fc;
> > + struct xfs_fs_context context;
> > +
> > + memset(&fc, 0, sizeof(fc));
> > + memset(&context, 0, sizeof(context));
> > + fc.fs_private = &context;
> > + fc.s_fs_info = mp;
> >
> > /*
> > * set up the mount name first so all the errors will refer to
> > the
> > @@ -413,16 +417,34 @@ xfs_parseargs(
> > goto done;
> >
> > while ((p = strsep(&options, ",")) != NULL) {
> > - int token;
> > - int ret;
> > + struct fs_parameter param;
> > + char *value;
> > + int ret;
> >
> > if (!*p)
> > continue;
> >
> > - token = match_token(p, tokens, args);
> > - ret = xfs_parse_param(token, p, args, mp,
> > - &dsunit, &dswidth, &iosizelog);
> > - if (ret)
> > + param.key = p;
> > + param.type = fs_value_is_string;
> > + param.string = NULL;
> > + param.size = 0;
> > +
> > + value = strchr(p, '=');
> > + if (value) {
> > + *value++ = 0;
> > + param.size = strlen(value);
> > + if (param.size > 0) {
> > + param.string = kmemdup_nul(value,
> > + param.size,
> > + GFP_KERNEL);
> > + if (!param.string)
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + ret = xfs_parse_param(&fc, ¶m);
> > + kfree(param.string);
> > + if (ret < 0)
> > return ret;
> > }
> >
> > @@ -435,7 +457,8 @@ xfs_parseargs(
> > return -EINVAL;
> > }
> >
> > - if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (dsunit || dswidth)) {
> > + if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
> > + (ctx->dsunit || ctx->dswidth)) {
> > xfs_warn(mp,
> > "sunit and swidth options incompatible with the noalign
> > option");
> > return -EINVAL;
> > @@ -448,28 +471,28 @@ xfs_parseargs(
> > }
> > #endif
> >
> > - if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
> > + if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx-
> > >dswidth)) {
> > xfs_warn(mp, "sunit and swidth must be specified
> > together");
> > return -EINVAL;
> > }
> >
> > - if (dsunit && (dswidth % dsunit != 0)) {
> > + if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
> > xfs_warn(mp,
> > "stripe width (%d) must be a multiple of the stripe unit (%d)",
> > - dswidth, dsunit);
> > + ctx->dswidth, ctx->dsunit);
> > return -EINVAL;
> > }
> >
> > done:
> > - if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> > + if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> > /*
> > * At this point the superblock has not been read
> > * in, therefore we do not know the block size.
> > * Before the mount call ends we will convert
> > * these to FSBs.
> > */
> > - mp->m_dalign = dsunit;
> > - mp->m_swidth = dswidth;
> > + mp->m_dalign = ctx->dsunit;
> > + mp->m_swidth = ctx->dswidth;
> > }
> >
> > if (mp->m_logbufs != -1 &&
> > @@ -491,18 +514,18 @@ xfs_parseargs(
> > return -EINVAL;
> > }
> >
> > - if (iosizelog) {
> > - if (iosizelog > XFS_MAX_IO_LOG ||
> > - iosizelog < XFS_MIN_IO_LOG) {
> > + if (ctx->iosizelog) {
> > + if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> > + ctx->iosizelog < XFS_MIN_IO_LOG) {
> > xfs_warn(mp, "invalid log iosize: %d [not %d-
> > %d]",
> > - iosizelog, XFS_MIN_IO_LOG,
> > + ctx->iosizelog, XFS_MIN_IO_LOG,
> > XFS_MAX_IO_LOG);
> > return -EINVAL;
> > }
> >
> > mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> > - mp->m_readio_log = iosizelog;
> > - mp->m_writeio_log = iosizelog;
> > + mp->m_readio_log = ctx->iosizelog;
> > + mp->m_writeio_log = ctx->iosizelog;
> > }
> >
> > return 0;
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 07/16] xfs: mount-api - move xfs_parseargs() validation to a helper
2019-09-24 10:56 ` Brian Foster
@ 2019-09-24 11:41 ` Ian Kent
0 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-09-24 11:41 UTC (permalink / raw)
To: Brian Foster
Cc: linux-xfs, David Howells, Dave Chinner, Al Viro, Eric Sandeen
On Tue, 2019-09-24 at 06:56 -0400, Brian Foster wrote:
> On Fri, Sep 20, 2019 at 05:56:09PM +0800, Ian Kent wrote:
> > Move the validation code of xfs_parseargs() into a helper for later
> > use within the mount context methods.
> >
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > ---
>
> More compile issues:
>
> $ make -j 8 M=fs/xfs
> CC [M] fs/xfs/xfs_super.o
> ...
> fs/xfs/xfs_super.c:543:7: note: each undeclared identifier is
> reported only once for each function it appears in
> fs/xfs/xfs_super.c:569:2: error: ‘ret’ undeclared (first use in this
> function); did you mean ‘net’?
> ret = xfs_validate_params(mp, &context, false);
> ^~~
> net
> fs/xfs/xfs_super.c:572:1: warning: control reaches end of non-void
> function [-Wreturn-type]
> }
> ^
> make[1]: *** [scripts/Makefile.build:280: fs/xfs/xfs_super.o] Error 1
> make: *** [Makefile:1624: _module_fs/xfs] Error 2
>
> I'll probably stop here for now since clearly this and the previous
> patch need some tweaks and I'd rather not review around compile
> errors.
Yep, I've found the series that should have been posted.
There are some other changes later on too to remove a couple
of function defined but not used warnings.
Let me repost the series.
I should check again to make absolutely sure I have them
right though so give me a bit.
>
> Brian
>
> > fs/xfs/xfs_super.c | 148 +++++++++++++++++++++++++++++++++-------
> > ------------
> > 1 file changed, 94 insertions(+), 54 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 041ab8b52a7d..5cb9a9fd1a15 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -356,6 +356,97 @@ xfs_parse_param(
> > return 0;
> > }
> >
> > +STATIC int
> > +xfs_validate_params(
> > + struct xfs_mount *mp,
> > + struct xfs_fs_context *ctx,
> > + bool nooptions)
> > +{
> > + if (nooptions)
> > + goto noopts;
> > +
> > + /*
> > + * no recovery flag requires a read-only mount
> > + */
> > + if ((mp->m_flags & XFS_MOUNT_NORECOVERY) &&
> > + !(mp->m_flags & XFS_MOUNT_RDONLY)) {
> > + xfs_warn(mp, "no-recovery mounts must be read-only.");
> > + return -EINVAL;
> > + }
> > +
> > + if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (ctx->dsunit || ctx-
> > >dswidth)) {
> > + xfs_warn(mp,
> > + "sunit and swidth options incompatible with the noalign
> > option");
> > + return -EINVAL;
> > + }
> > +
> > +#ifndef CONFIG_XFS_QUOTA
> > + if (XFS_IS_QUOTA_RUNNING(mp)) {
> > + xfs_warn(mp, "quota support not available in this
> > kernel.");
> > + return -EINVAL;
> > + }
> > +#endif
> > +
> > + if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx-
> > >dswidth)) {
> > + xfs_warn(mp, "sunit and swidth must be specified
> > together");
> > + return -EINVAL;
> > + }
> > +
> > + if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
> > + xfs_warn(mp,
> > + "stripe width (%d) must be a multiple of the stripe unit (%d)",
> > + ctx->dswidth, ctx->dsunit);
> > + return -EINVAL;
> > + }
> > +
> > +noopts:
> > + if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> > + /*
> > + * At this point the superblock has not been read
> > + * in, therefore we do not know the block size.
> > + * Before the mount call ends we will convert
> > + * these to FSBs.
> > + */
> > + mp->m_dalign = ctx->dsunit;
> > + mp->m_swidth = ctx->dswidth;
> > + }
> > +
> > + if (mp->m_logbufs != -1 &&
> > + mp->m_logbufs != 0 &&
> > + (mp->m_logbufs < XLOG_MIN_ICLOGS ||
> > + mp->m_logbufs > XLOG_MAX_ICLOGS)) {
> > + xfs_warn(mp, "invalid logbufs value: %d [not %d-%d]",
> > + mp->m_logbufs, XLOG_MIN_ICLOGS,
> > XLOG_MAX_ICLOGS);
> > + return -EINVAL;
> > + }
> > + if (mp->m_logbsize != -1 &&
> > + mp->m_logbsize != 0 &&
> > + (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
> > + mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
> > + !is_power_of_2(mp->m_logbsize))) {
> > + xfs_warn(mp,
> > + "invalid logbufsize: %d [not 16k,32k,64k,128k
> > or 256k]",
> > + mp->m_logbsize);
> > + return -EINVAL;
> > + }
> > +
> > + if (ctx->iosizelog) {
> > + if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> > + ctx->iosizelog < XFS_MIN_IO_LOG) {
> > + xfs_warn(mp, "invalid log iosize: %d [not %d-
> > %d]",
> > + ctx->iosizelog, XFS_MIN_IO_LOG,
> > + XFS_MAX_IO_LOG);
> > + return -EINVAL;
> > + }
> > +
> > + mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> > + mp->m_readio_log = ctx->iosizelog;
> > + mp->m_writeio_log = ctx->iosizelog;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * This function fills in xfs_mount_t fields based on mount args.
> > * Note: the superblock has _not_ yet been read in.
> > @@ -445,16 +536,7 @@ xfs_parseargs(
> > ret = xfs_parse_param(&fc, ¶m);
> > kfree(param.string);
> > if (ret < 0)
> > - return ret;
> > - }
> > -
> > - /*
> > - * no recovery flag requires a read-only mount
> > - */
> > - if ((mp->m_flags & XFS_MOUNT_NORECOVERY) &&
> > - !(mp->m_flags & XFS_MOUNT_RDONLY)) {
> > - xfs_warn(mp, "no-recovery mounts must be read-only.");
> > - return -EINVAL;
> > + goto done;
> > }
> >
> > if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
> > @@ -484,51 +566,9 @@ xfs_parseargs(
> > }
> >
> > done:
> > - if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> > - /*
> > - * At this point the superblock has not been read
> > - * in, therefore we do not know the block size.
> > - * Before the mount call ends we will convert
> > - * these to FSBs.
> > - */
> > - mp->m_dalign = ctx->dsunit;
> > - mp->m_swidth = ctx->dswidth;
> > - }
> > -
> > - if (mp->m_logbufs != -1 &&
> > - mp->m_logbufs != 0 &&
> > - (mp->m_logbufs < XLOG_MIN_ICLOGS ||
> > - mp->m_logbufs > XLOG_MAX_ICLOGS)) {
> > - xfs_warn(mp, "invalid logbufs value: %d [not %d-%d]",
> > - mp->m_logbufs, XLOG_MIN_ICLOGS,
> > XLOG_MAX_ICLOGS);
> > - return -EINVAL;
> > - }
> > - if (mp->m_logbsize != -1 &&
> > - mp->m_logbsize != 0 &&
> > - (mp->m_logbsize < XLOG_MIN_RECORD_BSIZE ||
> > - mp->m_logbsize > XLOG_MAX_RECORD_BSIZE ||
> > - !is_power_of_2(mp->m_logbsize))) {
> > - xfs_warn(mp,
> > - "invalid logbufsize: %d [not 16k,32k,64k,128k
> > or 256k]",
> > - mp->m_logbsize);
> > - return -EINVAL;
> > - }
> > + ret = xfs_validate_params(mp, &context, false);
> >
> > - if (ctx->iosizelog) {
> > - if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> > - ctx->iosizelog < XFS_MIN_IO_LOG) {
> > - xfs_warn(mp, "invalid log iosize: %d [not %d-
> > %d]",
> > - ctx->iosizelog, XFS_MIN_IO_LOG,
> > - XFS_MAX_IO_LOG);
> > - return -EINVAL;
> > - }
> > -
> > - mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> > - mp->m_readio_log = ctx->iosizelog;
> > - mp->m_writeio_log = ctx->iosizelog;
> > - }
> > -
> > - return 0;
> > + return ret;
> > }
> >
> > struct proc_xfs_info {
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2019-09-24 11:41 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-20 9:55 [PATCH v3 00/16] xfs: mount API patch series Ian Kent
2019-09-20 9:55 ` [PATCH v3 01/16] vfs: Create fs_context-aware mount_bdev() replacement Ian Kent
2019-09-20 9:55 ` [PATCH v3 02/16] xfs: remove very old mount option Ian Kent
2019-09-24 10:45 ` Brian Foster
2019-09-20 9:55 ` [PATCH v3 03/16] xfs: mount-api - add fs parameter description Ian Kent
2019-09-24 10:45 ` Brian Foster
2019-09-20 9:55 ` [PATCH v3 04/16] xfs: mount-api - refactor suffix_kstrtoint() Ian Kent
2019-09-24 10:46 ` Brian Foster
2019-09-20 9:55 ` [PATCH v3 05/16] xfs: mount-api - refactor xfs_parseags() Ian Kent
2019-09-24 10:46 ` Brian Foster
2019-09-20 9:56 ` [PATCH v3 06/16] xfs: mount-api - make xfs_parse_param() take context .parse_param() args Ian Kent
2019-09-24 10:54 ` Brian Foster
2019-09-24 11:00 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 07/16] xfs: mount-api - move xfs_parseargs() validation to a helper Ian Kent
2019-09-24 10:56 ` Brian Foster
2019-09-24 11:41 ` Ian Kent
2019-09-20 9:56 ` [PATCH v3 08/16] xfs: mount-api - refactor xfs_fs_fill_super() Ian Kent
2019-09-20 9:56 ` [PATCH v3 09/16] xfs: mount-api - add xfs_get_tree() Ian Kent
2019-09-20 9:56 ` [PATCH v3 10/16] xfs: mount-api - add xfs_remount_rw() helper Ian Kent
2019-09-20 9:56 ` [PATCH v3 11/16] xfs: mount-api - add xfs_remount_ro() helper Ian Kent
2019-09-20 9:56 ` [PATCH v3 12/16] xfs: mount api - add xfs_reconfigure() Ian Kent
2019-09-20 9:56 ` [PATCH v3 13/16] xfs: mount-api - add xfs_fc_free() Ian Kent
2019-09-20 9:56 ` [PATCH v3 14/16] xfs: mount-api - dont set sb in xfs_mount_alloc() Ian Kent
2019-09-20 9:56 ` [PATCH v3 15/16] xfs: mount-api - switch to new mount-api Ian Kent
2019-09-20 9:56 ` [PATCH v3 16/16] xfs: mount-api - remove legacy mount functions Ian Kent
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).