All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 00/17] xfs: mount API patch series
@ 2019-11-04 10:54 Ian Kent
  2019-11-04 10:54 ` [PATCH v9 01/17] xfs: remove unused struct xfs_mount field m_fsname_len Ian Kent
                   ` (16 more replies)
  0 siblings, 17 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:54 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

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 for-next branch, and
built and some simple tests run on it along with the default xfstests.

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 sometimes inconsistent logging.

  The mount-api feature of saving error message text to the mount
  context for later retrieval 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 so it which can lead to messages, possibly needed for debug
  purposes, not being sent to the kernel log. There's also the pr_xxx()
  log functions (not a problem for xfs AFAICS) that aren't aware of the
  mount context at all.

  In the xfs patches I've used the same method that is used in gfs2 and
  was suggested by Al Viro (essentially return the error if fs_parse()
  returns one) except that I've also not used the mount api log macros
  to minimise the possibility of lost log messages.

  This isn't the best so follow up patches for RFC (with a slightly
  wider audience) will be needed to try and improve this aspect of the
  mount api. 

Changes for v9:
- fix coding style in the patch to switch to new mount-api.
- fix sync_filesystem() call order.
- fold xfs_mount-alloc() into xfs_init_fs_context().

Changes for v8:
- update to for-next 21f55993eb7a.
- accomodate Christoph's options handling changes (thanks Christoph).
- remove redundant check in __xfs_printk().
- add missing sync_filesystem() in .reconfigure (fixes assertion fail
  reported by Darrick).
- fix IS_ENABLED() macro usage.
- move the xfs_fc_*() functions and related functions above the struct
  fs_context_operations to bring the parameter parsing and mount
  handling code together.

Changes for v7:
- fix s/mount info/struct xfs_mount/g in the patches that remove the
  m_fsname_len and m_fsname struct xfs_mount fields.
- also use IS_ENABLED() macro for the quota config check.
- don't use typedef'ed struct definitions in new or chencged code.
- extend comment lines to use full 80 characters and to 72 characters
  for commit log entries.
- fix function parameter and variable declaration style inconsistencies.
- use "return 0" instead of "break" in xfs_parse_param() as recommended.
- avoid redundant option checks for the case where no options are given.
- merge freeing of mp names and mp.
- change name field of struct fs_parameter_description to lower case.
- group mount related code together as much as possible to enhance
  readability.
- changed mount api related functions to all use an "xfs_fc" prefix.

Changes for v6:
- drop get_tree_bdev() patch since it's now present in the xfs-linux
  repo.
- eliminate unused mount info. field m_fsname_len.
- eliminate mount info field m_fsname, use super block field s_id
  instead.
- dont use XFS_IS_QUOTA_RUNNING() when checking if quota options have
  been specified.
- combine mount-api specific patches into a single patch to help with
  bi-sectability.

Changes for v5:
- give error exit label in xfs_fill_super() a sensible name.
- use original comment about options handling in xfs_fs_remount() for
  comment above xfs_reconfigure().
- use a much simpler comment in xfs_fc_free(), thanks to Brian Foster.
- move cover letter comment about the first two patches above the
  revision comentary so it isn't missed. 

Changes for v4:
- changed xfs_fill_super() cleanup back to what it was in v2, until I
  can work out what's causing the problem had previously seen (I can't
  reproduce it myself), since it looks like it was right from the start.
- use get_tree_bdev() instead of vfs_get_block_super() in xfs_get_tree()
  as requested by Al Viro.
- removed redundant initialisation in xfs_fs_fill_super().
- fix long line in xfs_validate_params().
- no need to validate if parameter parsing fails, just return the error.
- summarise reconfigure comment about option handling, transfer bulk of
  comment to commit log message.
- use minimal change in xfs_parse_param(), deffer discussion of mount
  api logging improvements until later and with a wider audience.

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.
- 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.

---

Ian Kent (17):
      xfs: remove unused struct xfs_mount field m_fsname_len
      xfs: use super s_id instead of struct xfs_mount m_fsname
      xfs: dont use XFS_IS_QUOTA_RUNNING() for option check
      xfs: use kmem functions for struct xfs_mount
      xfs: merge freeing of mp names and mp
      xfs: add xfs_remount_rw() helper
      xfs: add xfs_remount_ro() helper
      xfs: refactor suffix_kstrtoint()
      xfs: avoid redundant checks when options is empty
      xfs: refactor xfs_parseags()
      xfs: move xfs_parseargs() validation to a helper
      xfs: dont set sb in xfs_mount_alloc()
      xfs: switch to use the new mount-api
      xfs: move xfs_fc_reconfigure() above xfs_fc_free()
      xfs: move xfs_fc_get_tree() above xfs_fc_reconfigure()
      xfs: move xfs_fc_parse_param() above xfs_fc_get_tree()
      xfs: fold xfs_mount-alloc() into xfs_init_fs_context()


 fs/xfs/xfs_error.c     |    2 
 fs/xfs/xfs_log.c       |    2 
 fs/xfs/xfs_message.c   |    4 
 fs/xfs/xfs_mount.c     |    5 
 fs/xfs/xfs_mount.h     |    2 
 fs/xfs/xfs_pnfs.c      |    2 
 fs/xfs/xfs_super.c     | 1227 ++++++++++++++++++++++++------------------------
 fs/xfs/xfs_trans_ail.c |    2 
 8 files changed, 611 insertions(+), 635 deletions(-)

--
Ian

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v9 01/17] xfs: remove unused struct xfs_mount field m_fsname_len
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
@ 2019-11-04 10:54 ` Ian Kent
  2019-11-04 10:54 ` [PATCH v9 02/17] xfs: use super s_id instead of struct xfs_mount m_fsname Ian Kent
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:54 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

The struct xfs_mount field m_fsname_len is not used anywhere, remove it.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_mount.h |    1 -
 fs/xfs/xfs_super.c |    1 -
 2 files changed, 2 deletions(-)

diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index a46cb3fd24b1..6e7d746b41bc 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -90,7 +90,6 @@ typedef struct xfs_mount {
 
 	struct xfs_buf		*m_sb_bp;	/* buffer for superblock */
 	char			*m_fsname;	/* filesystem name */
-	int			m_fsname_len;	/* strlen of fs name */
 	char			*m_rtname;	/* realtime device name */
 	char			*m_logname;	/* external log device name */
 	int			m_bsize;	/* fs logical block size */
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index bcb1575a5652..f3ecd696180d 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -168,7 +168,6 @@ xfs_parseargs(
 	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.


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 02/17] xfs: use super s_id instead of struct xfs_mount m_fsname
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
  2019-11-04 10:54 ` [PATCH v9 01/17] xfs: remove unused struct xfs_mount field m_fsname_len Ian Kent
@ 2019-11-04 10:54 ` Ian Kent
  2019-11-04 10:54 ` [PATCH v9 03/17] xfs: dont use XFS_IS_QUOTA_RUNNING() for option check Ian Kent
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:54 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

Eliminate struct xfs_mount field m_fsname by using the super block s_id
field directly.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_error.c     |    2 +-
 fs/xfs/xfs_log.c       |    2 +-
 fs/xfs/xfs_message.c   |    4 ++--
 fs/xfs/xfs_mount.c     |    5 +++--
 fs/xfs/xfs_mount.h     |    1 -
 fs/xfs/xfs_pnfs.c      |    2 +-
 fs/xfs/xfs_super.c     |   35 +++++++++++++----------------------
 fs/xfs/xfs_trans_ail.c |    2 +-
 8 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 849fd4476950..3ff3fc202522 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -257,7 +257,7 @@ xfs_errortag_test(
 
 	xfs_warn_ratelimited(mp,
 "Injecting error (%s) at file %s, line %d, on filesystem \"%s\"",
-			expression, file, line, mp->m_fsname);
+			expression, file, line, mp->m_super->s_id);
 	return true;
 }
 
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 33fb38864207..d7d3bfd6a920 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1526,7 +1526,7 @@ xlog_alloc_log(
 
 	log->l_ioend_workqueue = alloc_workqueue("xfs-log/%s",
 			WQ_MEM_RECLAIM | WQ_FREEZABLE | WQ_HIGHPRI, 0,
-			mp->m_fsname);
+			mp->m_super->s_id);
 	if (!log->l_ioend_workqueue)
 		goto out_free_iclog;
 
diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
index 9804efe525a9..584d0eb7cbd8 100644
--- a/fs/xfs/xfs_message.c
+++ b/fs/xfs/xfs_message.c
@@ -20,8 +20,8 @@ __xfs_printk(
 	const struct xfs_mount	*mp,
 	struct va_format	*vaf)
 {
-	if (mp && mp->m_fsname) {
-		printk("%sXFS (%s): %pV\n", level, mp->m_fsname, vaf);
+	if (mp && mp->m_super) {
+		printk("%sXFS (%s): %pV\n", level, mp->m_super->s_id, vaf);
 		return;
 	}
 	printk("%sXFS: %pV\n", level, vaf);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 3e8eedf01eb2..5ea95247a37f 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -667,7 +667,8 @@ xfs_mountfs(
 	/* enable fail_at_unmount as default */
 	mp->m_fail_unmount = true;
 
-	error = xfs_sysfs_init(&mp->m_kobj, &xfs_mp_ktype, NULL, mp->m_fsname);
+	error = xfs_sysfs_init(&mp->m_kobj, &xfs_mp_ktype,
+			       NULL, mp->m_super->s_id);
 	if (error)
 		goto out;
 
@@ -1241,7 +1242,7 @@ xfs_mod_fdblocks(
 	printk_once(KERN_WARNING
 		"Filesystem \"%s\": reserve blocks depleted! "
 		"Consider increasing reserve pool size.",
-		mp->m_fsname);
+		mp->m_super->s_id);
 fdblocks_enospc:
 	spin_unlock(&mp->m_sb_lock);
 	return -ENOSPC;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 6e7d746b41bc..0481e6d086a7 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -89,7 +89,6 @@ typedef struct xfs_mount {
 	struct percpu_counter	m_delalloc_blks;
 
 	struct xfs_buf		*m_sb_bp;	/* buffer for superblock */
-	char			*m_fsname;	/* filesystem name */
 	char			*m_rtname;	/* realtime device name */
 	char			*m_logname;	/* external log device name */
 	int			m_bsize;	/* fs logical block size */
diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index c637f0192976..5233929d9538 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -60,7 +60,7 @@ xfs_fs_get_uuid(
 
 	printk_once(KERN_NOTICE
 "XFS (%s): using experimental pNFS feature, use at your own risk!\n",
-		mp->m_fsname);
+		mp->m_super->s_id);
 
 	if (*len < sizeof(uuid_t))
 		return -EINVAL;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f3ecd696180d..6438738a204a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -161,14 +161,6 @@ xfs_parseargs(
 	substring_t		args[MAX_OPT_ARGS];
 	int			size = 0;
 
-	/*
-	 * 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;
-
 	/*
 	 * Copy binary VFS mount flags we are interested in.
 	 */
@@ -778,33 +770,33 @@ xfs_init_mount_workqueues(
 	struct xfs_mount	*mp)
 {
 	mp->m_buf_workqueue = alloc_workqueue("xfs-buf/%s",
-			WQ_MEM_RECLAIM|WQ_FREEZABLE, 1, mp->m_fsname);
+			WQ_MEM_RECLAIM|WQ_FREEZABLE, 1, mp->m_super->s_id);
 	if (!mp->m_buf_workqueue)
 		goto out;
 
 	mp->m_unwritten_workqueue = alloc_workqueue("xfs-conv/%s",
-			WQ_MEM_RECLAIM|WQ_FREEZABLE, 0, mp->m_fsname);
+			WQ_MEM_RECLAIM|WQ_FREEZABLE, 0, mp->m_super->s_id);
 	if (!mp->m_unwritten_workqueue)
 		goto out_destroy_buf;
 
 	mp->m_cil_workqueue = alloc_workqueue("xfs-cil/%s",
 			WQ_MEM_RECLAIM | WQ_FREEZABLE | WQ_UNBOUND,
-			0, mp->m_fsname);
+			0, mp->m_super->s_id);
 	if (!mp->m_cil_workqueue)
 		goto out_destroy_unwritten;
 
 	mp->m_reclaim_workqueue = alloc_workqueue("xfs-reclaim/%s",
-			WQ_MEM_RECLAIM|WQ_FREEZABLE, 0, mp->m_fsname);
+			WQ_MEM_RECLAIM|WQ_FREEZABLE, 0, mp->m_super->s_id);
 	if (!mp->m_reclaim_workqueue)
 		goto out_destroy_cil;
 
 	mp->m_eofblocks_workqueue = alloc_workqueue("xfs-eofblocks/%s",
-			WQ_MEM_RECLAIM|WQ_FREEZABLE, 0, mp->m_fsname);
+			WQ_MEM_RECLAIM|WQ_FREEZABLE, 0, mp->m_super->s_id);
 	if (!mp->m_eofblocks_workqueue)
 		goto out_destroy_reclaim;
 
 	mp->m_sync_workqueue = alloc_workqueue("xfs-sync/%s", WQ_FREEZABLE, 0,
-					       mp->m_fsname);
+					       mp->m_super->s_id);
 	if (!mp->m_sync_workqueue)
 		goto out_destroy_eofb;
 
@@ -1009,10 +1001,9 @@ xfs_fs_drop_inode(
 }
 
 STATIC void
-xfs_free_fsname(
+xfs_free_names(
 	struct xfs_mount	*mp)
 {
-	kfree(mp->m_fsname);
 	kfree(mp->m_rtname);
 	kfree(mp->m_logname);
 }
@@ -1189,7 +1180,7 @@ xfs_test_remount_options(
 
 	tmp_mp->m_super = sb;
 	error = xfs_parseargs(tmp_mp, options);
-	xfs_free_fsname(tmp_mp);
+	xfs_free_names(tmp_mp);
 	kmem_free(tmp_mp);
 
 	return error;
@@ -1555,7 +1546,7 @@ xfs_fs_fill_super(
 
 	error = xfs_parseargs(mp, (char *)data);
 	if (error)
-		goto out_free_fsname;
+		goto out_free_names;
 
 	sb_min_blocksize(sb, BBSIZE);
 	sb->s_xattr = xfs_xattr_handlers;
@@ -1582,7 +1573,7 @@ xfs_fs_fill_super(
 
 	error = xfs_open_devices(mp);
 	if (error)
-		goto out_free_fsname;
+		goto out_free_names;
 
 	error = xfs_init_mount_workqueues(mp);
 	if (error)
@@ -1719,9 +1710,9 @@ xfs_fs_fill_super(
 	xfs_destroy_mount_workqueues(mp);
  out_close_devices:
 	xfs_close_devices(mp);
- out_free_fsname:
+ out_free_names:
 	sb->s_fs_info = NULL;
-	xfs_free_fsname(mp);
+	xfs_free_names(mp);
 	kfree(mp);
  out:
 	return error;
@@ -1753,7 +1744,7 @@ xfs_fs_put_super(
 	xfs_close_devices(mp);
 
 	sb->s_fs_info = NULL;
-	xfs_free_fsname(mp);
+	xfs_free_names(mp);
 	kfree(mp);
 }
 
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 6ccfd75d3c24..aea71ee189f5 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -836,7 +836,7 @@ xfs_trans_ail_init(
 	init_waitqueue_head(&ailp->ail_empty);
 
 	ailp->ail_task = kthread_run(xfsaild, ailp, "xfsaild/%s",
-			ailp->ail_mount->m_fsname);
+			ailp->ail_mount->m_super->s_id);
 	if (IS_ERR(ailp->ail_task))
 		goto out_free_ailp;
 


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 03/17] xfs: dont use XFS_IS_QUOTA_RUNNING() for option check
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
  2019-11-04 10:54 ` [PATCH v9 01/17] xfs: remove unused struct xfs_mount field m_fsname_len Ian Kent
  2019-11-04 10:54 ` [PATCH v9 02/17] xfs: use super s_id instead of struct xfs_mount m_fsname Ian Kent
@ 2019-11-04 10:54 ` Ian Kent
  2019-11-04 10:54 ` [PATCH v9 04/17] xfs: use kmem functions for struct xfs_mount Ian Kent
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:54 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

When CONFIG_XFS_QUOTA is not defined any quota option is invalid.

Using the macro XFS_IS_QUOTA_RUNNING() as a check if any quota option
has been given is a little misleading so use a simple m_qflags != 0
check to make the intended use more explicit.

Also change to use the IS_ENABLED() macro for the kernel config check.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.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 6438738a204a..fb90beeb3184 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -341,12 +341,10 @@ xfs_parseargs(
 		return -EINVAL;
 	}
 
-#ifndef CONFIG_XFS_QUOTA
-	if (XFS_IS_QUOTA_RUNNING(mp)) {
+	if (!IS_ENABLED(CONFIG_XFS_QUOTA) && mp->m_qflags != 0) {
 		xfs_warn(mp, "quota support not available in this kernel.");
 		return -EINVAL;
 	}
-#endif
 
 	if ((mp->m_dalign && !mp->m_swidth) ||
 	    (!mp->m_dalign && mp->m_swidth)) {


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 04/17] xfs: use kmem functions for struct xfs_mount
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (2 preceding siblings ...)
  2019-11-04 10:54 ` [PATCH v9 03/17] xfs: dont use XFS_IS_QUOTA_RUNNING() for option check Ian Kent
@ 2019-11-04 10:54 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 05/17] xfs: merge freeing of mp names and mp Ian Kent
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:54 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

The remount function uses the kmem functions for allocating and freeing
struct xfs_mount, for consistency use the kmem functions everwhere for
struct xfs_mount.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index fb90beeb3184..eb919e74d8eb 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1497,7 +1497,7 @@ xfs_mount_alloc(
 {
 	struct xfs_mount	*mp;
 
-	mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL);
+	mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
 	if (!mp)
 		return NULL;
 
@@ -1711,7 +1711,7 @@ xfs_fs_fill_super(
  out_free_names:
 	sb->s_fs_info = NULL;
 	xfs_free_names(mp);
-	kfree(mp);
+	kmem_free(mp);
  out:
 	return error;
 
@@ -1743,7 +1743,7 @@ xfs_fs_put_super(
 
 	sb->s_fs_info = NULL;
 	xfs_free_names(mp);
-	kfree(mp);
+	kmem_free(mp);
 }
 
 STATIC struct dentry *


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 05/17] xfs: merge freeing of mp names and mp
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (3 preceding siblings ...)
  2019-11-04 10:54 ` [PATCH v9 04/17] xfs: use kmem functions for struct xfs_mount Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 06/17] xfs: add xfs_remount_rw() helper Ian Kent
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

In all cases when struct xfs_mount (mp) fields m_rtname and m_logname
are freed mp is also freed, so merge these into a single function
xfs_mount_free()

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index eb919e74d8eb..6d908b76aa9e 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -998,12 +998,13 @@ xfs_fs_drop_inode(
 	return generic_drop_inode(inode) || (ip->i_flags & XFS_IDONTCACHE);
 }
 
-STATIC void
-xfs_free_names(
+static void
+xfs_mount_free(
 	struct xfs_mount	*mp)
 {
 	kfree(mp->m_rtname);
 	kfree(mp->m_logname);
+	kmem_free(mp);
 }
 
 STATIC int
@@ -1178,8 +1179,7 @@ xfs_test_remount_options(
 
 	tmp_mp->m_super = sb;
 	error = xfs_parseargs(tmp_mp, options);
-	xfs_free_names(tmp_mp);
-	kmem_free(tmp_mp);
+	xfs_mount_free(tmp_mp);
 
 	return error;
 }
@@ -1710,8 +1710,7 @@ xfs_fs_fill_super(
 	xfs_close_devices(mp);
  out_free_names:
 	sb->s_fs_info = NULL;
-	xfs_free_names(mp);
-	kmem_free(mp);
+	xfs_mount_free(mp);
  out:
 	return error;
 
@@ -1742,8 +1741,7 @@ xfs_fs_put_super(
 	xfs_close_devices(mp);
 
 	sb->s_fs_info = NULL;
-	xfs_free_names(mp);
-	kmem_free(mp);
+	xfs_mount_free(mp);
 }
 
 STATIC struct dentry *


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 06/17] xfs: add xfs_remount_rw() helper
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (4 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 05/17] xfs: merge freeing of mp names and mp Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 07/17] xfs: add xfs_remount_ro() helper Ian Kent
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

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>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 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 6d908b76aa9e..6eaa1b05897a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1184,6 +1184,68 @@ xfs_test_remount_options(
 	return error;
 }
 
+static int
+xfs_remount_rw(
+	struct xfs_mount	*mp)
+{
+	struct xfs_sb		*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,
@@ -1247,57 +1309,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 v9 07/17] xfs: add xfs_remount_ro() helper
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (5 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 06/17] xfs: add xfs_remount_rw() helper Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 08/17] xfs: refactor suffix_kstrtoint() Ian Kent
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

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>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 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 6eaa1b05897a..bdf6c069e3ea 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1246,6 +1246,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,
@@ -1316,37 +1357,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 v9 08/17] xfs: refactor suffix_kstrtoint()
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (6 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 07/17] xfs: add xfs_remount_ro() helper Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 09/17] xfs: avoid redundant checks when options is empty Ian Kent
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

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.

When xfs is switched to use the new mount-api match_kstrtoint() will no
longer be used and will be removed.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |   38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index bdf6c069e3ea..0dc072700599 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -108,14 +108,17 @@ static const match_table_t tokens = {
 };
 
 
-STATIC int
-suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)
+static int
+suffix_kstrtoint(
+	const char	*s,
+	unsigned int	base,
+	int		*res)
 {
-	int	last, shift_left_factor = 0, _res;
-	char	*value;
-	int	ret = 0;
+	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;
 
@@ -140,6 +143,23 @@ 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.
@@ -151,7 +171,7 @@ suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)
  * 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
+static int
 xfs_parseargs(
 	struct xfs_mount	*mp,
 	char			*options)
@@ -194,7 +214,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:
@@ -210,7 +230,7 @@ xfs_parseargs(
 				return -ENOMEM;
 			break;
 		case Opt_allocsize:
-			if (suffix_kstrtoint(args, 10, &size))
+			if (match_kstrtoint(args, 10, &size))
 				return -EINVAL;
 			mp->m_allocsize_log = ffs(size) - 1;
 			mp->m_flags |= XFS_MOUNT_ALLOCSIZE;


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 09/17] xfs: avoid redundant checks when options is empty
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (7 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 08/17] xfs: refactor suffix_kstrtoint() Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 10/17] xfs: refactor xfs_parseags() Ian Kent
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

When options passed to xfs_parseargs() is NULL the checks performed
after taking the branch are made with the initial values of dsunit,
dswidth and iosizelog. But all the checks do nothing in this case
so return immediately instead.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0dc072700599..17188a9ed541 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -199,7 +199,7 @@ xfs_parseargs(
 	mp->m_allocsize_log = 16; /* 64k */
 
 	if (!options)
-		goto done;
+		return 0;
 
 	while ((p = strsep(&options, ",")) != NULL) {
 		int		token;
@@ -379,7 +379,6 @@ xfs_parseargs(
 		return -EINVAL;
 	}
 
-done:
 	if (mp->m_logbufs != -1 &&
 	    mp->m_logbufs != 0 &&
 	    (mp->m_logbufs < XLOG_MIN_ICLOGS ||


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 10/17] xfs: refactor xfs_parseags()
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (8 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 09/17] xfs: avoid redundant checks when options is empty Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 11/17] xfs: move xfs_parseargs() validation to a helper Ian Kent
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

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.

Also change the break in the switch to a return in the factored out
xfs_fc_parse_param() function.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |  288 +++++++++++++++++++++++++++-------------------------
 1 file changed, 152 insertions(+), 136 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 17188a9ed541..391c07ca6a32 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -160,6 +160,154 @@ match_kstrtoint(
 	return ret;
 }
 
+static int
+xfs_fc_parse_param(
+	int			token,
+	char			*p,
+	substring_t		*args,
+	struct xfs_mount	*mp)
+{
+	int			size = 0;
+
+	switch (token) {
+	case Opt_logbufs:
+		if (match_int(args, &mp->m_logbufs))
+			return -EINVAL;
+		return 0;
+	case Opt_logbsize:
+		if (match_kstrtoint(args, 10, &mp->m_logbsize))
+			return -EINVAL;
+		return 0;
+	case Opt_logdev:
+		kfree(mp->m_logname);
+		mp->m_logname = match_strdup(args);
+		if (!mp->m_logname)
+			return -ENOMEM;
+		return 0;
+	case Opt_rtdev:
+		kfree(mp->m_rtname);
+		mp->m_rtname = match_strdup(args);
+		if (!mp->m_rtname)
+			return -ENOMEM;
+		return 0;
+	case Opt_allocsize:
+		if (match_kstrtoint(args, 10, &size))
+			return -EINVAL;
+		mp->m_allocsize_log = ffs(size) - 1;
+		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
+		return 0;
+	case Opt_grpid:
+	case Opt_bsdgroups:
+		mp->m_flags |= XFS_MOUNT_GRPID;
+		return 0;
+	case Opt_nogrpid:
+	case Opt_sysvgroups:
+		mp->m_flags &= ~XFS_MOUNT_GRPID;
+		return 0;
+	case Opt_wsync:
+		mp->m_flags |= XFS_MOUNT_WSYNC;
+		return 0;
+	case Opt_norecovery:
+		mp->m_flags |= XFS_MOUNT_NORECOVERY;
+		return 0;
+	case Opt_noalign:
+		mp->m_flags |= XFS_MOUNT_NOALIGN;
+		return 0;
+	case Opt_swalloc:
+		mp->m_flags |= XFS_MOUNT_SWALLOC;
+		return 0;
+	case Opt_sunit:
+		if (match_int(args, &mp->m_dalign))
+			return -EINVAL;
+		return 0;
+	case Opt_swidth:
+		if (match_int(args, &mp->m_swidth))
+			return -EINVAL;
+		return 0;
+	case Opt_inode32:
+		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+		return 0;
+	case Opt_inode64:
+		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
+		return 0;
+	case Opt_nouuid:
+		mp->m_flags |= XFS_MOUNT_NOUUID;
+		return 0;
+	case Opt_ikeep:
+		mp->m_flags |= XFS_MOUNT_IKEEP;
+		return 0;
+	case Opt_noikeep:
+		mp->m_flags &= ~XFS_MOUNT_IKEEP;
+		return 0;
+	case Opt_largeio:
+		mp->m_flags |= XFS_MOUNT_LARGEIO;
+		return 0;
+	case Opt_nolargeio:
+		mp->m_flags &= ~XFS_MOUNT_LARGEIO;
+		return 0;
+	case Opt_attr2:
+		mp->m_flags |= XFS_MOUNT_ATTR2;
+		return 0;
+	case Opt_noattr2:
+		mp->m_flags &= ~XFS_MOUNT_ATTR2;
+		mp->m_flags |= XFS_MOUNT_NOATTR2;
+		return 0;
+	case Opt_filestreams:
+		mp->m_flags |= XFS_MOUNT_FILESTREAMS;
+		return 0;
+	case Opt_noquota:
+		mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
+		mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
+		mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
+		return 0;
+	case Opt_quota:
+	case Opt_uquota:
+	case Opt_usrquota:
+		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
+				 XFS_UQUOTA_ENFD);
+		return 0;
+	case Opt_qnoenforce:
+	case Opt_uqnoenforce:
+		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
+		mp->m_qflags &= ~XFS_UQUOTA_ENFD;
+		return 0;
+	case Opt_pquota:
+	case Opt_prjquota:
+		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
+				 XFS_PQUOTA_ENFD);
+		return 0;
+	case Opt_pqnoenforce:
+		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
+		mp->m_qflags &= ~XFS_PQUOTA_ENFD;
+		return 0;
+	case Opt_gquota:
+	case Opt_grpquota:
+		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
+				 XFS_GQUOTA_ENFD);
+		return 0;
+	case Opt_gqnoenforce:
+		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
+		mp->m_qflags &= ~XFS_GQUOTA_ENFD;
+		return 0;
+	case Opt_discard:
+		mp->m_flags |= XFS_MOUNT_DISCARD;
+		return 0;
+	case Opt_nodiscard:
+		mp->m_flags &= ~XFS_MOUNT_DISCARD;
+		return 0;
+#ifdef CONFIG_FS_DAX
+	case Opt_dax:
+		mp->m_flags |= XFS_MOUNT_DAX;
+		return 0;
+#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.
@@ -179,7 +327,6 @@ xfs_parseargs(
 	const struct super_block *sb = mp->m_super;
 	char			*p;
 	substring_t		args[MAX_OPT_ARGS];
-	int			size = 0;
 
 	/*
 	 * Copy binary VFS mount flags we are interested in.
@@ -203,146 +350,15 @@ 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, &size))
-				return -EINVAL;
-			mp->m_allocsize_log = ffs(size) - 1;
-			mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
-			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, &mp->m_dalign))
-				return -EINVAL;
-			break;
-		case Opt_swidth:
-			if (match_int(args, &mp->m_swidth))
-				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_LARGEIO;
-			break;
-		case Opt_nolargeio:
-			mp->m_flags &= ~XFS_MOUNT_LARGEIO;
-			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_fc_parse_param(token, p, args, mp);
+		if (ret)
+			return ret;
 	}
 
 	/*


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 11/17] xfs: move xfs_parseargs() validation to a helper
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (9 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 10/17] xfs: refactor xfs_parseags() Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 12/17] xfs: dont set sb in xfs_mount_alloc() Ian Kent
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

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>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_super.c |  109 ++++++++++++++++++++++++++++------------------------
 1 file changed, 58 insertions(+), 51 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 391c07ca6a32..4b570ba3456a 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -308,59 +308,10 @@ xfs_fc_parse_param(
 	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)
+xfs_fc_validate_params(
+	struct xfs_mount	*mp)
 {
-	const struct super_block *sb = mp->m_super;
-	char			*p;
-	substring_t		args[MAX_OPT_ARGS];
-
-	/*
-	 * 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;
-
-	/*
-	 * These can be overridden by the mount option parsing.
-	 */
-	mp->m_logbufs = -1;
-	mp->m_logbsize = -1;
-	mp->m_allocsize_log = 16; /* 64k */
-
-	if (!options)
-		return 0;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		int		token;
-		int		ret;
-
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		ret = xfs_fc_parse_param(token, p, args, mp);
-		if (ret)
-			return ret;
-	}
-
 	/*
 	 * no recovery flag requires a read-only mount
 	 */
@@ -425,6 +376,62 @@ xfs_parseargs(
 	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;
+	substring_t		args[MAX_OPT_ARGS];
+
+	/*
+	 * 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;
+
+	/*
+	 * These can be overridden by the mount option parsing.
+	 */
+	mp->m_logbufs = -1;
+	mp->m_logbsize = -1;
+	mp->m_allocsize_log = 16; /* 64k */
+
+	if (!options)
+		return 0;
+
+	while ((p = strsep(&options, ",")) != NULL) {
+		int		token;
+		int		ret;
+
+		if (!*p)
+			continue;
+
+		token = match_token(p, tokens, args);
+		ret = xfs_fc_parse_param(token, p, args, mp);
+		if (ret)
+			return ret;
+	}
+
+	return xfs_fc_validate_params(mp);
+}
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 12/17] xfs: dont set sb in xfs_mount_alloc()
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (10 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 11/17] xfs: move xfs_parseargs() validation to a helper Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 13/17] xfs: switch to use the new mount-api Ian Kent
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

When changing to use the new mount api the super block won't be
available when the xfs_mount struct is allocated so move setting the
super block in xfs_mount to xfs_fs_fill_super().

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_super.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 4b570ba3456a..62dfc678c415 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1560,8 +1560,7 @@ xfs_destroy_percpu_counters(
 }
 
 static struct xfs_mount *
-xfs_mount_alloc(
-	struct super_block	*sb)
+xfs_mount_alloc(void)
 {
 	struct xfs_mount	*mp;
 
@@ -1569,7 +1568,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);
@@ -1605,9 +1603,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)
 		goto out;
+	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 v9 13/17] xfs: switch to use the new mount-api
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (11 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 12/17] xfs: dont set sb in xfs_mount_alloc() Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 15:14   ` Christoph Hellwig
  2019-11-04 22:53   ` Darrick J. Wong
  2019-11-04 10:55 ` [PATCH v9 14/17] xfs: move xfs_fc_reconfigure() above xfs_fc_free() Ian Kent
                   ` (3 subsequent siblings)
  16 siblings, 2 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

Define the struct fs_parameter_spec table that's used by the new
mount-api for options parsing.

Create the various fs context operations methods and define the
fs_context_operations struct.

Create the fs context initialization method and update the struct
file_system_type to utilize it. The initialization function is
responsible for working storage initialization, allocation and
initialization of file system private information storage and for
setting the operations in the fs context.

Also set struct file_system_type .parameters to the newly defined
struct fs_parameter_spec options parsing table for use by the fs
context methods and remove unused code.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/xfs/xfs_super.c |  408 ++++++++++++++++++++++------------------------------
 1 file changed, 171 insertions(+), 237 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 62dfc678c415..2d4edaaf934e 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -37,7 +37,8 @@
 #include "xfs_reflink.h"
 
 #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;
 
@@ -58,55 +59,57 @@ 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,
+	Opt_discard, Opt_nodiscard, Opt_dax,
 };
 
-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},
+static const struct fs_parameter_spec xfs_param_specs[] = {
+	fsparam_u32("logbufs",		Opt_logbufs),
+	fsparam_string("logbsize",	Opt_logbsize),
+	fsparam_string("logdev",	Opt_logdev),
+	fsparam_string("rtdev",		Opt_rtdev),
+	fsparam_flag("wsync",		Opt_wsync),
+	fsparam_flag("noalign",		Opt_noalign),
+	fsparam_flag("swalloc",		Opt_swalloc),
+	fsparam_u32("sunit",		Opt_sunit),
+	fsparam_u32("swidth",		Opt_swidth),
+	fsparam_flag("nouuid",		Opt_nouuid),
+	fsparam_flag("grpid",		Opt_grpid),
+	fsparam_flag("nogrpid",		Opt_nogrpid),
+	fsparam_flag("bsdgroups",	Opt_bsdgroups),
+	fsparam_flag("sysvgroups",	Opt_sysvgroups),
+	fsparam_string("allocsize",	Opt_allocsize),
+	fsparam_flag("norecovery",	Opt_norecovery),
+	fsparam_flag("inode64",		Opt_inode64),
+	fsparam_flag("inode32",		Opt_inode32),
+	fsparam_flag("ikeep",		Opt_ikeep),
+	fsparam_flag("noikeep",		Opt_noikeep),
+	fsparam_flag("largeio",		Opt_largeio),
+	fsparam_flag("nolargeio",	Opt_nolargeio),
+	fsparam_flag("attr2",		Opt_attr2),
+	fsparam_flag("noattr2",		Opt_noattr2),
+	fsparam_flag("filestreams",	Opt_filestreams),
+	fsparam_flag("quota",		Opt_quota),
+	fsparam_flag("noquota",		Opt_noquota),
+	fsparam_flag("usrquota",	Opt_usrquota),
+	fsparam_flag("grpquota",	Opt_grpquota),
+	fsparam_flag("prjquota",	Opt_prjquota),
+	fsparam_flag("uquota",		Opt_uquota),
+	fsparam_flag("gquota",		Opt_gquota),
+	fsparam_flag("pquota",		Opt_pquota),
+	fsparam_flag("uqnoenforce",	Opt_uqnoenforce),
+	fsparam_flag("gqnoenforce",	Opt_gqnoenforce),
+	fsparam_flag("pqnoenforce",	Opt_pqnoenforce),
+	fsparam_flag("qnoenforce",	Opt_qnoenforce),
+	fsparam_flag("discard",		Opt_discard),
+	fsparam_flag("nodiscard",	Opt_nodiscard),
+	fsparam_flag("dax",		Opt_dax),
+	{}
 };
 
+static const struct fs_parameter_description xfs_fs_parameters = {
+	.name		= "xfs",
+	.specs		= xfs_param_specs,
+};
 
 static int
 suffix_kstrtoint(
@@ -143,55 +146,42 @@ suffix_kstrtoint(
 	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;
-}
-
 static int
 xfs_fc_parse_param(
-	int			token,
-	char			*p,
-	substring_t		*args,
-	struct xfs_mount	*mp)
+	struct fs_context	*fc,
+	struct fs_parameter	*param)
 {
+	struct xfs_mount	*mp = fc->s_fs_info;
+	struct fs_parse_result	result;
 	int			size = 0;
+	int			opt;
+
+	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
 
-	switch (token) {
+	switch (opt) {
 	case Opt_logbufs:
-		if (match_int(args, &mp->m_logbufs))
-			return -EINVAL;
+		mp->m_logbufs = result.uint_32;
 		return 0;
 	case Opt_logbsize:
-		if (match_kstrtoint(args, 10, &mp->m_logbsize))
+		if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
 			return -EINVAL;
 		return 0;
 	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;
 		return 0;
 	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;
 		return 0;
 	case Opt_allocsize:
-		if (match_kstrtoint(args, 10, &size))
+		if (suffix_kstrtoint(param->string, 10, &size))
 			return -EINVAL;
 		mp->m_allocsize_log = ffs(size) - 1;
 		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
@@ -217,12 +207,10 @@ xfs_fc_parse_param(
 		mp->m_flags |= XFS_MOUNT_SWALLOC;
 		return 0;
 	case Opt_sunit:
-		if (match_int(args, &mp->m_dalign))
-			return -EINVAL;
+		mp->m_dalign = result.uint_32;
 		return 0;
 	case Opt_swidth:
-		if (match_int(args, &mp->m_swidth))
-			return -EINVAL;
+		mp->m_swidth = result.uint_32;
 		return 0;
 	case Opt_inode32:
 		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
@@ -301,7 +289,7 @@ xfs_fc_parse_param(
 		return 0;
 #endif
 	default:
-		xfs_warn(mp, "unknown mount option [%s].", p);
+		xfs_warn(mp, "unknown mount option [%s].", param->key);
 		return -EINVAL;
 	}
 
@@ -376,62 +364,6 @@ xfs_fc_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;
-	substring_t		args[MAX_OPT_ARGS];
-
-	/*
-	 * 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;
-
-	/*
-	 * These can be overridden by the mount option parsing.
-	 */
-	mp->m_logbufs = -1;
-	mp->m_logbsize = -1;
-	mp->m_allocsize_log = 16; /* 64k */
-
-	if (!options)
-		return 0;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		int		token;
-		int		ret;
-
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		ret = xfs_fc_parse_param(token, p, args, mp);
-		if (ret)
-			return ret;
-	}
-
-	return xfs_fc_validate_params(mp);
-}
-
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -1207,25 +1139,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_mount_free(tmp_mp);
-
-	return error;
-}
-
 static int
 xfs_remount_rw(
 	struct xfs_mount	*mp)
@@ -1329,76 +1242,57 @@ xfs_remount_ro(
 	return 0;
 }
 
-STATIC int
-xfs_fs_remount(
-	struct super_block	*sb,
-	int			*flags,
-	char			*options)
+/*
+ * 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.
+ */
+static int
+xfs_fc_reconfigure(
+	struct fs_context	*fc)
 {
-	struct xfs_mount	*mp = XFS_M(sb);
+	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;
-	substring_t		args[MAX_OPT_ARGS];
-	char			*p;
+	int			flags = fc->sb_flags;
 	int			error;
 
-	/* First, check for complete junk; i.e. invalid options */
-	error = xfs_test_remount_options(sb, options);
+	error = xfs_fc_validate_params(new_mp);
 	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
-		}
+	sync_filesystem(mp->m_super);
+
+	/* 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)) {
+	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)) {
+	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
 		error = xfs_remount_ro(mp);
 		if (error)
 			return error;
@@ -1588,28 +1482,18 @@ xfs_mount_alloc(void)
 	return mp;
 }
 
-
-STATIC int
-xfs_fs_fill_super(
+static int
+xfs_fc_fill_super(
 	struct super_block	*sb,
-	void			*data,
-	int			silent)
+	struct fs_context	*fc)
 {
+	struct xfs_mount	*mp = sb->s_fs_info;
 	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();
-	if (!mp)
-		goto out;
 	mp->m_super = sb;
-	sb->s_fs_info = mp;
 
-	error = xfs_parseargs(mp, (char *)data);
+	error = xfs_fc_validate_params(mp);
 	if (error)
 		goto out_free_names;
 
@@ -1633,7 +1517,7 @@ xfs_fs_fill_super(
 		msleep(xfs_globals.mount_delay * 1000);
 	}
 
-	if (silent)
+	if (fc->sb_flags & SB_SILENT)
 		flags |= XFS_MFSI_QUIET;
 
 	error = xfs_open_devices(mp);
@@ -1778,7 +1662,6 @@ xfs_fs_fill_super(
  out_free_names:
 	sb->s_fs_info = NULL;
 	xfs_mount_free(mp);
- out:
 	return error;
 
  out_unmount:
@@ -1787,6 +1670,13 @@ xfs_fs_fill_super(
 	goto out_free_sb;
 }
 
+static int
+xfs_fc_get_tree(
+	struct fs_context	*fc)
+{
+	return get_tree_bdev(fc, xfs_fc_fill_super);
+}
+
 STATIC void
 xfs_fs_put_super(
 	struct super_block	*sb)
@@ -1811,16 +1701,6 @@ xfs_fs_put_super(
 	xfs_mount_free(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,
@@ -1850,16 +1730,70 @@ 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,
 };
 
+static void xfs_fc_free(
+	struct fs_context	*fc)
+{
+	struct xfs_mount	*mp = fc->s_fs_info;
+
+	/*
+	 * mp is stored in the fs_context when it is initialized.
+	 * mp is transferred to the superblock on a successful mount,
+	 * but if an error occurs before the transfer we have to free
+	 * it here.
+	 */
+	if (mp)
+		xfs_mount_free(mp);
+}
+
+static const struct fs_context_operations xfs_context_ops = {
+	.parse_param = xfs_fc_parse_param,
+	.get_tree    = xfs_fc_get_tree,
+	.reconfigure = xfs_fc_reconfigure,
+	.free        = xfs_fc_free,
+};
+
+static int xfs_init_fs_context(
+	struct fs_context	*fc)
+{
+	struct xfs_mount	*mp;
+
+	mp = xfs_mount_alloc();
+	if (!mp)
+		return -ENOMEM;
+
+	/*
+	 * These can be overridden by the mount option parsing.
+	 */
+	mp->m_logbufs = -1;
+	mp->m_logbsize = -1;
+	mp->m_allocsize_log = 16; /* 64k */
+
+	/*
+	 * 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->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 v9 14/17] xfs: move xfs_fc_reconfigure() above xfs_fc_free()
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (12 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 13/17] xfs: switch to use the new mount-api Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:55 ` [PATCH v9 15/17] xfs: move xfs_fc_get_tree() above xfs_fc_reconfigure() Ian Kent
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

Grouping the options parsing and mount handling functions above the
struct fs_context_operations but below the struct super_operations
should improve (some) the grouping of the super operations while also
improving the grouping of the options parsing and mount handling code.

Start by moving xfs_fc_reconfigure() and friends.
This is a straight code move, there aren't any functional changes.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_super.c |  324 ++++++++++++++++++++++++++--------------------------
 1 file changed, 162 insertions(+), 162 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 2d4edaaf934e..2746051da8c1 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1139,168 +1139,6 @@ xfs_quiesce_attr(
 	xfs_log_quiesce(mp);
 }
 
-static int
-xfs_remount_rw(
-	struct xfs_mount	*mp)
-{
-	struct xfs_sb		*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_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;
-}
-
-/*
- * 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.
- */
-static int
-xfs_fc_reconfigure(
-	struct fs_context	*fc)
-{
-	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_fc_validate_params(new_mp);
-	if (error)
-		return error;
-
-	sync_filesystem(mp->m_super);
-
-	/* 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
@@ -1735,6 +1573,168 @@ static const struct super_operations xfs_super_operations = {
 	.free_cached_objects	= xfs_fs_free_cached_objects,
 };
 
+static int
+xfs_remount_rw(
+	struct xfs_mount	*mp)
+{
+	struct xfs_sb		*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_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;
+}
+
+/*
+ * 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.
+ */
+static int
+xfs_fc_reconfigure(
+	struct fs_context *fc)
+{
+	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_fc_validate_params(new_mp);
+	if (error)
+		return error;
+
+	sync_filesystem(mp->m_super);
+
+	/* 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;
+}
+
 static void xfs_fc_free(
 	struct fs_context	*fc)
 {


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 15/17] xfs: move xfs_fc_get_tree() above xfs_fc_reconfigure()
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (13 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 14/17] xfs: move xfs_fc_reconfigure() above xfs_fc_free() Ian Kent
@ 2019-11-04 10:55 ` Ian Kent
  2019-11-04 10:56 ` [PATCH v9 16/17] xfs: move xfs_fc_parse_param() above xfs_fc_get_tree() Ian Kent
  2019-11-04 10:56 ` [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context() Ian Kent
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:55 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

Grouping the options parsing and mount handling functions above the
struct fs_context_operations but below the struct super_operations
should improve (some) the grouping of the super operations while also
improving the grouping of the options parsing and mount handling code.

Now move xfs_fc_get_tree() and friends, also take the oppertunity to
change STATIC to static for the xfs_fs_put_super() function.
This is a straight code move, there aren't any functional changes.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_super.c |  116 ++++++++++++++++++++++++++--------------------------
 1 file changed, 58 insertions(+), 58 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 2746051da8c1..a0233fc9f569 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1291,6 +1291,64 @@ xfs_destroy_percpu_counters(
 	percpu_counter_destroy(&mp->m_delalloc_blks);
 }
 
+static void
+xfs_fs_put_super(
+	struct super_block	*sb)
+{
+	struct xfs_mount	*mp = XFS_M(sb);
+
+	/* if ->fill_super failed, we have no mount to tear down */
+	if (!sb->s_fs_info)
+		return;
+
+	xfs_notice(mp, "Unmounting Filesystem");
+	xfs_filestream_unmount(mp);
+	xfs_unmountfs(mp);
+
+	xfs_freesb(mp);
+	free_percpu(mp->m_stats.xs_stats);
+	xfs_destroy_percpu_counters(mp);
+	xfs_destroy_mount_workqueues(mp);
+	xfs_close_devices(mp);
+
+	sb->s_fs_info = NULL;
+	xfs_mount_free(mp);
+}
+
+static long
+xfs_fs_nr_cached_objects(
+	struct super_block	*sb,
+	struct shrink_control	*sc)
+{
+	/* Paranoia: catch incorrect calls during mount setup or teardown */
+	if (WARN_ON_ONCE(!sb->s_fs_info))
+		return 0;
+	return xfs_reclaim_inodes_count(XFS_M(sb));
+}
+
+static long
+xfs_fs_free_cached_objects(
+	struct super_block	*sb,
+	struct shrink_control	*sc)
+{
+	return xfs_reclaim_inodes_nr(XFS_M(sb), sc->nr_to_scan);
+}
+
+static const struct super_operations xfs_super_operations = {
+	.alloc_inode		= xfs_fs_alloc_inode,
+	.destroy_inode		= xfs_fs_destroy_inode,
+	.dirty_inode		= xfs_fs_dirty_inode,
+	.drop_inode		= xfs_fs_drop_inode,
+	.put_super		= xfs_fs_put_super,
+	.sync_fs		= xfs_fs_sync_fs,
+	.freeze_fs		= xfs_fs_freeze,
+	.unfreeze_fs		= xfs_fs_unfreeze,
+	.statfs			= xfs_fs_statfs,
+	.show_options		= xfs_fs_show_options,
+	.nr_cached_objects	= xfs_fs_nr_cached_objects,
+	.free_cached_objects	= xfs_fs_free_cached_objects,
+};
+
 static struct xfs_mount *
 xfs_mount_alloc(void)
 {
@@ -1515,64 +1573,6 @@ xfs_fc_get_tree(
 	return get_tree_bdev(fc, xfs_fc_fill_super);
 }
 
-STATIC void
-xfs_fs_put_super(
-	struct super_block	*sb)
-{
-	struct xfs_mount	*mp = XFS_M(sb);
-
-	/* if ->fill_super failed, we have no mount to tear down */
-	if (!sb->s_fs_info)
-		return;
-
-	xfs_notice(mp, "Unmounting Filesystem");
-	xfs_filestream_unmount(mp);
-	xfs_unmountfs(mp);
-
-	xfs_freesb(mp);
-	free_percpu(mp->m_stats.xs_stats);
-	xfs_destroy_percpu_counters(mp);
-	xfs_destroy_mount_workqueues(mp);
-	xfs_close_devices(mp);
-
-	sb->s_fs_info = NULL;
-	xfs_mount_free(mp);
-}
-
-static long
-xfs_fs_nr_cached_objects(
-	struct super_block	*sb,
-	struct shrink_control	*sc)
-{
-	/* Paranoia: catch incorrect calls during mount setup or teardown */
-	if (WARN_ON_ONCE(!sb->s_fs_info))
-		return 0;
-	return xfs_reclaim_inodes_count(XFS_M(sb));
-}
-
-static long
-xfs_fs_free_cached_objects(
-	struct super_block	*sb,
-	struct shrink_control	*sc)
-{
-	return xfs_reclaim_inodes_nr(XFS_M(sb), sc->nr_to_scan);
-}
-
-static const struct super_operations xfs_super_operations = {
-	.alloc_inode		= xfs_fs_alloc_inode,
-	.destroy_inode		= xfs_fs_destroy_inode,
-	.dirty_inode		= xfs_fs_dirty_inode,
-	.drop_inode		= xfs_fs_drop_inode,
-	.put_super		= xfs_fs_put_super,
-	.sync_fs		= xfs_fs_sync_fs,
-	.freeze_fs		= xfs_fs_freeze,
-	.unfreeze_fs		= xfs_fs_unfreeze,
-	.statfs			= xfs_fs_statfs,
-	.show_options		= xfs_fs_show_options,
-	.nr_cached_objects	= xfs_fs_nr_cached_objects,
-	.free_cached_objects	= xfs_fs_free_cached_objects,
-};
-
 static int
 xfs_remount_rw(
 	struct xfs_mount	*mp)


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 16/17] xfs: move xfs_fc_parse_param() above xfs_fc_get_tree()
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (14 preceding siblings ...)
  2019-11-04 10:55 ` [PATCH v9 15/17] xfs: move xfs_fc_get_tree() above xfs_fc_reconfigure() Ian Kent
@ 2019-11-04 10:56 ` Ian Kent
  2019-11-04 10:56 ` [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context() Ian Kent
  16 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:56 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

Grouping the options parsing and mount handling functions above the
struct fs_context_operations but below the struct super_operations
should improve (some) the grouping of the super operations while also
improving the grouping of the options parsing and mount handling code.

Lastly move xfs_fc_parse_param() and related functions down to above
xfs_fc_get_tree() and it's related functions.

But leave the options enum, struct fs_parameter_spec and the struct
fs_parameter_description declarations at the top since that's the
logical place for them.

This is a straight code move, there aren't any functional changes.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_super.c |  507 ++++++++++++++++++++++++++--------------------------
 1 file changed, 254 insertions(+), 253 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index a0233fc9f569..e156fd59d592 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -111,259 +111,6 @@ static const struct fs_parameter_description xfs_fs_parameters = {
 	.specs		= xfs_param_specs,
 };
 
-static int
-suffix_kstrtoint(
-	const char	*s,
-	unsigned int	base,
-	int		*res)
-{
-	int		last, shift_left_factor = 0, _res;
-	char		*value;
-	int		ret = 0;
-
-	value = kstrdup(s, GFP_KERNEL);
-	if (!value)
-		return -ENOMEM;
-
-	last = strlen(value) - 1;
-	if (value[last] == 'K' || value[last] == 'k') {
-		shift_left_factor = 10;
-		value[last] = '\0';
-	}
-	if (value[last] == 'M' || value[last] == 'm') {
-		shift_left_factor = 20;
-		value[last] = '\0';
-	}
-	if (value[last] == 'G' || value[last] == 'g') {
-		shift_left_factor = 30;
-		value[last] = '\0';
-	}
-
-	if (kstrtoint(value, base, &_res))
-		ret = -EINVAL;
-	kfree(value);
-	*res = _res << shift_left_factor;
-	return ret;
-}
-
-static int
-xfs_fc_parse_param(
-	struct fs_context	*fc,
-	struct fs_parameter	*param)
-{
-	struct xfs_mount	*mp = fc->s_fs_info;
-	struct fs_parse_result	result;
-	int			size = 0;
-	int			opt;
-
-	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
-	if (opt < 0)
-		return opt;
-
-	switch (opt) {
-	case Opt_logbufs:
-		mp->m_logbufs = result.uint_32;
-		return 0;
-	case Opt_logbsize:
-		if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
-			return -EINVAL;
-		return 0;
-	case Opt_logdev:
-		kfree(mp->m_logname);
-		mp->m_logname = kstrdup(param->string, GFP_KERNEL);
-		if (!mp->m_logname)
-			return -ENOMEM;
-		return 0;
-	case Opt_rtdev:
-		kfree(mp->m_rtname);
-		mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
-		if (!mp->m_rtname)
-			return -ENOMEM;
-		return 0;
-	case Opt_allocsize:
-		if (suffix_kstrtoint(param->string, 10, &size))
-			return -EINVAL;
-		mp->m_allocsize_log = ffs(size) - 1;
-		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
-		return 0;
-	case Opt_grpid:
-	case Opt_bsdgroups:
-		mp->m_flags |= XFS_MOUNT_GRPID;
-		return 0;
-	case Opt_nogrpid:
-	case Opt_sysvgroups:
-		mp->m_flags &= ~XFS_MOUNT_GRPID;
-		return 0;
-	case Opt_wsync:
-		mp->m_flags |= XFS_MOUNT_WSYNC;
-		return 0;
-	case Opt_norecovery:
-		mp->m_flags |= XFS_MOUNT_NORECOVERY;
-		return 0;
-	case Opt_noalign:
-		mp->m_flags |= XFS_MOUNT_NOALIGN;
-		return 0;
-	case Opt_swalloc:
-		mp->m_flags |= XFS_MOUNT_SWALLOC;
-		return 0;
-	case Opt_sunit:
-		mp->m_dalign = result.uint_32;
-		return 0;
-	case Opt_swidth:
-		mp->m_swidth = result.uint_32;
-		return 0;
-	case Opt_inode32:
-		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
-		return 0;
-	case Opt_inode64:
-		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
-		return 0;
-	case Opt_nouuid:
-		mp->m_flags |= XFS_MOUNT_NOUUID;
-		return 0;
-	case Opt_ikeep:
-		mp->m_flags |= XFS_MOUNT_IKEEP;
-		return 0;
-	case Opt_noikeep:
-		mp->m_flags &= ~XFS_MOUNT_IKEEP;
-		return 0;
-	case Opt_largeio:
-		mp->m_flags |= XFS_MOUNT_LARGEIO;
-		return 0;
-	case Opt_nolargeio:
-		mp->m_flags &= ~XFS_MOUNT_LARGEIO;
-		return 0;
-	case Opt_attr2:
-		mp->m_flags |= XFS_MOUNT_ATTR2;
-		return 0;
-	case Opt_noattr2:
-		mp->m_flags &= ~XFS_MOUNT_ATTR2;
-		mp->m_flags |= XFS_MOUNT_NOATTR2;
-		return 0;
-	case Opt_filestreams:
-		mp->m_flags |= XFS_MOUNT_FILESTREAMS;
-		return 0;
-	case Opt_noquota:
-		mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
-		mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
-		mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
-		return 0;
-	case Opt_quota:
-	case Opt_uquota:
-	case Opt_usrquota:
-		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
-				 XFS_UQUOTA_ENFD);
-		return 0;
-	case Opt_qnoenforce:
-	case Opt_uqnoenforce:
-		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
-		mp->m_qflags &= ~XFS_UQUOTA_ENFD;
-		return 0;
-	case Opt_pquota:
-	case Opt_prjquota:
-		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
-				 XFS_PQUOTA_ENFD);
-		return 0;
-	case Opt_pqnoenforce:
-		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
-		mp->m_qflags &= ~XFS_PQUOTA_ENFD;
-		return 0;
-	case Opt_gquota:
-	case Opt_grpquota:
-		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
-				 XFS_GQUOTA_ENFD);
-		return 0;
-	case Opt_gqnoenforce:
-		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
-		mp->m_qflags &= ~XFS_GQUOTA_ENFD;
-		return 0;
-	case Opt_discard:
-		mp->m_flags |= XFS_MOUNT_DISCARD;
-		return 0;
-	case Opt_nodiscard:
-		mp->m_flags &= ~XFS_MOUNT_DISCARD;
-		return 0;
-#ifdef CONFIG_FS_DAX
-	case Opt_dax:
-		mp->m_flags |= XFS_MOUNT_DAX;
-		return 0;
-#endif
-	default:
-		xfs_warn(mp, "unknown mount option [%s].", param->key);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int
-xfs_fc_validate_params(
-	struct xfs_mount	*mp)
-{
-	/*
-	 * 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) &&
-	    (mp->m_dalign || mp->m_swidth)) {
-		xfs_warn(mp,
-	"sunit and swidth options incompatible with the noalign option");
-		return -EINVAL;
-	}
-
-	if (!IS_ENABLED(CONFIG_XFS_QUOTA) && mp->m_qflags != 0) {
-		xfs_warn(mp, "quota support not available in this kernel.");
-		return -EINVAL;
-	}
-
-	if ((mp->m_dalign && !mp->m_swidth) ||
-	    (!mp->m_dalign && mp->m_swidth)) {
-		xfs_warn(mp, "sunit and swidth must be specified together");
-		return -EINVAL;
-	}
-
-	if (mp->m_dalign && (mp->m_swidth % mp->m_dalign != 0)) {
-		xfs_warn(mp,
-	"stripe width (%d) must be a multiple of the stripe unit (%d)",
-			mp->m_swidth, mp->m_dalign);
-		return -EINVAL;
-	}
-
-	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 ((mp->m_flags & XFS_MOUNT_ALLOCSIZE) &&
-	    (mp->m_allocsize_log > XFS_MAX_IO_LOG ||
-	     mp->m_allocsize_log < XFS_MIN_IO_LOG)) {
-		xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
-			mp->m_allocsize_log, XFS_MIN_IO_LOG, XFS_MAX_IO_LOG);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -1378,6 +1125,260 @@ xfs_mount_alloc(void)
 	return mp;
 }
 
+static int
+suffix_kstrtoint(
+	const char	*s,
+	unsigned int	base,
+	int		*res)
+{
+	int		last, shift_left_factor = 0, _res;
+	char		*value;
+	int		ret = 0;
+
+	value = kstrdup(s, GFP_KERNEL);
+	if (!value)
+		return -ENOMEM;
+
+	last = strlen(value) - 1;
+	if (value[last] == 'K' || value[last] == 'k') {
+		shift_left_factor = 10;
+		value[last] = '\0';
+	}
+	if (value[last] == 'M' || value[last] == 'm') {
+		shift_left_factor = 20;
+		value[last] = '\0';
+	}
+	if (value[last] == 'G' || value[last] == 'g') {
+		shift_left_factor = 30;
+		value[last] = '\0';
+	}
+
+	if (kstrtoint(value, base, &_res))
+		ret = -EINVAL;
+	kfree(value);
+	*res = _res << shift_left_factor;
+	return ret;
+}
+
+static int
+xfs_fc_parse_param(
+	struct fs_context	*fc,
+	struct fs_parameter	*param)
+{
+	struct xfs_mount	*mp = fc->s_fs_info;
+	struct fs_parse_result	result;
+	int			size = 0;
+	int			opt;
+
+	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_logbufs:
+		mp->m_logbufs = result.uint_32;
+		return 0;
+	case Opt_logbsize:
+		if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
+			return -EINVAL;
+		return 0;
+	case Opt_logdev:
+		kfree(mp->m_logname);
+		mp->m_logname = kstrdup(param->string, GFP_KERNEL);
+		if (!mp->m_logname)
+			return -ENOMEM;
+		return 0;
+	case Opt_rtdev:
+		kfree(mp->m_rtname);
+		mp->m_rtname = kstrdup(param->string, GFP_KERNEL);
+		if (!mp->m_rtname)
+			return -ENOMEM;
+		return 0;
+	case Opt_allocsize:
+		if (suffix_kstrtoint(param->string, 10, &size))
+			return -EINVAL;
+		mp->m_allocsize_log = ffs(size) - 1;
+		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
+		return 0;
+	case Opt_grpid:
+	case Opt_bsdgroups:
+		mp->m_flags |= XFS_MOUNT_GRPID;
+		return 0;
+	case Opt_nogrpid:
+	case Opt_sysvgroups:
+		mp->m_flags &= ~XFS_MOUNT_GRPID;
+		return 0;
+	case Opt_wsync:
+		mp->m_flags |= XFS_MOUNT_WSYNC;
+		return 0;
+	case Opt_norecovery:
+		mp->m_flags |= XFS_MOUNT_NORECOVERY;
+		return 0;
+	case Opt_noalign:
+		mp->m_flags |= XFS_MOUNT_NOALIGN;
+		return 0;
+	case Opt_swalloc:
+		mp->m_flags |= XFS_MOUNT_SWALLOC;
+		return 0;
+	case Opt_sunit:
+		mp->m_dalign = result.uint_32;
+		return 0;
+	case Opt_swidth:
+		mp->m_swidth = result.uint_32;
+		return 0;
+	case Opt_inode32:
+		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+		return 0;
+	case Opt_inode64:
+		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
+		return 0;
+	case Opt_nouuid:
+		mp->m_flags |= XFS_MOUNT_NOUUID;
+		return 0;
+	case Opt_ikeep:
+		mp->m_flags |= XFS_MOUNT_IKEEP;
+		return 0;
+	case Opt_noikeep:
+		mp->m_flags &= ~XFS_MOUNT_IKEEP;
+		return 0;
+	case Opt_largeio:
+		mp->m_flags |= XFS_MOUNT_LARGEIO;
+		return 0;
+	case Opt_nolargeio:
+		mp->m_flags &= ~XFS_MOUNT_LARGEIO;
+		return 0;
+	case Opt_attr2:
+		mp->m_flags |= XFS_MOUNT_ATTR2;
+		return 0;
+	case Opt_noattr2:
+		mp->m_flags &= ~XFS_MOUNT_ATTR2;
+		mp->m_flags |= XFS_MOUNT_NOATTR2;
+		return 0;
+	case Opt_filestreams:
+		mp->m_flags |= XFS_MOUNT_FILESTREAMS;
+		return 0;
+	case Opt_noquota:
+		mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
+		mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
+		mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
+		return 0;
+	case Opt_quota:
+	case Opt_uquota:
+	case Opt_usrquota:
+		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
+				 XFS_UQUOTA_ENFD);
+		return 0;
+	case Opt_qnoenforce:
+	case Opt_uqnoenforce:
+		mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
+		mp->m_qflags &= ~XFS_UQUOTA_ENFD;
+		return 0;
+	case Opt_pquota:
+	case Opt_prjquota:
+		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
+				 XFS_PQUOTA_ENFD);
+		return 0;
+	case Opt_pqnoenforce:
+		mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
+		mp->m_qflags &= ~XFS_PQUOTA_ENFD;
+		return 0;
+	case Opt_gquota:
+	case Opt_grpquota:
+		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
+				 XFS_GQUOTA_ENFD);
+		return 0;
+	case Opt_gqnoenforce:
+		mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
+		mp->m_qflags &= ~XFS_GQUOTA_ENFD;
+		return 0;
+	case Opt_discard:
+		mp->m_flags |= XFS_MOUNT_DISCARD;
+		return 0;
+	case Opt_nodiscard:
+		mp->m_flags &= ~XFS_MOUNT_DISCARD;
+		return 0;
+#ifdef CONFIG_FS_DAX
+	case Opt_dax:
+		mp->m_flags |= XFS_MOUNT_DAX;
+		return 0;
+#endif
+	default:
+		xfs_warn(mp, "unknown mount option [%s].", param->key);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+xfs_fc_validate_params(
+	struct xfs_mount	*mp)
+{
+	/*
+	 * 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) &&
+	    (mp->m_dalign || mp->m_swidth)) {
+		xfs_warn(mp,
+	"sunit and swidth options incompatible with the noalign option");
+		return -EINVAL;
+	}
+
+	if (!IS_ENABLED(CONFIG_XFS_QUOTA) && mp->m_qflags != 0) {
+		xfs_warn(mp, "quota support not available in this kernel.");
+		return -EINVAL;
+	}
+
+	if ((mp->m_dalign && !mp->m_swidth) ||
+	    (!mp->m_dalign && mp->m_swidth)) {
+		xfs_warn(mp, "sunit and swidth must be specified together");
+		return -EINVAL;
+	}
+
+	if (mp->m_dalign && (mp->m_swidth % mp->m_dalign != 0)) {
+		xfs_warn(mp,
+	"stripe width (%d) must be a multiple of the stripe unit (%d)",
+			mp->m_swidth, mp->m_dalign);
+		return -EINVAL;
+	}
+
+	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 ((mp->m_flags & XFS_MOUNT_ALLOCSIZE) &&
+	    (mp->m_allocsize_log > XFS_MAX_IO_LOG ||
+	     mp->m_allocsize_log < XFS_MIN_IO_LOG)) {
+		xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
+			mp->m_allocsize_log, XFS_MIN_IO_LOG, XFS_MAX_IO_LOG);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int
 xfs_fc_fill_super(
 	struct super_block	*sb,


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context()
  2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
                   ` (15 preceding siblings ...)
  2019-11-04 10:56 ` [PATCH v9 16/17] xfs: move xfs_fc_parse_param() above xfs_fc_get_tree() Ian Kent
@ 2019-11-04 10:56 ` Ian Kent
  2019-11-04 15:14   ` Christoph Hellwig
  2019-11-04 20:55   ` Darrick J. Wong
  16 siblings, 2 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-04 10:56 UTC (permalink / raw)
  To: linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

After switching to use the mount-api the only remaining caller of
xfs_mount_alloc() is xfs_init_fs_context(), so fold xfs_mount_alloc()
into it.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/xfs/xfs_super.c |   49 +++++++++++++++++++------------------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index e156fd59d592..c14f285f3256 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1096,35 +1096,6 @@ static const struct super_operations xfs_super_operations = {
 	.free_cached_objects	= xfs_fs_free_cached_objects,
 };
 
-static struct xfs_mount *
-xfs_mount_alloc(void)
-{
-	struct xfs_mount	*mp;
-
-	mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
-	if (!mp)
-		return NULL;
-
-	spin_lock_init(&mp->m_sb_lock);
-	spin_lock_init(&mp->m_agirotor_lock);
-	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC);
-	spin_lock_init(&mp->m_perag_lock);
-	mutex_init(&mp->m_growlock);
-	atomic_set(&mp->m_active_trans, 0);
-	INIT_DELAYED_WORK(&mp->m_reclaim_work, xfs_reclaim_worker);
-	INIT_DELAYED_WORK(&mp->m_eofblocks_work, xfs_eofblocks_worker);
-	INIT_DELAYED_WORK(&mp->m_cowblocks_work, xfs_cowblocks_worker);
-	mp->m_kobj.kobject.kset = xfs_kset;
-	/*
-	 * We don't create the finobt per-ag space reservation until after log
-	 * recovery, so we must set this to true so that an ifree transaction
-	 * started during log recovery will not depend on space reservations
-	 * for finobt expansion.
-	 */
-	mp->m_finobt_nores = true;
-	return mp;
-}
-
 static int
 suffix_kstrtoint(
 	const char	*s,
@@ -1763,10 +1734,28 @@ static int xfs_init_fs_context(
 {
 	struct xfs_mount	*mp;
 
-	mp = xfs_mount_alloc();
+	mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
 	if (!mp)
 		return -ENOMEM;
 
+	spin_lock_init(&mp->m_sb_lock);
+	spin_lock_init(&mp->m_agirotor_lock);
+	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC);
+	spin_lock_init(&mp->m_perag_lock);
+	mutex_init(&mp->m_growlock);
+	atomic_set(&mp->m_active_trans, 0);
+	INIT_DELAYED_WORK(&mp->m_reclaim_work, xfs_reclaim_worker);
+	INIT_DELAYED_WORK(&mp->m_eofblocks_work, xfs_eofblocks_worker);
+	INIT_DELAYED_WORK(&mp->m_cowblocks_work, xfs_cowblocks_worker);
+	mp->m_kobj.kobject.kset = xfs_kset;
+	/*
+	 * We don't create the finobt per-ag space reservation until after log
+	 * recovery, so we must set this to true so that an ifree transaction
+	 * started during log recovery will not depend on space reservations
+	 * for finobt expansion.
+	 */
+	mp->m_finobt_nores = true;
+
 	/*
 	 * These can be overridden by the mount option parsing.
 	 */


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v9 13/17] xfs: switch to use the new mount-api
  2019-11-04 10:55 ` [PATCH v9 13/17] xfs: switch to use the new mount-api Ian Kent
@ 2019-11-04 15:14   ` Christoph Hellwig
  2019-11-04 22:53   ` Darrick J. Wong
  1 sibling, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2019-11-04 15:14 UTC (permalink / raw)
  To: Ian Kent
  Cc: linux-xfs, Christoph Hellwig, Darrick J. Wong, Brian Foster,
	Eric Sandeen, David Howells, Dave Chinner, Al Viro

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context()
  2019-11-04 10:56 ` [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context() Ian Kent
@ 2019-11-04 15:14   ` Christoph Hellwig
  2019-11-04 20:55   ` Darrick J. Wong
  1 sibling, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2019-11-04 15:14 UTC (permalink / raw)
  To: Ian Kent
  Cc: linux-xfs, Christoph Hellwig, Darrick J. Wong, Brian Foster,
	Eric Sandeen, David Howells, Dave Chinner, Al Viro

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context()
  2019-11-04 10:56 ` [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context() Ian Kent
  2019-11-04 15:14   ` Christoph Hellwig
@ 2019-11-04 20:55   ` Darrick J. Wong
  1 sibling, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2019-11-04 20:55 UTC (permalink / raw)
  To: Ian Kent
  Cc: linux-xfs, Christoph Hellwig, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

On Mon, Nov 04, 2019 at 06:56:05PM +0800, Ian Kent wrote:
> After switching to use the mount-api the only remaining caller of
> xfs_mount_alloc() is xfs_init_fs_context(), so fold xfs_mount_alloc()
> into it.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_super.c |   49 +++++++++++++++++++------------------------------
>  1 file changed, 19 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index e156fd59d592..c14f285f3256 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1096,35 +1096,6 @@ static const struct super_operations xfs_super_operations = {
>  	.free_cached_objects	= xfs_fs_free_cached_objects,
>  };
>  
> -static struct xfs_mount *
> -xfs_mount_alloc(void)
> -{
> -	struct xfs_mount	*mp;
> -
> -	mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
> -	if (!mp)
> -		return NULL;
> -
> -	spin_lock_init(&mp->m_sb_lock);
> -	spin_lock_init(&mp->m_agirotor_lock);
> -	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC);
> -	spin_lock_init(&mp->m_perag_lock);
> -	mutex_init(&mp->m_growlock);
> -	atomic_set(&mp->m_active_trans, 0);
> -	INIT_DELAYED_WORK(&mp->m_reclaim_work, xfs_reclaim_worker);
> -	INIT_DELAYED_WORK(&mp->m_eofblocks_work, xfs_eofblocks_worker);
> -	INIT_DELAYED_WORK(&mp->m_cowblocks_work, xfs_cowblocks_worker);
> -	mp->m_kobj.kobject.kset = xfs_kset;
> -	/*
> -	 * We don't create the finobt per-ag space reservation until after log
> -	 * recovery, so we must set this to true so that an ifree transaction
> -	 * started during log recovery will not depend on space reservations
> -	 * for finobt expansion.
> -	 */
> -	mp->m_finobt_nores = true;
> -	return mp;
> -}
> -
>  static int
>  suffix_kstrtoint(
>  	const char	*s,
> @@ -1763,10 +1734,28 @@ static int xfs_init_fs_context(
>  {
>  	struct xfs_mount	*mp;
>  
> -	mp = xfs_mount_alloc();
> +	mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
>  	if (!mp)
>  		return -ENOMEM;
>  
> +	spin_lock_init(&mp->m_sb_lock);
> +	spin_lock_init(&mp->m_agirotor_lock);
> +	INIT_RADIX_TREE(&mp->m_perag_tree, GFP_ATOMIC);
> +	spin_lock_init(&mp->m_perag_lock);
> +	mutex_init(&mp->m_growlock);
> +	atomic_set(&mp->m_active_trans, 0);
> +	INIT_DELAYED_WORK(&mp->m_reclaim_work, xfs_reclaim_worker);
> +	INIT_DELAYED_WORK(&mp->m_eofblocks_work, xfs_eofblocks_worker);
> +	INIT_DELAYED_WORK(&mp->m_cowblocks_work, xfs_cowblocks_worker);
> +	mp->m_kobj.kobject.kset = xfs_kset;
> +	/*
> +	 * We don't create the finobt per-ag space reservation until after log
> +	 * recovery, so we must set this to true so that an ifree transaction
> +	 * started during log recovery will not depend on space reservations
> +	 * for finobt expansion.
> +	 */
> +	mp->m_finobt_nores = true;
> +
>  	/*
>  	 * These can be overridden by the mount option parsing.
>  	 */
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v9 13/17] xfs: switch to use the new mount-api
  2019-11-04 10:55 ` [PATCH v9 13/17] xfs: switch to use the new mount-api Ian Kent
  2019-11-04 15:14   ` Christoph Hellwig
@ 2019-11-04 22:53   ` Darrick J. Wong
  2019-11-05  3:00     ` Ian Kent
  1 sibling, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2019-11-04 22:53 UTC (permalink / raw)
  To: Ian Kent
  Cc: linux-xfs, Christoph Hellwig, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

On Mon, Nov 04, 2019 at 06:55:44PM +0800, Ian Kent wrote:
> Define the struct fs_parameter_spec table that's used by the new
> mount-api for options parsing.
> 
> Create the various fs context operations methods and define the
> fs_context_operations struct.
> 
> Create the fs context initialization method and update the struct
> file_system_type to utilize it. The initialization function is
> responsible for working storage initialization, allocation and
> initialization of file system private information storage and for
> setting the operations in the fs context.
> 
> Also set struct file_system_type .parameters to the newly defined
> struct fs_parameter_spec options parsing table for use by the fs
> context methods and remove unused code.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
>  fs/xfs/xfs_super.c |  408 ++++++++++++++++++++++------------------------------
>  1 file changed, 171 insertions(+), 237 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 62dfc678c415..2d4edaaf934e 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -37,7 +37,8 @@
>  #include "xfs_reflink.h"
>  
>  #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;
>  
> @@ -58,55 +59,57 @@ 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,
> +	Opt_discard, Opt_nodiscard, Opt_dax,
>  };
>  
> -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},
> +static const struct fs_parameter_spec xfs_param_specs[] = {
> +	fsparam_u32("logbufs",		Opt_logbufs),
> +	fsparam_string("logbsize",	Opt_logbsize),
> +	fsparam_string("logdev",	Opt_logdev),
> +	fsparam_string("rtdev",		Opt_rtdev),
> +	fsparam_flag("wsync",		Opt_wsync),
> +	fsparam_flag("noalign",		Opt_noalign),
> +	fsparam_flag("swalloc",		Opt_swalloc),
> +	fsparam_u32("sunit",		Opt_sunit),
> +	fsparam_u32("swidth",		Opt_swidth),
> +	fsparam_flag("nouuid",		Opt_nouuid),
> +	fsparam_flag("grpid",		Opt_grpid),
> +	fsparam_flag("nogrpid",		Opt_nogrpid),
> +	fsparam_flag("bsdgroups",	Opt_bsdgroups),
> +	fsparam_flag("sysvgroups",	Opt_sysvgroups),
> +	fsparam_string("allocsize",	Opt_allocsize),
> +	fsparam_flag("norecovery",	Opt_norecovery),
> +	fsparam_flag("inode64",		Opt_inode64),
> +	fsparam_flag("inode32",		Opt_inode32),
> +	fsparam_flag("ikeep",		Opt_ikeep),
> +	fsparam_flag("noikeep",		Opt_noikeep),
> +	fsparam_flag("largeio",		Opt_largeio),
> +	fsparam_flag("nolargeio",	Opt_nolargeio),
> +	fsparam_flag("attr2",		Opt_attr2),
> +	fsparam_flag("noattr2",		Opt_noattr2),
> +	fsparam_flag("filestreams",	Opt_filestreams),
> +	fsparam_flag("quota",		Opt_quota),
> +	fsparam_flag("noquota",		Opt_noquota),
> +	fsparam_flag("usrquota",	Opt_usrquota),
> +	fsparam_flag("grpquota",	Opt_grpquota),
> +	fsparam_flag("prjquota",	Opt_prjquota),
> +	fsparam_flag("uquota",		Opt_uquota),
> +	fsparam_flag("gquota",		Opt_gquota),
> +	fsparam_flag("pquota",		Opt_pquota),
> +	fsparam_flag("uqnoenforce",	Opt_uqnoenforce),
> +	fsparam_flag("gqnoenforce",	Opt_gqnoenforce),
> +	fsparam_flag("pqnoenforce",	Opt_pqnoenforce),
> +	fsparam_flag("qnoenforce",	Opt_qnoenforce),
> +	fsparam_flag("discard",		Opt_discard),
> +	fsparam_flag("nodiscard",	Opt_nodiscard),
> +	fsparam_flag("dax",		Opt_dax),
> +	{}
>  };
>  
> +static const struct fs_parameter_description xfs_fs_parameters = {
> +	.name		= "xfs",
> +	.specs		= xfs_param_specs,
> +};
>  
>  static int
>  suffix_kstrtoint(
> @@ -143,55 +146,42 @@ suffix_kstrtoint(
>  	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;
> -}
> -
>  static int
>  xfs_fc_parse_param(

This is the only function where mp->m_super can be NULL, correct?

If so, I'll add a comment to that effect, and,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

(for this and patch 12.)

Will throw this at a testing cloud and see what happens...

--D

> -	int			token,
> -	char			*p,
> -	substring_t		*args,
> -	struct xfs_mount	*mp)
> +	struct fs_context	*fc,
> +	struct fs_parameter	*param)
>  {
> +	struct xfs_mount	*mp = fc->s_fs_info;
> +	struct fs_parse_result	result;
>  	int			size = 0;
> +	int			opt;
> +
> +	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> +	if (opt < 0)
> +		return opt;
>  
> -	switch (token) {
> +	switch (opt) {
>  	case Opt_logbufs:
> -		if (match_int(args, &mp->m_logbufs))
> -			return -EINVAL;
> +		mp->m_logbufs = result.uint_32;
>  		return 0;
>  	case Opt_logbsize:
> -		if (match_kstrtoint(args, 10, &mp->m_logbsize))
> +		if (suffix_kstrtoint(param->string, 10, &mp->m_logbsize))
>  			return -EINVAL;
>  		return 0;
>  	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;
>  		return 0;
>  	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;
>  		return 0;
>  	case Opt_allocsize:
> -		if (match_kstrtoint(args, 10, &size))
> +		if (suffix_kstrtoint(param->string, 10, &size))
>  			return -EINVAL;
>  		mp->m_allocsize_log = ffs(size) - 1;
>  		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
> @@ -217,12 +207,10 @@ xfs_fc_parse_param(
>  		mp->m_flags |= XFS_MOUNT_SWALLOC;
>  		return 0;
>  	case Opt_sunit:
> -		if (match_int(args, &mp->m_dalign))
> -			return -EINVAL;
> +		mp->m_dalign = result.uint_32;
>  		return 0;
>  	case Opt_swidth:
> -		if (match_int(args, &mp->m_swidth))
> -			return -EINVAL;
> +		mp->m_swidth = result.uint_32;
>  		return 0;
>  	case Opt_inode32:
>  		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> @@ -301,7 +289,7 @@ xfs_fc_parse_param(
>  		return 0;
>  #endif
>  	default:
> -		xfs_warn(mp, "unknown mount option [%s].", p);
> +		xfs_warn(mp, "unknown mount option [%s].", param->key);
>  		return -EINVAL;
>  	}
>  
> @@ -376,62 +364,6 @@ xfs_fc_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;
> -	substring_t		args[MAX_OPT_ARGS];
> -
> -	/*
> -	 * 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;
> -
> -	/*
> -	 * These can be overridden by the mount option parsing.
> -	 */
> -	mp->m_logbufs = -1;
> -	mp->m_logbsize = -1;
> -	mp->m_allocsize_log = 16; /* 64k */
> -
> -	if (!options)
> -		return 0;
> -
> -	while ((p = strsep(&options, ",")) != NULL) {
> -		int		token;
> -		int		ret;
> -
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, tokens, args);
> -		ret = xfs_fc_parse_param(token, p, args, mp);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	return xfs_fc_validate_params(mp);
> -}
> -
>  struct proc_xfs_info {
>  	uint64_t	flag;
>  	char		*str;
> @@ -1207,25 +1139,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_mount_free(tmp_mp);
> -
> -	return error;
> -}
> -
>  static int
>  xfs_remount_rw(
>  	struct xfs_mount	*mp)
> @@ -1329,76 +1242,57 @@ xfs_remount_ro(
>  	return 0;
>  }
>  
> -STATIC int
> -xfs_fs_remount(
> -	struct super_block	*sb,
> -	int			*flags,
> -	char			*options)
> +/*
> + * 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.
> + */
> +static int
> +xfs_fc_reconfigure(
> +	struct fs_context	*fc)
>  {
> -	struct xfs_mount	*mp = XFS_M(sb);
> +	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;
> -	substring_t		args[MAX_OPT_ARGS];
> -	char			*p;
> +	int			flags = fc->sb_flags;
>  	int			error;
>  
> -	/* First, check for complete junk; i.e. invalid options */
> -	error = xfs_test_remount_options(sb, options);
> +	error = xfs_fc_validate_params(new_mp);
>  	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
> -		}
> +	sync_filesystem(mp->m_super);
> +
> +	/* 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)) {
> +	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)) {
> +	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
>  		error = xfs_remount_ro(mp);
>  		if (error)
>  			return error;
> @@ -1588,28 +1482,18 @@ xfs_mount_alloc(void)
>  	return mp;
>  }
>  
> -
> -STATIC int
> -xfs_fs_fill_super(
> +static int
> +xfs_fc_fill_super(
>  	struct super_block	*sb,
> -	void			*data,
> -	int			silent)
> +	struct fs_context	*fc)
>  {
> +	struct xfs_mount	*mp = sb->s_fs_info;
>  	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();
> -	if (!mp)
> -		goto out;
>  	mp->m_super = sb;
> -	sb->s_fs_info = mp;
>  
> -	error = xfs_parseargs(mp, (char *)data);
> +	error = xfs_fc_validate_params(mp);
>  	if (error)
>  		goto out_free_names;
>  
> @@ -1633,7 +1517,7 @@ xfs_fs_fill_super(
>  		msleep(xfs_globals.mount_delay * 1000);
>  	}
>  
> -	if (silent)
> +	if (fc->sb_flags & SB_SILENT)
>  		flags |= XFS_MFSI_QUIET;
>  
>  	error = xfs_open_devices(mp);
> @@ -1778,7 +1662,6 @@ xfs_fs_fill_super(
>   out_free_names:
>  	sb->s_fs_info = NULL;
>  	xfs_mount_free(mp);
> - out:
>  	return error;
>  
>   out_unmount:
> @@ -1787,6 +1670,13 @@ xfs_fs_fill_super(
>  	goto out_free_sb;
>  }
>  
> +static int
> +xfs_fc_get_tree(
> +	struct fs_context	*fc)
> +{
> +	return get_tree_bdev(fc, xfs_fc_fill_super);
> +}
> +
>  STATIC void
>  xfs_fs_put_super(
>  	struct super_block	*sb)
> @@ -1811,16 +1701,6 @@ xfs_fs_put_super(
>  	xfs_mount_free(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,
> @@ -1850,16 +1730,70 @@ 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,
>  };
>  
> +static void xfs_fc_free(
> +	struct fs_context	*fc)
> +{
> +	struct xfs_mount	*mp = fc->s_fs_info;
> +
> +	/*
> +	 * mp is stored in the fs_context when it is initialized.
> +	 * mp is transferred to the superblock on a successful mount,
> +	 * but if an error occurs before the transfer we have to free
> +	 * it here.
> +	 */
> +	if (mp)
> +		xfs_mount_free(mp);
> +}
> +
> +static const struct fs_context_operations xfs_context_ops = {
> +	.parse_param = xfs_fc_parse_param,
> +	.get_tree    = xfs_fc_get_tree,
> +	.reconfigure = xfs_fc_reconfigure,
> +	.free        = xfs_fc_free,
> +};
> +
> +static int xfs_init_fs_context(
> +	struct fs_context	*fc)
> +{
> +	struct xfs_mount	*mp;
> +
> +	mp = xfs_mount_alloc();
> +	if (!mp)
> +		return -ENOMEM;
> +
> +	/*
> +	 * These can be overridden by the mount option parsing.
> +	 */
> +	mp->m_logbufs = -1;
> +	mp->m_logbsize = -1;
> +	mp->m_allocsize_log = 16; /* 64k */
> +
> +	/*
> +	 * 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->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	[flat|nested] 25+ messages in thread

* Re: [PATCH v9 13/17] xfs: switch to use the new mount-api
  2019-11-04 22:53   ` Darrick J. Wong
@ 2019-11-05  3:00     ` Ian Kent
  2019-11-05 15:23       ` Darrick J. Wong
  0 siblings, 1 reply; 25+ messages in thread
From: Ian Kent @ 2019-11-05  3:00 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: linux-xfs, Christoph Hellwig, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

On Mon, 2019-11-04 at 14:53 -0800, Darrick J. Wong wrote:
> On Mon, Nov 04, 2019 at 06:55:44PM +0800, Ian Kent wrote:
> > Define the struct fs_parameter_spec table that's used by the new
> > mount-api for options parsing.
> > 
> > Create the various fs context operations methods and define the
> > fs_context_operations struct.
> > 
> > Create the fs context initialization method and update the struct
> > file_system_type to utilize it. The initialization function is
> > responsible for working storage initialization, allocation and
> > initialization of file system private information storage and for
> > setting the operations in the fs context.
> > 
> > Also set struct file_system_type .parameters to the newly defined
> > struct fs_parameter_spec options parsing table for use by the fs
> > context methods and remove unused code.
> > 
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > ---
> >  fs/xfs/xfs_super.c |  408 ++++++++++++++++++++++----------------
> > --------------
> >  1 file changed, 171 insertions(+), 237 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 62dfc678c415..2d4edaaf934e 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -37,7 +37,8 @@
> >  #include "xfs_reflink.h"
> >  
> >  #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;
> >  
> > @@ -58,55 +59,57 @@ 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,
> > +	Opt_discard, Opt_nodiscard, Opt_dax,
> >  };
> >  
> > -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},
> > +static const struct fs_parameter_spec xfs_param_specs[] = {
> > +	fsparam_u32("logbufs",		Opt_logbufs),
> > +	fsparam_string("logbsize",	Opt_logbsize),
> > +	fsparam_string("logdev",	Opt_logdev),
> > +	fsparam_string("rtdev",		Opt_rtdev),
> > +	fsparam_flag("wsync",		Opt_wsync),
> > +	fsparam_flag("noalign",		Opt_noalign),
> > +	fsparam_flag("swalloc",		Opt_swalloc),
> > +	fsparam_u32("sunit",		Opt_sunit),
> > +	fsparam_u32("swidth",		Opt_swidth),
> > +	fsparam_flag("nouuid",		Opt_nouuid),
> > +	fsparam_flag("grpid",		Opt_grpid),
> > +	fsparam_flag("nogrpid",		Opt_nogrpid),
> > +	fsparam_flag("bsdgroups",	Opt_bsdgroups),
> > +	fsparam_flag("sysvgroups",	Opt_sysvgroups),
> > +	fsparam_string("allocsize",	Opt_allocsize),
> > +	fsparam_flag("norecovery",	Opt_norecovery),
> > +	fsparam_flag("inode64",		Opt_inode64),
> > +	fsparam_flag("inode32",		Opt_inode32),
> > +	fsparam_flag("ikeep",		Opt_ikeep),
> > +	fsparam_flag("noikeep",		Opt_noikeep),
> > +	fsparam_flag("largeio",		Opt_largeio),
> > +	fsparam_flag("nolargeio",	Opt_nolargeio),
> > +	fsparam_flag("attr2",		Opt_attr2),
> > +	fsparam_flag("noattr2",		Opt_noattr2),
> > +	fsparam_flag("filestreams",	Opt_filestreams),
> > +	fsparam_flag("quota",		Opt_quota),
> > +	fsparam_flag("noquota",		Opt_noquota),
> > +	fsparam_flag("usrquota",	Opt_usrquota),
> > +	fsparam_flag("grpquota",	Opt_grpquota),
> > +	fsparam_flag("prjquota",	Opt_prjquota),
> > +	fsparam_flag("uquota",		Opt_uquota),
> > +	fsparam_flag("gquota",		Opt_gquota),
> > +	fsparam_flag("pquota",		Opt_pquota),
> > +	fsparam_flag("uqnoenforce",	Opt_uqnoenforce),
> > +	fsparam_flag("gqnoenforce",	Opt_gqnoenforce),
> > +	fsparam_flag("pqnoenforce",	Opt_pqnoenforce),
> > +	fsparam_flag("qnoenforce",	Opt_qnoenforce),
> > +	fsparam_flag("discard",		Opt_discard),
> > +	fsparam_flag("nodiscard",	Opt_nodiscard),
> > +	fsparam_flag("dax",		Opt_dax),
> > +	{}
> >  };
> >  
> > +static const struct fs_parameter_description xfs_fs_parameters = {
> > +	.name		= "xfs",
> > +	.specs		= xfs_param_specs,
> > +};
> >  
> >  static int
> >  suffix_kstrtoint(
> > @@ -143,55 +146,42 @@ suffix_kstrtoint(
> >  	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;
> > -}
> > -
> >  static int
> >  xfs_fc_parse_param(
> 
> This is the only function where mp->m_super can be NULL, correct?

Yep.

> 
> If so, I'll add a comment to that effect, and,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, much appreciated.

> 
> (for this and patch 12.)
> 
> Will throw this at a testing cloud and see what happens...

It's seems ok when I run it against xfstests but I'm not as familiar
with what to expect as you would be, I'm interested to hear how it
goes, ;)

I noticed a significant change in test run times with the latest
for-next but there was also an xfstests update so I'm not sure
what caused it. Some test run times went up by 8 to 10 or more
times, which seemed like a big change, possibly a regression.

Ian

> 
> --D
> 
> > -	int			token,
> > -	char			*p,
> > -	substring_t		*args,
> > -	struct xfs_mount	*mp)
> > +	struct fs_context	*fc,
> > +	struct fs_parameter	*param)
> >  {
> > +	struct xfs_mount	*mp = fc->s_fs_info;
> > +	struct fs_parse_result	result;
> >  	int			size = 0;
> > +	int			opt;
> > +
> > +	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> > +	if (opt < 0)
> > +		return opt;
> >  
> > -	switch (token) {
> > +	switch (opt) {
> >  	case Opt_logbufs:
> > -		if (match_int(args, &mp->m_logbufs))
> > -			return -EINVAL;
> > +		mp->m_logbufs = result.uint_32;
> >  		return 0;
> >  	case Opt_logbsize:
> > -		if (match_kstrtoint(args, 10, &mp->m_logbsize))
> > +		if (suffix_kstrtoint(param->string, 10, &mp-
> > >m_logbsize))
> >  			return -EINVAL;
> >  		return 0;
> >  	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;
> >  		return 0;
> >  	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;
> >  		return 0;
> >  	case Opt_allocsize:
> > -		if (match_kstrtoint(args, 10, &size))
> > +		if (suffix_kstrtoint(param->string, 10, &size))
> >  			return -EINVAL;
> >  		mp->m_allocsize_log = ffs(size) - 1;
> >  		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
> > @@ -217,12 +207,10 @@ xfs_fc_parse_param(
> >  		mp->m_flags |= XFS_MOUNT_SWALLOC;
> >  		return 0;
> >  	case Opt_sunit:
> > -		if (match_int(args, &mp->m_dalign))
> > -			return -EINVAL;
> > +		mp->m_dalign = result.uint_32;
> >  		return 0;
> >  	case Opt_swidth:
> > -		if (match_int(args, &mp->m_swidth))
> > -			return -EINVAL;
> > +		mp->m_swidth = result.uint_32;
> >  		return 0;
> >  	case Opt_inode32:
> >  		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> > @@ -301,7 +289,7 @@ xfs_fc_parse_param(
> >  		return 0;
> >  #endif
> >  	default:
> > -		xfs_warn(mp, "unknown mount option [%s].", p);
> > +		xfs_warn(mp, "unknown mount option [%s].", param->key);
> >  		return -EINVAL;
> >  	}
> >  
> > @@ -376,62 +364,6 @@ xfs_fc_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;
> > -	substring_t		args[MAX_OPT_ARGS];
> > -
> > -	/*
> > -	 * 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;
> > -
> > -	/*
> > -	 * These can be overridden by the mount option parsing.
> > -	 */
> > -	mp->m_logbufs = -1;
> > -	mp->m_logbsize = -1;
> > -	mp->m_allocsize_log = 16; /* 64k */
> > -
> > -	if (!options)
> > -		return 0;
> > -
> > -	while ((p = strsep(&options, ",")) != NULL) {
> > -		int		token;
> > -		int		ret;
> > -
> > -		if (!*p)
> > -			continue;
> > -
> > -		token = match_token(p, tokens, args);
> > -		ret = xfs_fc_parse_param(token, p, args, mp);
> > -		if (ret)
> > -			return ret;
> > -	}
> > -
> > -	return xfs_fc_validate_params(mp);
> > -}
> > -
> >  struct proc_xfs_info {
> >  	uint64_t	flag;
> >  	char		*str;
> > @@ -1207,25 +1139,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_mount_free(tmp_mp);
> > -
> > -	return error;
> > -}
> > -
> >  static int
> >  xfs_remount_rw(
> >  	struct xfs_mount	*mp)
> > @@ -1329,76 +1242,57 @@ xfs_remount_ro(
> >  	return 0;
> >  }
> >  
> > -STATIC int
> > -xfs_fs_remount(
> > -	struct super_block	*sb,
> > -	int			*flags,
> > -	char			*options)
> > +/*
> > + * 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.
> > + */
> > +static int
> > +xfs_fc_reconfigure(
> > +	struct fs_context	*fc)
> >  {
> > -	struct xfs_mount	*mp = XFS_M(sb);
> > +	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;
> > -	substring_t		args[MAX_OPT_ARGS];
> > -	char			*p;
> > +	int			flags = fc->sb_flags;
> >  	int			error;
> >  
> > -	/* First, check for complete junk; i.e. invalid options */
> > -	error = xfs_test_remount_options(sb, options);
> > +	error = xfs_fc_validate_params(new_mp);
> >  	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
> > -		}
> > +	sync_filesystem(mp->m_super);
> > +
> > +	/* 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))
> > {
> > +	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))
> > {
> > +	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
> >  		error = xfs_remount_ro(mp);
> >  		if (error)
> >  			return error;
> > @@ -1588,28 +1482,18 @@ xfs_mount_alloc(void)
> >  	return mp;
> >  }
> >  
> > -
> > -STATIC int
> > -xfs_fs_fill_super(
> > +static int
> > +xfs_fc_fill_super(
> >  	struct super_block	*sb,
> > -	void			*data,
> > -	int			silent)
> > +	struct fs_context	*fc)
> >  {
> > +	struct xfs_mount	*mp = sb->s_fs_info;
> >  	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();
> > -	if (!mp)
> > -		goto out;
> >  	mp->m_super = sb;
> > -	sb->s_fs_info = mp;
> >  
> > -	error = xfs_parseargs(mp, (char *)data);
> > +	error = xfs_fc_validate_params(mp);
> >  	if (error)
> >  		goto out_free_names;
> >  
> > @@ -1633,7 +1517,7 @@ xfs_fs_fill_super(
> >  		msleep(xfs_globals.mount_delay * 1000);
> >  	}
> >  
> > -	if (silent)
> > +	if (fc->sb_flags & SB_SILENT)
> >  		flags |= XFS_MFSI_QUIET;
> >  
> >  	error = xfs_open_devices(mp);
> > @@ -1778,7 +1662,6 @@ xfs_fs_fill_super(
> >   out_free_names:
> >  	sb->s_fs_info = NULL;
> >  	xfs_mount_free(mp);
> > - out:
> >  	return error;
> >  
> >   out_unmount:
> > @@ -1787,6 +1670,13 @@ xfs_fs_fill_super(
> >  	goto out_free_sb;
> >  }
> >  
> > +static int
> > +xfs_fc_get_tree(
> > +	struct fs_context	*fc)
> > +{
> > +	return get_tree_bdev(fc, xfs_fc_fill_super);
> > +}
> > +
> >  STATIC void
> >  xfs_fs_put_super(
> >  	struct super_block	*sb)
> > @@ -1811,16 +1701,6 @@ xfs_fs_put_super(
> >  	xfs_mount_free(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,
> > @@ -1850,16 +1730,70 @@ 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,
> >  };
> >  
> > +static void xfs_fc_free(
> > +	struct fs_context	*fc)
> > +{
> > +	struct xfs_mount	*mp = fc->s_fs_info;
> > +
> > +	/*
> > +	 * mp is stored in the fs_context when it is initialized.
> > +	 * mp is transferred to the superblock on a successful mount,
> > +	 * but if an error occurs before the transfer we have to free
> > +	 * it here.
> > +	 */
> > +	if (mp)
> > +		xfs_mount_free(mp);
> > +}
> > +
> > +static const struct fs_context_operations xfs_context_ops = {
> > +	.parse_param = xfs_fc_parse_param,
> > +	.get_tree    = xfs_fc_get_tree,
> > +	.reconfigure = xfs_fc_reconfigure,
> > +	.free        = xfs_fc_free,
> > +};
> > +
> > +static int xfs_init_fs_context(
> > +	struct fs_context	*fc)
> > +{
> > +	struct xfs_mount	*mp;
> > +
> > +	mp = xfs_mount_alloc();
> > +	if (!mp)
> > +		return -ENOMEM;
> > +
> > +	/*
> > +	 * These can be overridden by the mount option parsing.
> > +	 */
> > +	mp->m_logbufs = -1;
> > +	mp->m_logbsize = -1;
> > +	mp->m_allocsize_log = 16; /* 64k */
> > +
> > +	/*
> > +	 * 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->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	[flat|nested] 25+ messages in thread

* Re: [PATCH v9 13/17] xfs: switch to use the new mount-api
  2019-11-05  3:00     ` Ian Kent
@ 2019-11-05 15:23       ` Darrick J. Wong
  2019-11-06  2:27         ` Ian Kent
  0 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2019-11-05 15:23 UTC (permalink / raw)
  To: Ian Kent
  Cc: linux-xfs, Christoph Hellwig, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

On Tue, Nov 05, 2019 at 11:00:36AM +0800, Ian Kent wrote:
> On Mon, 2019-11-04 at 14:53 -0800, Darrick J. Wong wrote:
> > On Mon, Nov 04, 2019 at 06:55:44PM +0800, Ian Kent wrote:
> > > Define the struct fs_parameter_spec table that's used by the new
> > > mount-api for options parsing.
> > > 
> > > Create the various fs context operations methods and define the
> > > fs_context_operations struct.
> > > 
> > > Create the fs context initialization method and update the struct
> > > file_system_type to utilize it. The initialization function is
> > > responsible for working storage initialization, allocation and
> > > initialization of file system private information storage and for
> > > setting the operations in the fs context.
> > > 
> > > Also set struct file_system_type .parameters to the newly defined
> > > struct fs_parameter_spec options parsing table for use by the fs
> > > context methods and remove unused code.
> > > 
> > > Signed-off-by: Ian Kent <raven@themaw.net>
> > > ---
> > >  fs/xfs/xfs_super.c |  408 ++++++++++++++++++++++----------------
> > > --------------
> > >  1 file changed, 171 insertions(+), 237 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index 62dfc678c415..2d4edaaf934e 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -37,7 +37,8 @@
> > >  #include "xfs_reflink.h"
> > >  
> > >  #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;
> > >  
> > > @@ -58,55 +59,57 @@ 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,
> > > +	Opt_discard, Opt_nodiscard, Opt_dax,
> > >  };
> > >  
> > > -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},
> > > +static const struct fs_parameter_spec xfs_param_specs[] = {
> > > +	fsparam_u32("logbufs",		Opt_logbufs),
> > > +	fsparam_string("logbsize",	Opt_logbsize),
> > > +	fsparam_string("logdev",	Opt_logdev),
> > > +	fsparam_string("rtdev",		Opt_rtdev),
> > > +	fsparam_flag("wsync",		Opt_wsync),
> > > +	fsparam_flag("noalign",		Opt_noalign),
> > > +	fsparam_flag("swalloc",		Opt_swalloc),
> > > +	fsparam_u32("sunit",		Opt_sunit),
> > > +	fsparam_u32("swidth",		Opt_swidth),
> > > +	fsparam_flag("nouuid",		Opt_nouuid),
> > > +	fsparam_flag("grpid",		Opt_grpid),
> > > +	fsparam_flag("nogrpid",		Opt_nogrpid),
> > > +	fsparam_flag("bsdgroups",	Opt_bsdgroups),
> > > +	fsparam_flag("sysvgroups",	Opt_sysvgroups),
> > > +	fsparam_string("allocsize",	Opt_allocsize),
> > > +	fsparam_flag("norecovery",	Opt_norecovery),
> > > +	fsparam_flag("inode64",		Opt_inode64),
> > > +	fsparam_flag("inode32",		Opt_inode32),
> > > +	fsparam_flag("ikeep",		Opt_ikeep),
> > > +	fsparam_flag("noikeep",		Opt_noikeep),
> > > +	fsparam_flag("largeio",		Opt_largeio),
> > > +	fsparam_flag("nolargeio",	Opt_nolargeio),
> > > +	fsparam_flag("attr2",		Opt_attr2),
> > > +	fsparam_flag("noattr2",		Opt_noattr2),
> > > +	fsparam_flag("filestreams",	Opt_filestreams),
> > > +	fsparam_flag("quota",		Opt_quota),
> > > +	fsparam_flag("noquota",		Opt_noquota),
> > > +	fsparam_flag("usrquota",	Opt_usrquota),
> > > +	fsparam_flag("grpquota",	Opt_grpquota),
> > > +	fsparam_flag("prjquota",	Opt_prjquota),
> > > +	fsparam_flag("uquota",		Opt_uquota),
> > > +	fsparam_flag("gquota",		Opt_gquota),
> > > +	fsparam_flag("pquota",		Opt_pquota),
> > > +	fsparam_flag("uqnoenforce",	Opt_uqnoenforce),
> > > +	fsparam_flag("gqnoenforce",	Opt_gqnoenforce),
> > > +	fsparam_flag("pqnoenforce",	Opt_pqnoenforce),
> > > +	fsparam_flag("qnoenforce",	Opt_qnoenforce),
> > > +	fsparam_flag("discard",		Opt_discard),
> > > +	fsparam_flag("nodiscard",	Opt_nodiscard),
> > > +	fsparam_flag("dax",		Opt_dax),
> > > +	{}
> > >  };
> > >  
> > > +static const struct fs_parameter_description xfs_fs_parameters = {
> > > +	.name		= "xfs",
> > > +	.specs		= xfs_param_specs,
> > > +};
> > >  
> > >  static int
> > >  suffix_kstrtoint(
> > > @@ -143,55 +146,42 @@ suffix_kstrtoint(
> > >  	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;
> > > -}
> > > -
> > >  static int
> > >  xfs_fc_parse_param(
> > 
> > This is the only function where mp->m_super can be NULL, correct?
> 
> Yep.
> 
> > 
> > If so, I'll add a comment to that effect, and,
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Thanks, much appreciated.
> 
> > 
> > (for this and patch 12.)
> > 
> > Will throw this at a testing cloud and see what happens...
> 
> It's seems ok when I run it against xfstests but I'm not as familiar
> with what to expect as you would be, I'm interested to hear how it
> goes, ;)

Looked like it ran just fine.

> I noticed a significant change in test run times with the latest
> for-next but there was also an xfstests update so I'm not sure
> what caused it. Some test run times went up by 8 to 10 or more
> times, which seemed like a big change, possibly a regression.

Hmm, I haven't seen any such regression; it still takes ~3h to run all
the non-dangerous tests.

--D

> Ian
> 
> > 
> > --D
> > 
> > > -	int			token,
> > > -	char			*p,
> > > -	substring_t		*args,
> > > -	struct xfs_mount	*mp)
> > > +	struct fs_context	*fc,
> > > +	struct fs_parameter	*param)
> > >  {
> > > +	struct xfs_mount	*mp = fc->s_fs_info;
> > > +	struct fs_parse_result	result;
> > >  	int			size = 0;
> > > +	int			opt;
> > > +
> > > +	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> > > +	if (opt < 0)
> > > +		return opt;
> > >  
> > > -	switch (token) {
> > > +	switch (opt) {
> > >  	case Opt_logbufs:
> > > -		if (match_int(args, &mp->m_logbufs))
> > > -			return -EINVAL;
> > > +		mp->m_logbufs = result.uint_32;
> > >  		return 0;
> > >  	case Opt_logbsize:
> > > -		if (match_kstrtoint(args, 10, &mp->m_logbsize))
> > > +		if (suffix_kstrtoint(param->string, 10, &mp-
> > > >m_logbsize))
> > >  			return -EINVAL;
> > >  		return 0;
> > >  	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;
> > >  		return 0;
> > >  	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;
> > >  		return 0;
> > >  	case Opt_allocsize:
> > > -		if (match_kstrtoint(args, 10, &size))
> > > +		if (suffix_kstrtoint(param->string, 10, &size))
> > >  			return -EINVAL;
> > >  		mp->m_allocsize_log = ffs(size) - 1;
> > >  		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
> > > @@ -217,12 +207,10 @@ xfs_fc_parse_param(
> > >  		mp->m_flags |= XFS_MOUNT_SWALLOC;
> > >  		return 0;
> > >  	case Opt_sunit:
> > > -		if (match_int(args, &mp->m_dalign))
> > > -			return -EINVAL;
> > > +		mp->m_dalign = result.uint_32;
> > >  		return 0;
> > >  	case Opt_swidth:
> > > -		if (match_int(args, &mp->m_swidth))
> > > -			return -EINVAL;
> > > +		mp->m_swidth = result.uint_32;
> > >  		return 0;
> > >  	case Opt_inode32:
> > >  		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> > > @@ -301,7 +289,7 @@ xfs_fc_parse_param(
> > >  		return 0;
> > >  #endif
> > >  	default:
> > > -		xfs_warn(mp, "unknown mount option [%s].", p);
> > > +		xfs_warn(mp, "unknown mount option [%s].", param->key);
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > @@ -376,62 +364,6 @@ xfs_fc_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;
> > > -	substring_t		args[MAX_OPT_ARGS];
> > > -
> > > -	/*
> > > -	 * 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;
> > > -
> > > -	/*
> > > -	 * These can be overridden by the mount option parsing.
> > > -	 */
> > > -	mp->m_logbufs = -1;
> > > -	mp->m_logbsize = -1;
> > > -	mp->m_allocsize_log = 16; /* 64k */
> > > -
> > > -	if (!options)
> > > -		return 0;
> > > -
> > > -	while ((p = strsep(&options, ",")) != NULL) {
> > > -		int		token;
> > > -		int		ret;
> > > -
> > > -		if (!*p)
> > > -			continue;
> > > -
> > > -		token = match_token(p, tokens, args);
> > > -		ret = xfs_fc_parse_param(token, p, args, mp);
> > > -		if (ret)
> > > -			return ret;
> > > -	}
> > > -
> > > -	return xfs_fc_validate_params(mp);
> > > -}
> > > -
> > >  struct proc_xfs_info {
> > >  	uint64_t	flag;
> > >  	char		*str;
> > > @@ -1207,25 +1139,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_mount_free(tmp_mp);
> > > -
> > > -	return error;
> > > -}
> > > -
> > >  static int
> > >  xfs_remount_rw(
> > >  	struct xfs_mount	*mp)
> > > @@ -1329,76 +1242,57 @@ xfs_remount_ro(
> > >  	return 0;
> > >  }
> > >  
> > > -STATIC int
> > > -xfs_fs_remount(
> > > -	struct super_block	*sb,
> > > -	int			*flags,
> > > -	char			*options)
> > > +/*
> > > + * 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.
> > > + */
> > > +static int
> > > +xfs_fc_reconfigure(
> > > +	struct fs_context	*fc)
> > >  {
> > > -	struct xfs_mount	*mp = XFS_M(sb);
> > > +	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;
> > > -	substring_t		args[MAX_OPT_ARGS];
> > > -	char			*p;
> > > +	int			flags = fc->sb_flags;
> > >  	int			error;
> > >  
> > > -	/* First, check for complete junk; i.e. invalid options */
> > > -	error = xfs_test_remount_options(sb, options);
> > > +	error = xfs_fc_validate_params(new_mp);
> > >  	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
> > > -		}
> > > +	sync_filesystem(mp->m_super);
> > > +
> > > +	/* 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))
> > > {
> > > +	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))
> > > {
> > > +	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
> > >  		error = xfs_remount_ro(mp);
> > >  		if (error)
> > >  			return error;
> > > @@ -1588,28 +1482,18 @@ xfs_mount_alloc(void)
> > >  	return mp;
> > >  }
> > >  
> > > -
> > > -STATIC int
> > > -xfs_fs_fill_super(
> > > +static int
> > > +xfs_fc_fill_super(
> > >  	struct super_block	*sb,
> > > -	void			*data,
> > > -	int			silent)
> > > +	struct fs_context	*fc)
> > >  {
> > > +	struct xfs_mount	*mp = sb->s_fs_info;
> > >  	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();
> > > -	if (!mp)
> > > -		goto out;
> > >  	mp->m_super = sb;
> > > -	sb->s_fs_info = mp;
> > >  
> > > -	error = xfs_parseargs(mp, (char *)data);
> > > +	error = xfs_fc_validate_params(mp);
> > >  	if (error)
> > >  		goto out_free_names;
> > >  
> > > @@ -1633,7 +1517,7 @@ xfs_fs_fill_super(
> > >  		msleep(xfs_globals.mount_delay * 1000);
> > >  	}
> > >  
> > > -	if (silent)
> > > +	if (fc->sb_flags & SB_SILENT)
> > >  		flags |= XFS_MFSI_QUIET;
> > >  
> > >  	error = xfs_open_devices(mp);
> > > @@ -1778,7 +1662,6 @@ xfs_fs_fill_super(
> > >   out_free_names:
> > >  	sb->s_fs_info = NULL;
> > >  	xfs_mount_free(mp);
> > > - out:
> > >  	return error;
> > >  
> > >   out_unmount:
> > > @@ -1787,6 +1670,13 @@ xfs_fs_fill_super(
> > >  	goto out_free_sb;
> > >  }
> > >  
> > > +static int
> > > +xfs_fc_get_tree(
> > > +	struct fs_context	*fc)
> > > +{
> > > +	return get_tree_bdev(fc, xfs_fc_fill_super);
> > > +}
> > > +
> > >  STATIC void
> > >  xfs_fs_put_super(
> > >  	struct super_block	*sb)
> > > @@ -1811,16 +1701,6 @@ xfs_fs_put_super(
> > >  	xfs_mount_free(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,
> > > @@ -1850,16 +1730,70 @@ 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,
> > >  };
> > >  
> > > +static void xfs_fc_free(
> > > +	struct fs_context	*fc)
> > > +{
> > > +	struct xfs_mount	*mp = fc->s_fs_info;
> > > +
> > > +	/*
> > > +	 * mp is stored in the fs_context when it is initialized.
> > > +	 * mp is transferred to the superblock on a successful mount,
> > > +	 * but if an error occurs before the transfer we have to free
> > > +	 * it here.
> > > +	 */
> > > +	if (mp)
> > > +		xfs_mount_free(mp);
> > > +}
> > > +
> > > +static const struct fs_context_operations xfs_context_ops = {
> > > +	.parse_param = xfs_fc_parse_param,
> > > +	.get_tree    = xfs_fc_get_tree,
> > > +	.reconfigure = xfs_fc_reconfigure,
> > > +	.free        = xfs_fc_free,
> > > +};
> > > +
> > > +static int xfs_init_fs_context(
> > > +	struct fs_context	*fc)
> > > +{
> > > +	struct xfs_mount	*mp;
> > > +
> > > +	mp = xfs_mount_alloc();
> > > +	if (!mp)
> > > +		return -ENOMEM;
> > > +
> > > +	/*
> > > +	 * These can be overridden by the mount option parsing.
> > > +	 */
> > > +	mp->m_logbufs = -1;
> > > +	mp->m_logbsize = -1;
> > > +	mp->m_allocsize_log = 16; /* 64k */
> > > +
> > > +	/*
> > > +	 * 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->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	[flat|nested] 25+ messages in thread

* Re: [PATCH v9 13/17] xfs: switch to use the new mount-api
  2019-11-05 15:23       ` Darrick J. Wong
@ 2019-11-06  2:27         ` Ian Kent
  0 siblings, 0 replies; 25+ messages in thread
From: Ian Kent @ 2019-11-06  2:27 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: linux-xfs, Christoph Hellwig, Brian Foster, Eric Sandeen,
	David Howells, Dave Chinner, Al Viro

On Tue, 2019-11-05 at 07:23 -0800, Darrick J. Wong wrote:
> On Tue, Nov 05, 2019 at 11:00:36AM +0800, Ian Kent wrote:
> > On Mon, 2019-11-04 at 14:53 -0800, Darrick J. Wong wrote:
> > > On Mon, Nov 04, 2019 at 06:55:44PM +0800, Ian Kent wrote:
> > > > Define the struct fs_parameter_spec table that's used by the
> > > > new
> > > > mount-api for options parsing.
> > > > 
> > > > Create the various fs context operations methods and define the
> > > > fs_context_operations struct.
> > > > 
> > > > Create the fs context initialization method and update the
> > > > struct
> > > > file_system_type to utilize it. The initialization function is
> > > > responsible for working storage initialization, allocation and
> > > > initialization of file system private information storage and
> > > > for
> > > > setting the operations in the fs context.
> > > > 
> > > > Also set struct file_system_type .parameters to the newly
> > > > defined
> > > > struct fs_parameter_spec options parsing table for use by the
> > > > fs
> > > > context methods and remove unused code.
> > > > 
> > > > Signed-off-by: Ian Kent <raven@themaw.net>
> > > > ---
> > > >  fs/xfs/xfs_super.c |  408 ++++++++++++++++++++++------------
> > > > ----
> > > > --------------
> > > >  1 file changed, 171 insertions(+), 237 deletions(-)
> > > > 
> > > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > > index 62dfc678c415..2d4edaaf934e 100644
> > > > --- a/fs/xfs/xfs_super.c
> > > > +++ b/fs/xfs/xfs_super.c
> > > > @@ -37,7 +37,8 @@
> > > >  #include "xfs_reflink.h"
> > > >  
> > > >  #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;
> > > >  
> > > > @@ -58,55 +59,57 @@ 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,
> > > > +	Opt_discard, Opt_nodiscard, Opt_dax,
> > > >  };
> > > >  
> > > > -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},
> > > > +static const struct fs_parameter_spec xfs_param_specs[] = {
> > > > +	fsparam_u32("logbufs",		Opt_logbufs),
> > > > +	fsparam_string("logbsize",	Opt_logbsize),
> > > > +	fsparam_string("logdev",	Opt_logdev),
> > > > +	fsparam_string("rtdev",		Opt_rtdev),
> > > > +	fsparam_flag("wsync",		Opt_wsync),
> > > > +	fsparam_flag("noalign",		Opt_noalign),
> > > > +	fsparam_flag("swalloc",		Opt_swalloc),
> > > > +	fsparam_u32("sunit",		Opt_sunit),
> > > > +	fsparam_u32("swidth",		Opt_swidth),
> > > > +	fsparam_flag("nouuid",		Opt_nouuid),
> > > > +	fsparam_flag("grpid",		Opt_grpid),
> > > > +	fsparam_flag("nogrpid",		Opt_nogrpid),
> > > > +	fsparam_flag("bsdgroups",	Opt_bsdgroups),
> > > > +	fsparam_flag("sysvgroups",	Opt_sysvgroups),
> > > > +	fsparam_string("allocsize",	Opt_allocsize),
> > > > +	fsparam_flag("norecovery",	Opt_norecovery),
> > > > +	fsparam_flag("inode64",		Opt_inode64),
> > > > +	fsparam_flag("inode32",		Opt_inode32),
> > > > +	fsparam_flag("ikeep",		Opt_ikeep),
> > > > +	fsparam_flag("noikeep",		Opt_noikeep),
> > > > +	fsparam_flag("largeio",		Opt_largeio),
> > > > +	fsparam_flag("nolargeio",	Opt_nolargeio),
> > > > +	fsparam_flag("attr2",		Opt_attr2),
> > > > +	fsparam_flag("noattr2",		Opt_noattr2),
> > > > +	fsparam_flag("filestreams",	Opt_filestreams),
> > > > +	fsparam_flag("quota",		Opt_quota),
> > > > +	fsparam_flag("noquota",		Opt_noquota),
> > > > +	fsparam_flag("usrquota",	Opt_usrquota),
> > > > +	fsparam_flag("grpquota",	Opt_grpquota),
> > > > +	fsparam_flag("prjquota",	Opt_prjquota),
> > > > +	fsparam_flag("uquota",		Opt_uquota),
> > > > +	fsparam_flag("gquota",		Opt_gquota),
> > > > +	fsparam_flag("pquota",		Opt_pquota),
> > > > +	fsparam_flag("uqnoenforce",	Opt_uqnoenforce),
> > > > +	fsparam_flag("gqnoenforce",	Opt_gqnoenforce),
> > > > +	fsparam_flag("pqnoenforce",	Opt_pqnoenforce),
> > > > +	fsparam_flag("qnoenforce",	Opt_qnoenforce),
> > > > +	fsparam_flag("discard",		Opt_discard),
> > > > +	fsparam_flag("nodiscard",	Opt_nodiscard),
> > > > +	fsparam_flag("dax",		Opt_dax),
> > > > +	{}
> > > >  };
> > > >  
> > > > +static const struct fs_parameter_description xfs_fs_parameters
> > > > = {
> > > > +	.name		= "xfs",
> > > > +	.specs		= xfs_param_specs,
> > > > +};
> > > >  
> > > >  static int
> > > >  suffix_kstrtoint(
> > > > @@ -143,55 +146,42 @@ suffix_kstrtoint(
> > > >  	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;
> > > > -}
> > > > -
> > > >  static int
> > > >  xfs_fc_parse_param(
> > > 
> > > This is the only function where mp->m_super can be NULL, correct?
> > 
> > Yep.
> > 
> > > If so, I'll add a comment to that effect, and,
> > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Thanks, much appreciated.
> > 
> > > (for this and patch 12.)
> > > 
> > > Will throw this at a testing cloud and see what happens...
> > 
> > It's seems ok when I run it against xfstests but I'm not as
> > familiar
> > with what to expect as you would be, I'm interested to hear how it
> > goes, ;)
> 
> Looked like it ran just fine.
> 
> > I noticed a significant change in test run times with the latest
> > for-next but there was also an xfstests update so I'm not sure
> > what caused it. Some test run times went up by 8 to 10 or more
> > times, which seemed like a big change, possibly a regression.
> 
> Hmm, I haven't seen any such regression; it still takes ~3h to run
> all
> the non-dangerous tests.

Ok, it must be my setup then.
Thought I'd mention it just in case there was something wrong.

Ian
> 
> --D
> 
> > Ian
> > 
> > > --D
> > > 
> > > > -	int			token,
> > > > -	char			*p,
> > > > -	substring_t		*args,
> > > > -	struct xfs_mount	*mp)
> > > > +	struct fs_context	*fc,
> > > > +	struct fs_parameter	*param)
> > > >  {
> > > > +	struct xfs_mount	*mp = fc->s_fs_info;
> > > > +	struct fs_parse_result	result;
> > > >  	int			size = 0;
> > > > +	int			opt;
> > > > +
> > > > +	opt = fs_parse(fc, &xfs_fs_parameters, param, &result);
> > > > +	if (opt < 0)
> > > > +		return opt;
> > > >  
> > > > -	switch (token) {
> > > > +	switch (opt) {
> > > >  	case Opt_logbufs:
> > > > -		if (match_int(args, &mp->m_logbufs))
> > > > -			return -EINVAL;
> > > > +		mp->m_logbufs = result.uint_32;
> > > >  		return 0;
> > > >  	case Opt_logbsize:
> > > > -		if (match_kstrtoint(args, 10, &mp->m_logbsize))
> > > > +		if (suffix_kstrtoint(param->string, 10, &mp-
> > > > > m_logbsize))
> > > >  			return -EINVAL;
> > > >  		return 0;
> > > >  	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;
> > > >  		return 0;
> > > >  	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;
> > > >  		return 0;
> > > >  	case Opt_allocsize:
> > > > -		if (match_kstrtoint(args, 10, &size))
> > > > +		if (suffix_kstrtoint(param->string, 10, &size))
> > > >  			return -EINVAL;
> > > >  		mp->m_allocsize_log = ffs(size) - 1;
> > > >  		mp->m_flags |= XFS_MOUNT_ALLOCSIZE;
> > > > @@ -217,12 +207,10 @@ xfs_fc_parse_param(
> > > >  		mp->m_flags |= XFS_MOUNT_SWALLOC;
> > > >  		return 0;
> > > >  	case Opt_sunit:
> > > > -		if (match_int(args, &mp->m_dalign))
> > > > -			return -EINVAL;
> > > > +		mp->m_dalign = result.uint_32;
> > > >  		return 0;
> > > >  	case Opt_swidth:
> > > > -		if (match_int(args, &mp->m_swidth))
> > > > -			return -EINVAL;
> > > > +		mp->m_swidth = result.uint_32;
> > > >  		return 0;
> > > >  	case Opt_inode32:
> > > >  		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> > > > @@ -301,7 +289,7 @@ xfs_fc_parse_param(
> > > >  		return 0;
> > > >  #endif
> > > >  	default:
> > > > -		xfs_warn(mp, "unknown mount option [%s].", p);
> > > > +		xfs_warn(mp, "unknown mount option [%s].",
> > > > param->key);
> > > >  		return -EINVAL;
> > > >  	}
> > > >  
> > > > @@ -376,62 +364,6 @@ xfs_fc_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;
> > > > -	substring_t		args[MAX_OPT_ARGS];
> > > > -
> > > > -	/*
> > > > -	 * 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;
> > > > -
> > > > -	/*
> > > > -	 * These can be overridden by the mount option parsing.
> > > > -	 */
> > > > -	mp->m_logbufs = -1;
> > > > -	mp->m_logbsize = -1;
> > > > -	mp->m_allocsize_log = 16; /* 64k */
> > > > -
> > > > -	if (!options)
> > > > -		return 0;
> > > > -
> > > > -	while ((p = strsep(&options, ",")) != NULL) {
> > > > -		int		token;
> > > > -		int		ret;
> > > > -
> > > > -		if (!*p)
> > > > -			continue;
> > > > -
> > > > -		token = match_token(p, tokens, args);
> > > > -		ret = xfs_fc_parse_param(token, p, args, mp);
> > > > -		if (ret)
> > > > -			return ret;
> > > > -	}
> > > > -
> > > > -	return xfs_fc_validate_params(mp);
> > > > -}
> > > > -
> > > >  struct proc_xfs_info {
> > > >  	uint64_t	flag;
> > > >  	char		*str;
> > > > @@ -1207,25 +1139,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_mount_free(tmp_mp);
> > > > -
> > > > -	return error;
> > > > -}
> > > > -
> > > >  static int
> > > >  xfs_remount_rw(
> > > >  	struct xfs_mount	*mp)
> > > > @@ -1329,76 +1242,57 @@ xfs_remount_ro(
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -STATIC int
> > > > -xfs_fs_remount(
> > > > -	struct super_block	*sb,
> > > > -	int			*flags,
> > > > -	char			*options)
> > > > +/*
> > > > + * 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.
> > > > + */
> > > > +static int
> > > > +xfs_fc_reconfigure(
> > > > +	struct fs_context	*fc)
> > > >  {
> > > > -	struct xfs_mount	*mp = XFS_M(sb);
> > > > +	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;
> > > > -	substring_t		args[MAX_OPT_ARGS];
> > > > -	char			*p;
> > > > +	int			flags = fc->sb_flags;
> > > >  	int			error;
> > > >  
> > > > -	/* First, check for complete junk; i.e. invalid options
> > > > */
> > > > -	error = xfs_test_remount_options(sb, options);
> > > > +	error = xfs_fc_validate_params(new_mp);
> > > >  	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
> > > > -		}
> > > > +	sync_filesystem(mp->m_super);
> > > > +
> > > > +	/* 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))
> > > > {
> > > > +	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))
> > > > {
> > > > +	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags &
> > > > SB_RDONLY)) {
> > > >  		error = xfs_remount_ro(mp);
> > > >  		if (error)
> > > >  			return error;
> > > > @@ -1588,28 +1482,18 @@ xfs_mount_alloc(void)
> > > >  	return mp;
> > > >  }
> > > >  
> > > > -
> > > > -STATIC int
> > > > -xfs_fs_fill_super(
> > > > +static int
> > > > +xfs_fc_fill_super(
> > > >  	struct super_block	*sb,
> > > > -	void			*data,
> > > > -	int			silent)
> > > > +	struct fs_context	*fc)
> > > >  {
> > > > +	struct xfs_mount	*mp = sb->s_fs_info;
> > > >  	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();
> > > > -	if (!mp)
> > > > -		goto out;
> > > >  	mp->m_super = sb;
> > > > -	sb->s_fs_info = mp;
> > > >  
> > > > -	error = xfs_parseargs(mp, (char *)data);
> > > > +	error = xfs_fc_validate_params(mp);
> > > >  	if (error)
> > > >  		goto out_free_names;
> > > >  
> > > > @@ -1633,7 +1517,7 @@ xfs_fs_fill_super(
> > > >  		msleep(xfs_globals.mount_delay * 1000);
> > > >  	}
> > > >  
> > > > -	if (silent)
> > > > +	if (fc->sb_flags & SB_SILENT)
> > > >  		flags |= XFS_MFSI_QUIET;
> > > >  
> > > >  	error = xfs_open_devices(mp);
> > > > @@ -1778,7 +1662,6 @@ xfs_fs_fill_super(
> > > >   out_free_names:
> > > >  	sb->s_fs_info = NULL;
> > > >  	xfs_mount_free(mp);
> > > > - out:
> > > >  	return error;
> > > >  
> > > >   out_unmount:
> > > > @@ -1787,6 +1670,13 @@ xfs_fs_fill_super(
> > > >  	goto out_free_sb;
> > > >  }
> > > >  
> > > > +static int
> > > > +xfs_fc_get_tree(
> > > > +	struct fs_context	*fc)
> > > > +{
> > > > +	return get_tree_bdev(fc, xfs_fc_fill_super);
> > > > +}
> > > > +
> > > >  STATIC void
> > > >  xfs_fs_put_super(
> > > >  	struct super_block	*sb)
> > > > @@ -1811,16 +1701,6 @@ xfs_fs_put_super(
> > > >  	xfs_mount_free(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,
> > > > @@ -1850,16 +1730,70 @@ 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,
> > > >  };
> > > >  
> > > > +static void xfs_fc_free(
> > > > +	struct fs_context	*fc)
> > > > +{
> > > > +	struct xfs_mount	*mp = fc->s_fs_info;
> > > > +
> > > > +	/*
> > > > +	 * mp is stored in the fs_context when it is
> > > > initialized.
> > > > +	 * mp is transferred to the superblock on a successful
> > > > mount,
> > > > +	 * but if an error occurs before the transfer we have
> > > > to free
> > > > +	 * it here.
> > > > +	 */
> > > > +	if (mp)
> > > > +		xfs_mount_free(mp);
> > > > +}
> > > > +
> > > > +static const struct fs_context_operations xfs_context_ops = {
> > > > +	.parse_param = xfs_fc_parse_param,
> > > > +	.get_tree    = xfs_fc_get_tree,
> > > > +	.reconfigure = xfs_fc_reconfigure,
> > > > +	.free        = xfs_fc_free,
> > > > +};
> > > > +
> > > > +static int xfs_init_fs_context(
> > > > +	struct fs_context	*fc)
> > > > +{
> > > > +	struct xfs_mount	*mp;
> > > > +
> > > > +	mp = xfs_mount_alloc();
> > > > +	if (!mp)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	/*
> > > > +	 * These can be overridden by the mount option parsing.
> > > > +	 */
> > > > +	mp->m_logbufs = -1;
> > > > +	mp->m_logbsize = -1;
> > > > +	mp->m_allocsize_log = 16; /* 64k */
> > > > +
> > > > +	/*
> > > > +	 * 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->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	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2019-11-06  2:27 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 10:54 [PATCH v9 00/17] xfs: mount API patch series Ian Kent
2019-11-04 10:54 ` [PATCH v9 01/17] xfs: remove unused struct xfs_mount field m_fsname_len Ian Kent
2019-11-04 10:54 ` [PATCH v9 02/17] xfs: use super s_id instead of struct xfs_mount m_fsname Ian Kent
2019-11-04 10:54 ` [PATCH v9 03/17] xfs: dont use XFS_IS_QUOTA_RUNNING() for option check Ian Kent
2019-11-04 10:54 ` [PATCH v9 04/17] xfs: use kmem functions for struct xfs_mount Ian Kent
2019-11-04 10:55 ` [PATCH v9 05/17] xfs: merge freeing of mp names and mp Ian Kent
2019-11-04 10:55 ` [PATCH v9 06/17] xfs: add xfs_remount_rw() helper Ian Kent
2019-11-04 10:55 ` [PATCH v9 07/17] xfs: add xfs_remount_ro() helper Ian Kent
2019-11-04 10:55 ` [PATCH v9 08/17] xfs: refactor suffix_kstrtoint() Ian Kent
2019-11-04 10:55 ` [PATCH v9 09/17] xfs: avoid redundant checks when options is empty Ian Kent
2019-11-04 10:55 ` [PATCH v9 10/17] xfs: refactor xfs_parseags() Ian Kent
2019-11-04 10:55 ` [PATCH v9 11/17] xfs: move xfs_parseargs() validation to a helper Ian Kent
2019-11-04 10:55 ` [PATCH v9 12/17] xfs: dont set sb in xfs_mount_alloc() Ian Kent
2019-11-04 10:55 ` [PATCH v9 13/17] xfs: switch to use the new mount-api Ian Kent
2019-11-04 15:14   ` Christoph Hellwig
2019-11-04 22:53   ` Darrick J. Wong
2019-11-05  3:00     ` Ian Kent
2019-11-05 15:23       ` Darrick J. Wong
2019-11-06  2:27         ` Ian Kent
2019-11-04 10:55 ` [PATCH v9 14/17] xfs: move xfs_fc_reconfigure() above xfs_fc_free() Ian Kent
2019-11-04 10:55 ` [PATCH v9 15/17] xfs: move xfs_fc_get_tree() above xfs_fc_reconfigure() Ian Kent
2019-11-04 10:56 ` [PATCH v9 16/17] xfs: move xfs_fc_parse_param() above xfs_fc_get_tree() Ian Kent
2019-11-04 10:56 ` [PATCH v9 17/17] xfs: fold xfs_mount-alloc() into xfs_init_fs_context() Ian Kent
2019-11-04 15:14   ` Christoph Hellwig
2019-11-04 20:55   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.