[PATCH v7 00/17] xfs: mount API patch series

[PATCH v7 00/17] xfs: mount API patch series

[Ian Kent]

This patch series add support to xfs for the new kernel mount API
as described in the LWN article at https://lwn.net/Articles/780267/.

In the article there's a lengthy description of the reasons for
adopting the API and problems expected to be resolved by using it.

The series has been applied to the repository located at
git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git, and built and
some simple tests run on it along with the generic xfstests.

Note: the patches "xfs: add xfs_remount_rw() helper" and
 "xfs: add xfs_remount_ro() helper" that have Reviewed-by attributions
 each needed a forward declartion added due grouping all the mount
 related code together. Reviewers may want to check the attribution
 is still acceptable.

The first patch in the series is needed only for testing. It is now
present in the current upstream rc kernel so it will need to be dropped
before merging.

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 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 initialisation in xfs_parseargs().
- move call to xfs_validate_params() below label "done".
- if allocation of xfs_mount fails return ENOMEM immediately.
- remove erroneous kfree(mp) in xfs_fill_super().
- move the removal of xfs_fs_remount() and xfs_test_remount_options()
  to the switch to mount api patch.
- retain original usage of distinct <option>, no<option> usage.
- fix line length and a white space problem in xfs_parseargs().
- defer introduction of struct fs_context_operations until mount
  api implementation.
- don't use a new line for the void parameter of xfs_mount_alloc().
- check for -ENOPARAM in xfs_parse_param() to report invalid options
  using the options switch (to avoid double entry log messages).
- remove obsolete 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.
- clarify 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 manageable.
---

Ian Kent (17):
      vfs: add missing blkdev_put() in get_tree_bdev()
      xfs: remove very old mount option
      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: move xfs_mount_alloc to be with parsing code
      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: move xfs_fs_fill_super() to be with parsing code
      xfs: switch to use the new mount-api


 fs/super.c             |    5 
 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     | 1607 ++++++++++++++++++++++++------------------------
 fs/xfs/xfs_trans_ail.c |    2 
 9 files changed, 824 insertions(+), 807 deletions(-)

--
Ian

[PATCH v7 01/17] vfs: add missing blkdev_put() in get_tree_bdev()

[Ian Kent]

There appear to be a couple of missing blkdev_put() in get_tree_bdev().
This change has been sent to Linus and has been included in the current
rc kernel but is not present in the xfs-linux tree.
---
 fs/super.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index f627b7c53d2b..cfadab2cbf35 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1300,6 +1300,7 @@ int get_tree_bdev(struct fs_context *fc,
 	mutex_lock(&bdev->bd_fsfreeze_mutex);
 	if (bdev->bd_fsfreeze_count > 0) {
 		mutex_unlock(&bdev->bd_fsfreeze_mutex);
+		blkdev_put(bdev, mode);
 		warnf(fc, "%pg: Can't mount, blockdev is frozen", bdev);
 		return -EBUSY;
 	}
@@ -1308,8 +1309,10 @@ int get_tree_bdev(struct fs_context *fc,
 	fc->sget_key = bdev;
 	s = sget_fc(fc, test_bdev_super_fc, set_bdev_super_fc);
 	mutex_unlock(&bdev->bd_fsfreeze_mutex);
-	if (IS_ERR(s))
+	if (IS_ERR(s)) {
+		blkdev_put(bdev, mode);
 		return PTR_ERR(s);
+	}
 
 	if (s->s_root) {
 		/* Don't summarily change the RO/RW state. */

[PATCH v7 02/17] xfs: remove very old mount option

[Ian Kent]

It appears the biosize mount option hasn't been documented as a valid
option since 2005, remove it.

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 |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 8d1df9f8be07..1bb7ede5d75b 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -51,7 +51,7 @@ static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
  * Table driven mount option parser.
  */
 enum {
-	Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev, Opt_biosize,
+	Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev,
 	Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, Opt_nouuid,
 	Opt_grpid, Opt_nogrpid, Opt_bsdgroups, Opt_sysvgroups,
 	Opt_allocsize, Opt_norecovery, Opt_inode64, Opt_inode32, Opt_ikeep,
@@ -67,7 +67,6 @@ static const match_table_t tokens = {
 	{Opt_logbsize,	"logbsize=%s"},	/* size of XFS log buffers */
 	{Opt_logdev,	"logdev=%s"},	/* log device */
 	{Opt_rtdev,	"rtdev=%s"},	/* realtime I/O device */
-	{Opt_biosize,	"biosize=%u"},	/* log2 of preferred buffered io size */
 	{Opt_wsync,	"wsync"},	/* safe-mode nfs compatible mount */
 	{Opt_noalign,	"noalign"},	/* turn off stripe alignment */
 	{Opt_swalloc,	"swalloc"},	/* turn on stripe width allocation */
@@ -229,7 +228,6 @@ xfs_parseargs(
 				return -ENOMEM;
 			break;
 		case Opt_allocsize:
-		case Opt_biosize:
 			if (suffix_kstrtoint(args, 10, &iosize))
 				return -EINVAL;
 			iosizelog = ffs(iosize) - 1;

[PATCH v7 03/17] xfs: remove unused struct xfs_mount field m_fsname_len

[Ian Kent]

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 fdb60e09a9c5..b3230f7ca2bf 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 1bb7ede5d75b..cfa306f62bec 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -172,7 +172,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.

[PATCH v7 04/17] xfs: use super s_id instead of struct xfs_mount m_fsname

[Ian Kent]

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 a2beee9f74da..ed99f4f063c3 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1479,7 +1479,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..aaea6faf5acc 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 && mp->m_super->s_id) {
+		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 ba5b6f3b2b88..b6145a70a593 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -706,7 +706,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;
 
@@ -1277,7 +1278,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 b3230f7ca2bf..f2a9f316fa6c 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 a339bd5fa260..81df10f69ade 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -59,7 +59,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 cfa306f62bec..5876c2b551b5 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -165,14 +165,6 @@ xfs_parseargs(
 	int			iosize = 0;
 	uint8_t			iosizelog = 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.
 	 */
@@ -805,33 +797,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;
 
@@ -1036,10 +1028,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);
 }
@@ -1216,7 +1207,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;
@@ -1591,7 +1582,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;
@@ -1618,7 +1609,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)
@@ -1755,9 +1746,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;
@@ -1789,7 +1780,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;
 

[PATCH v7 05/17] xfs: dont use XFS_IS_QUOTA_RUNNING() for option check

[Ian Kent]

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: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_super.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 5876c2b551b5..a0805b74256c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -348,8 +348,8 @@ xfs_parseargs(
 		return -EINVAL;
 	}
 
-#ifndef CONFIG_XFS_QUOTA
-	if (XFS_IS_QUOTA_RUNNING(mp)) {
+#if !IS_ENABLED(CONFIG_XFS_QUOTA)
+	if (mp->m_qflags != 0) {
 		xfs_warn(mp, "quota support not available in this kernel.");
 		return -EINVAL;
 	}

Re: [PATCH v7 05/17] xfs: dont use XFS_IS_QUOTA_RUNNING() for option check

[Christoph Hellwig]

On Thu, Oct 24, 2019 at 03:51:01PM +0800, Ian Kent wrote:
> -#ifndef CONFIG_XFS_QUOTA
> -	if (XFS_IS_QUOTA_RUNNING(mp)) {
> +#if !IS_ENABLED(CONFIG_XFS_QUOTA)
> +	if (mp->m_qflags != 0) {

The whole point of IS_ENABLED is that you can directly use it in C code
instead of only through the preprocessor.  So this should be:

	if (!IS_ENABLED(CONFIG_XFS_QUOTA) && mp->m_qflags != 0) {

[PATCH v7 06/17] xfs: use kmem functions for struct xfs_mount

[Ian Kent]

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>
---
 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 a0805b74256c..896609827e3c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1535,7 +1535,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;
 
@@ -1749,7 +1749,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;
 
@@ -1781,7 +1781,7 @@ xfs_fs_put_super(
 
 	sb->s_fs_info = NULL;
 	xfs_free_names(mp);
-	kfree(mp);
+	kmem_free(mp);
 }
 
 STATIC struct dentry *

Re: [PATCH v7 06/17] xfs: use kmem functions for struct xfs_mount

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:06PM +0800, Ian Kent wrote:
> 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>

Looks fine (there are direct callers of kmalloc for anyone who wants to
take on a small cleanup...)

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

--D

> ---
>  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 a0805b74256c..896609827e3c 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1535,7 +1535,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;
>  
> @@ -1749,7 +1749,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;
>  
> @@ -1781,7 +1781,7 @@ xfs_fs_put_super(
>  
>  	sb->s_fs_info = NULL;
>  	xfs_free_names(mp);
> -	kfree(mp);
> +	kmem_free(mp);
>  }
>  
>  STATIC struct dentry *
> 

Re: [PATCH v7 06/17] xfs: use kmem functions for struct xfs_mount

[Christoph Hellwig]

On Thu, Oct 24, 2019 at 03:51:06PM +0800, Ian Kent wrote:
> 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>

Looks good,

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

[PATCH v7 07/17] xfs: move xfs_mount_alloc to be with parsing code

[Ian Kent]

The struct xfs_mount allocation (and freeing) is only used by the mount
code, so move xfs_mount_alloc() to the same area as the option handling
code (as part of the work to locate the mount code together).

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

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 896609827e3c..0596d491dbbe 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -415,6 +415,37 @@ xfs_parseargs(
 	return 0;
 }
 
+static struct xfs_mount *
+xfs_mount_alloc(
+	struct super_block	*sb)
+{
+	struct xfs_mount	*mp;
+
+	mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
+	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);
+	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;
+}
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -1529,38 +1560,6 @@ xfs_destroy_percpu_counters(
 	percpu_counter_destroy(&mp->m_delalloc_blks);
 }
 
-static struct xfs_mount *
-xfs_mount_alloc(
-	struct super_block	*sb)
-{
-	struct xfs_mount	*mp;
-
-	mp = kmem_alloc(sizeof(struct xfs_mount), KM_ZERO);
-	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);
-	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
 xfs_fs_fill_super(
 	struct super_block	*sb,

Re: [PATCH v7 07/17] xfs: move xfs_mount_alloc to be with parsing code

[Christoph Hellwig]

On Thu, Oct 24, 2019 at 03:51:12PM +0800, Ian Kent wrote:
> The struct xfs_mount allocation (and freeing) is only used by the mount
> code, so move xfs_mount_alloc() to the same area as the option handling
> code (as part of the work to locate the mount code together).
> 
> Signed-off-by: Ian Kent <raven@themaw.net>

Looks good,

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

[PATCH v7 08/17] xfs: merge freeing of mp names and mp

[Ian Kent]

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>
---
 fs/xfs/xfs_super.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 0596d491dbbe..297e6c98742e 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -446,6 +446,15 @@ xfs_mount_alloc(
 	return mp;
 }
 
+static void
+xfs_mount_free(
+	struct xfs_mount	*mp)
+{
+	kfree(mp->m_rtname);
+	kfree(mp->m_logname);
+	kmem_free(mp);
+}
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -1058,14 +1067,6 @@ xfs_fs_drop_inode(
 	return generic_drop_inode(inode) || (ip->i_flags & XFS_IDONTCACHE);
 }
 
-STATIC void
-xfs_free_names(
-	struct xfs_mount	*mp)
-{
-	kfree(mp->m_rtname);
-	kfree(mp->m_logname);
-}
-
 STATIC int
 xfs_fs_sync_fs(
 	struct super_block	*sb,
@@ -1238,8 +1239,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;
 }
@@ -1747,8 +1747,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;
 
@@ -1779,8 +1778,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 *

Re: [PATCH v7 08/17] xfs: merge freeing of mp names and mp

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:17PM +0800, Ian Kent wrote:
> 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>

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

--D

> ---
>  fs/xfs/xfs_super.c |   26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 0596d491dbbe..297e6c98742e 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -446,6 +446,15 @@ xfs_mount_alloc(
>  	return mp;
>  }
>  
> +static void
> +xfs_mount_free(
> +	struct xfs_mount	*mp)
> +{
> +	kfree(mp->m_rtname);
> +	kfree(mp->m_logname);
> +	kmem_free(mp);
> +}
> +
>  struct proc_xfs_info {
>  	uint64_t	flag;
>  	char		*str;
> @@ -1058,14 +1067,6 @@ xfs_fs_drop_inode(
>  	return generic_drop_inode(inode) || (ip->i_flags & XFS_IDONTCACHE);
>  }
>  
> -STATIC void
> -xfs_free_names(
> -	struct xfs_mount	*mp)
> -{
> -	kfree(mp->m_rtname);
> -	kfree(mp->m_logname);
> -}
> -
>  STATIC int
>  xfs_fs_sync_fs(
>  	struct super_block	*sb,
> @@ -1238,8 +1239,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;
>  }
> @@ -1747,8 +1747,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;
>  
> @@ -1779,8 +1778,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 *
> 

Re: [PATCH v7 08/17] xfs: merge freeing of mp names and mp

[Christoph Hellwig]

On Thu, Oct 24, 2019 at 03:51:17PM +0800, Ian Kent wrote:
> 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>

Looks good,

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

[PATCH v7 09/17] xfs: add xfs_remount_rw() helper

[Ian Kent]

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.

This helper is only used by the mount code, so locate it along with
that code.

While we are at it change STATIC -> static for xfs_restore_resvblks().

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_super.c |  119 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 67 insertions(+), 52 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 297e6c98742e..c07e41489e75 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -47,6 +47,8 @@ static struct kset *xfs_kset;		/* top-level xfs sysfs dir */
 static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
 #endif
 
+static void xfs_restore_resvblks(struct xfs_mount *mp);
+
 /*
  * Table driven mount option parser.
  */
@@ -455,6 +457,68 @@ xfs_mount_free(
 	kmem_free(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;
+}
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -1169,7 +1233,7 @@ xfs_save_resvblks(struct xfs_mount *mp)
 	xfs_reserve_blocks(mp, &resblks, NULL);
 }
 
-STATIC void
+static void
 xfs_restore_resvblks(struct xfs_mount *mp)
 {
 	uint64_t resblks;
@@ -1307,57 +1371,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;
 	}
 

Re: [PATCH v7 09/17] xfs: add xfs_remount_rw() helper

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:22PM +0800, Ian Kent wrote:
> 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.
> 
> This helper is only used by the mount code, so locate it along with
> that code.
> 
> While we are at it change STATIC -> static for xfs_restore_resvblks().
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_super.c |  119 +++++++++++++++++++++++++++++-----------------------
>  1 file changed, 67 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 297e6c98742e..c07e41489e75 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -47,6 +47,8 @@ static struct kset *xfs_kset;		/* top-level xfs sysfs dir */
>  static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
>  #endif
>  
> +static void xfs_restore_resvblks(struct xfs_mount *mp);

What's the reason for putting xfs_remount_rw above xfs_restore_resvblks?
I assume that's related to where you want to land later code chunks?

--D

> +
>  /*
>   * Table driven mount option parser.
>   */
> @@ -455,6 +457,68 @@ xfs_mount_free(
>  	kmem_free(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;
> +}
> +
>  struct proc_xfs_info {
>  	uint64_t	flag;
>  	char		*str;
> @@ -1169,7 +1233,7 @@ xfs_save_resvblks(struct xfs_mount *mp)
>  	xfs_reserve_blocks(mp, &resblks, NULL);
>  }
>  
> -STATIC void
> +static void
>  xfs_restore_resvblks(struct xfs_mount *mp)
>  {
>  	uint64_t resblks;
> @@ -1307,57 +1371,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;
>  	}
>  
> 

Re: [PATCH v7 09/17] xfs: add xfs_remount_rw() helper

[Ian Kent]

On Thu, 2019-10-24 at 08:31 -0700, Darrick J. Wong wrote:
> On Thu, Oct 24, 2019 at 03:51:22PM +0800, Ian Kent wrote:
> > 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.
> > 
> > This helper is only used by the mount code, so locate it along with
> > that code.
> > 
> > While we are at it change STATIC -> static for
> > xfs_restore_resvblks().
> > 
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > Reviewed-by: Brian Foster <bfoster@redhat.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_super.c |  119 +++++++++++++++++++++++++++++-----------
> > ------------
> >  1 file changed, 67 insertions(+), 52 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 297e6c98742e..c07e41489e75 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -47,6 +47,8 @@ static struct kset *xfs_kset;		/* top-
> > level xfs sysfs dir */
> >  static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs
> > attrs */
> >  #endif
> >  
> > +static void xfs_restore_resvblks(struct xfs_mount *mp);
> 
> What's the reason for putting xfs_remount_rw above
> xfs_restore_resvblks?
> I assume that's related to where you want to land later code chunks?

In the cover letter:

Note: the patches "xfs: add xfs_remount_rw() helper" and
 "xfs: add xfs_remount_ro() helper" that have Reviewed-by attributions
 each needed a forward declartion added due grouping all the mount
 related code together. Reviewers may want to check the attribution
 is still acceptable.

The fill super method needs quite a few more forward declarations
too.

I responded to Christoph's suggestion of grouping the mount code
together saying this would be needed, and that I thought the
improvement of grouping the code together was worth the forward
declarations, and asked if anyone had a different POV on it and
got no replies, ;)

The other thing is that the options definitions notionally belong
near the top of the mount/super block handling code so moving it
all down seemed like the wrong thing to do ...

So what do you think of the extra noise of forward declarations
in this case?

Ian

> 
> --D
> 
> > +
> >  /*
> >   * Table driven mount option parser.
> >   */
> > @@ -455,6 +457,68 @@ xfs_mount_free(
> >  	kmem_free(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;
> > +}
> > +
> >  struct proc_xfs_info {
> >  	uint64_t	flag;
> >  	char		*str;
> > @@ -1169,7 +1233,7 @@ xfs_save_resvblks(struct xfs_mount *mp)
> >  	xfs_reserve_blocks(mp, &resblks, NULL);
> >  }
> >  
> > -STATIC void
> > +static void
> >  xfs_restore_resvblks(struct xfs_mount *mp)
> >  {
> >  	uint64_t resblks;
> > @@ -1307,57 +1371,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;
> >  	}
> >  
> > 

Re: [PATCH v7 09/17] xfs: add xfs_remount_rw() helper

[Darrick J. Wong]

On Fri, Oct 25, 2019 at 05:53:08AM +0800, Ian Kent wrote:
> On Thu, 2019-10-24 at 08:31 -0700, Darrick J. Wong wrote:
> > On Thu, Oct 24, 2019 at 03:51:22PM +0800, Ian Kent wrote:
> > > 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.
> > > 
> > > This helper is only used by the mount code, so locate it along with
> > > that code.
> > > 
> > > While we are at it change STATIC -> static for
> > > xfs_restore_resvblks().
> > > 
> > > Signed-off-by: Ian Kent <raven@themaw.net>
> > > Reviewed-by: Brian Foster <bfoster@redhat.com>
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  fs/xfs/xfs_super.c |  119 +++++++++++++++++++++++++++++-----------
> > > ------------
> > >  1 file changed, 67 insertions(+), 52 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index 297e6c98742e..c07e41489e75 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -47,6 +47,8 @@ static struct kset *xfs_kset;		/* top-
> > > level xfs sysfs dir */
> > >  static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs
> > > attrs */
> > >  #endif
> > >  
> > > +static void xfs_restore_resvblks(struct xfs_mount *mp);
> > 
> > What's the reason for putting xfs_remount_rw above
> > xfs_restore_resvblks?
> > I assume that's related to where you want to land later code chunks?
> 
> In the cover letter:
> 
> Note: the patches "xfs: add xfs_remount_rw() helper" and
>  "xfs: add xfs_remount_ro() helper" that have Reviewed-by attributions
>  each needed a forward declartion added due grouping all the mount
>  related code together. Reviewers may want to check the attribution
>  is still acceptable.
> 
> The fill super method needs quite a few more forward declarations
> too.
> 
> I responded to Christoph's suggestion of grouping the mount code
> together saying this would be needed, and that I thought the
> improvement of grouping the code together was worth the forward
> declarations, and asked if anyone had a different POV on it and
> got no replies, ;)
> 
> The other thing is that the options definitions notionally belong
> near the top of the mount/super block handling code so moving it
> all down seemed like the wrong thing to do ...
> 
> So what do you think of the extra noise of forward declarations
> in this case?

Eh, fine with me.  I was just curious, having speed-read over the
previous iterations. :)

--D

> Ian
> 
> > 
> > --D
> > 
> > > +
> > >  /*
> > >   * Table driven mount option parser.
> > >   */
> > > @@ -455,6 +457,68 @@ xfs_mount_free(
> > >  	kmem_free(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;
> > > +}
> > > +
> > >  struct proc_xfs_info {
> > >  	uint64_t	flag;
> > >  	char		*str;
> > > @@ -1169,7 +1233,7 @@ xfs_save_resvblks(struct xfs_mount *mp)
> > >  	xfs_reserve_blocks(mp, &resblks, NULL);
> > >  }
> > >  
> > > -STATIC void
> > > +static void
> > >  xfs_restore_resvblks(struct xfs_mount *mp)
> > >  {
> > >  	uint64_t resblks;
> > > @@ -1307,57 +1371,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;
> > >  	}
> > >  
> > > 
> 

Re: [PATCH v7 09/17] xfs: add xfs_remount_rw() helper

[Christoph Hellwig]

On Thu, Oct 24, 2019 at 04:12:58PM -0700, Darrick J. Wong wrote:
> > The fill super method needs quite a few more forward declarations
> > too.
> > 
> > I responded to Christoph's suggestion of grouping the mount code
> > together saying this would be needed, and that I thought the
> > improvement of grouping the code together was worth the forward
> > declarations, and asked if anyone had a different POV on it and
> > got no replies, ;)
> > 
> > The other thing is that the options definitions notionally belong
> > near the top of the mount/super block handling code so moving it
> > all down seemed like the wrong thing to do ...
> > 
> > So what do you think of the extra noise of forward declarations
> > in this case?
> 
> Eh, fine with me.  I was just curious, having speed-read over the
> previous iterations. :)

So looking at the result I'm not sure my suggestion was productive.  It
turns out there is a fair chunk of mount code at the end of the file as
well, and there is literally nothing left of the the code towards the
top of the file except for the Opt_* definitions.  So while I think
using this rewrite as a chance to group the code is still a good
idea, I suspect the better (and actually more natural) place is toward
the bottom of the file.
> 
> --D
> 
> > Ian
> > 
> > > 
> > > --D
> > > 
> > > > +
> > > >  /*
> > > >   * Table driven mount option parser.
> > > >   */
> > > > @@ -455,6 +457,68 @@ xfs_mount_free(
> > > >  	kmem_free(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;
> > > > +}
> > > > +
> > > >  struct proc_xfs_info {
> > > >  	uint64_t	flag;
> > > >  	char		*str;
> > > > @@ -1169,7 +1233,7 @@ xfs_save_resvblks(struct xfs_mount *mp)
> > > >  	xfs_reserve_blocks(mp, &resblks, NULL);
> > > >  }
> > > >  
> > > > -STATIC void
> > > > +static void
> > > >  xfs_restore_resvblks(struct xfs_mount *mp)
> > > >  {
> > > >  	uint64_t resblks;
> > > > @@ -1307,57 +1371,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;
> > > >  	}
> > > >  
> > > > 
> > 
---end quoted text---

[PATCH v7 10/17] xfs: add xfs_remount_ro() helper

[Ian Kent]

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.

This helper is only used by the mount code, so locate it along with
that code.

While we are at it change STATIC -> static for xfs_save_resvblks().

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_super.c |   76 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index c07e41489e75..97c3f1edb69c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -48,6 +48,7 @@ static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
 #endif
 
 static void xfs_restore_resvblks(struct xfs_mount *mp);
+static void xfs_save_resvblks(struct xfs_mount *mp);
 
 /*
  * Table driven mount option parser.
@@ -519,6 +520,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;
+}
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -1224,7 +1266,7 @@ xfs_fs_statfs(
 	return 0;
 }
 
-STATIC void
+static void
 xfs_save_resvblks(struct xfs_mount *mp)
 {
 	uint64_t resblks = 0;
@@ -1378,37 +1420,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;

Re: [PATCH v7 10/17] xfs: add xfs_remount_ro() helper

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:27PM +0800, Ian Kent wrote:
> 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.
> 
> This helper is only used by the mount code, so locate it along with
> that code.
> 
> While we are at it change STATIC -> static for xfs_save_resvblks().
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks like a straightforward code extraction, so

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

--D

> ---
>  fs/xfs/xfs_super.c |   76 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 45 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index c07e41489e75..97c3f1edb69c 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -48,6 +48,7 @@ static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
>  #endif
>  
>  static void xfs_restore_resvblks(struct xfs_mount *mp);
> +static void xfs_save_resvblks(struct xfs_mount *mp);
>  
>  /*
>   * Table driven mount option parser.
> @@ -519,6 +520,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;
> +}
> +
>  struct proc_xfs_info {
>  	uint64_t	flag;
>  	char		*str;
> @@ -1224,7 +1266,7 @@ xfs_fs_statfs(
>  	return 0;
>  }
>  
> -STATIC void
> +static void
>  xfs_save_resvblks(struct xfs_mount *mp)
>  {
>  	uint64_t resblks = 0;
> @@ -1378,37 +1420,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;
> 

[PATCH v7 11/17] xfs: refactor suffix_kstrtoint()

[Ian Kent]

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>
---
 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 97c3f1edb69c..003ec725d4b6 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -112,14 +112,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;
 
@@ -144,6 +147,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.
@@ -155,7 +175,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)
@@ -206,7 +226,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:
@@ -222,7 +242,7 @@ xfs_parseargs(
 				return -ENOMEM;
 			break;
 		case Opt_allocsize:
-			if (suffix_kstrtoint(args, 10, &iosize))
+			if (match_kstrtoint(args, 10, &iosize))
 				return -EINVAL;
 			iosizelog = ffs(iosize) - 1;
 			break;

Re: [PATCH v7 11/17] xfs: refactor suffix_kstrtoint()

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:32PM +0800, Ian Kent wrote:
> The mount-api doesn't have a "human unit" parse type yet so the options
> that have values like "10k" etc. still need to be converted by the fs.

Do you have plans to add such a thing to the mount api?  Or port the xfs
helper?  TBH I'd have thought that would show up as one of the vfs
patches at the start of this series.

I guess the patch itself looks fine as temporary support structures for
handling a transition.

--D

> 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>
> ---
>  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 97c3f1edb69c..003ec725d4b6 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -112,14 +112,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;
>  
> @@ -144,6 +147,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.
> @@ -155,7 +175,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)
> @@ -206,7 +226,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:
> @@ -222,7 +242,7 @@ xfs_parseargs(
>  				return -ENOMEM;
>  			break;
>  		case Opt_allocsize:
> -			if (suffix_kstrtoint(args, 10, &iosize))
> +			if (match_kstrtoint(args, 10, &iosize))
>  				return -EINVAL;
>  			iosizelog = ffs(iosize) - 1;
>  			break;
> 

Re: [PATCH v7 11/17] xfs: refactor suffix_kstrtoint()

[Ian Kent]

On Thu, 2019-10-24 at 08:38 -0700, Darrick J. Wong wrote:
> On Thu, Oct 24, 2019 at 03:51:32PM +0800, Ian Kent wrote:
> > The mount-api doesn't have a "human unit" parse type yet so the
> > options
> > that have values like "10k" etc. still need to be converted by the
> > fs.
> 
> Do you have plans to add such a thing to the mount api?  Or port the
> xfs
> helper?  TBH I'd have thought that would show up as one of the vfs
> patches at the start of this series.

I asked David (Howells) about this a while back and he did have some
patches for it but they weren't posted.

I can't remember now but I think there was a reason for holding back
on it.

I expect they will be posted for merge at some point but I don't
know when that will be.

> 
> I guess the patch itself looks fine as temporary support structures
> for
> handling a transition.
> 
> --D
> 
> > 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>
> > ---
> >  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 97c3f1edb69c..003ec725d4b6 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -112,14 +112,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;
> >  
> > @@ -144,6 +147,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.
> > @@ -155,7 +175,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)
> > @@ -206,7 +226,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:
> > @@ -222,7 +242,7 @@ xfs_parseargs(
> >  				return -ENOMEM;
> >  			break;
> >  		case Opt_allocsize:
> > -			if (suffix_kstrtoint(args, 10, &iosize))
> > +			if (match_kstrtoint(args, 10, &iosize))
> >  				return -EINVAL;
> >  			iosizelog = ffs(iosize) - 1;
> >  			break;
> > 

Re: [PATCH v7 11/17] xfs: refactor suffix_kstrtoint()

[Christoph Hellwig]

On Thu, Oct 24, 2019 at 03:51:32PM +0800, Ian Kent wrote:
> The mount-api doesn't have a "human unit" parse type yet so the options
> that have values like "10k" etc. still need to be converted by the fs.
> 
> But the value comes to the fs as a string (not a substring_t type) so
> there's a need to change the conversion function to take a character
> string instead.
> 
> 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>

Looks good,

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

[PATCH v7 12/17] xfs: avoid redundant checks when options is empty

[Ian Kent]

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>
---
 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 003ec725d4b6..92a37ac0b907 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -211,7 +211,7 @@ xfs_parseargs(
 	mp->m_logbsize = -1;
 
 	if (!options)
-		goto done;
+		return 0;
 
 	while ((p = strsep(&options, ",")) != NULL) {
 		int		token;
@@ -390,7 +390,6 @@ xfs_parseargs(
 		return -EINVAL;
 	}
 
-done:
 	if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
 		/*
 		 * At this point the superblock has not been read

Re: [PATCH v7 12/17] xfs: avoid redundant checks when options is empty

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:38PM +0800, Ian Kent wrote:
> 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>

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

--D

> ---
>  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 003ec725d4b6..92a37ac0b907 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -211,7 +211,7 @@ xfs_parseargs(
>  	mp->m_logbsize = -1;
>  
>  	if (!options)
> -		goto done;
> +		return 0;
>  
>  	while ((p = strsep(&options, ",")) != NULL) {
>  		int		token;
> @@ -390,7 +390,6 @@ xfs_parseargs(
>  		return -EINVAL;
>  	}
>  
> -done:
>  	if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
>  		/*
>  		 * At this point the superblock has not been read
> 

Re: [PATCH v7 12/17] xfs: avoid redundant checks when options is empty

[Christoph Hellwig]

On Thu, Oct 24, 2019 at 03:51:38PM +0800, Ian Kent wrote:
> 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>

Looks good,

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

[PATCH v7 13/17] xfs: refactor xfs_parseags()

[Ian Kent]

Refactor xfs_parseags(), move the entire token case block to a separate
function in an attempt to highlight the code that actually changes in
converting to use the new mount api.

The only changes are what's needed to communicate the variables dsunit,
dswidth and iosizelog back to xfs_parseags().

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

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 92a37ac0b907..de0ab79536b3 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -164,6 +164,156 @@ match_kstrtoint(
 	return ret;
 }
 
+static int
+xfs_fc_parse_param(
+	int			token,
+	char			*p,
+	substring_t		*args,
+	struct xfs_mount	*mp,
+	int			*dsunit,
+	int			*dswidth,
+	uint8_t			*iosizelog)
+{
+	int			iosize = 0;
+
+	switch (token) {
+	case Opt_logbufs:
+		if (match_int(args, &mp->m_logbufs))
+			return -EINVAL;
+		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, &iosize))
+			return -EINVAL;
+		*iosizelog = ffs(iosize) - 1;
+		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, dsunit))
+			return -EINVAL;
+		return 0;
+	case Opt_swidth:
+		if (match_int(args, dswidth))
+			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_COMPAT_IOSIZE;
+		return 0;
+	case Opt_nolargeio:
+		mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
+		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.
@@ -185,7 +335,6 @@ xfs_parseargs(
 	substring_t		args[MAX_OPT_ARGS];
 	int			dsunit = 0;
 	int			dswidth = 0;
-	int			iosize = 0;
 	uint8_t			iosizelog = 0;
 
 	/*
@@ -215,145 +364,16 @@ xfs_parseargs(
 
 	while ((p = strsep(&options, ",")) != NULL) {
 		int		token;
+		int		ret;
 
 		if (!*p)
 			continue;
 
 		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_logbufs:
-			if (match_int(args, &mp->m_logbufs))
-				return -EINVAL;
-			break;
-		case Opt_logbsize:
-			if (match_kstrtoint(args, 10, &mp->m_logbsize))
-				return -EINVAL;
-			break;
-		case Opt_logdev:
-			kfree(mp->m_logname);
-			mp->m_logname = match_strdup(args);
-			if (!mp->m_logname)
-				return -ENOMEM;
-			break;
-		case Opt_rtdev:
-			kfree(mp->m_rtname);
-			mp->m_rtname = match_strdup(args);
-			if (!mp->m_rtname)
-				return -ENOMEM;
-			break;
-		case Opt_allocsize:
-			if (match_kstrtoint(args, 10, &iosize))
-				return -EINVAL;
-			iosizelog = ffs(iosize) - 1;
-			break;
-		case Opt_grpid:
-		case Opt_bsdgroups:
-			mp->m_flags |= XFS_MOUNT_GRPID;
-			break;
-		case Opt_nogrpid:
-		case Opt_sysvgroups:
-			mp->m_flags &= ~XFS_MOUNT_GRPID;
-			break;
-		case Opt_wsync:
-			mp->m_flags |= XFS_MOUNT_WSYNC;
-			break;
-		case Opt_norecovery:
-			mp->m_flags |= XFS_MOUNT_NORECOVERY;
-			break;
-		case Opt_noalign:
-			mp->m_flags |= XFS_MOUNT_NOALIGN;
-			break;
-		case Opt_swalloc:
-			mp->m_flags |= XFS_MOUNT_SWALLOC;
-			break;
-		case Opt_sunit:
-			if (match_int(args, &dsunit))
-				return -EINVAL;
-			break;
-		case Opt_swidth:
-			if (match_int(args, &dswidth))
-				return -EINVAL;
-			break;
-		case Opt_inode32:
-			mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
-			break;
-		case Opt_inode64:
-			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
-			break;
-		case Opt_nouuid:
-			mp->m_flags |= XFS_MOUNT_NOUUID;
-			break;
-		case Opt_ikeep:
-			mp->m_flags |= XFS_MOUNT_IKEEP;
-			break;
-		case Opt_noikeep:
-			mp->m_flags &= ~XFS_MOUNT_IKEEP;
-			break;
-		case Opt_largeio:
-			mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
-			break;
-		case Opt_nolargeio:
-			mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
-			break;
-		case Opt_attr2:
-			mp->m_flags |= XFS_MOUNT_ATTR2;
-			break;
-		case Opt_noattr2:
-			mp->m_flags &= ~XFS_MOUNT_ATTR2;
-			mp->m_flags |= XFS_MOUNT_NOATTR2;
-			break;
-		case Opt_filestreams:
-			mp->m_flags |= XFS_MOUNT_FILESTREAMS;
-			break;
-		case Opt_noquota:
-			mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
-			mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
-			mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
-			break;
-		case Opt_quota:
-		case Opt_uquota:
-		case Opt_usrquota:
-			mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
-					 XFS_UQUOTA_ENFD);
-			break;
-		case Opt_qnoenforce:
-		case Opt_uqnoenforce:
-			mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
-			mp->m_qflags &= ~XFS_UQUOTA_ENFD;
-			break;
-		case Opt_pquota:
-		case Opt_prjquota:
-			mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
-					 XFS_PQUOTA_ENFD);
-			break;
-		case Opt_pqnoenforce:
-			mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
-			mp->m_qflags &= ~XFS_PQUOTA_ENFD;
-			break;
-		case Opt_gquota:
-		case Opt_grpquota:
-			mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
-					 XFS_GQUOTA_ENFD);
-			break;
-		case Opt_gqnoenforce:
-			mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
-			mp->m_qflags &= ~XFS_GQUOTA_ENFD;
-			break;
-		case Opt_discard:
-			mp->m_flags |= XFS_MOUNT_DISCARD;
-			break;
-		case Opt_nodiscard:
-			mp->m_flags &= ~XFS_MOUNT_DISCARD;
-			break;
-#ifdef CONFIG_FS_DAX
-		case Opt_dax:
-			mp->m_flags |= XFS_MOUNT_DAX;
-			break;
-#endif
-		default:
-			xfs_warn(mp, "unknown mount option [%s].", p);
-			return -EINVAL;
-		}
+		ret = xfs_fc_parse_param(token, p, args, mp, &dsunit, &dswidth,
+					 &iosizelog);
+		if (ret)
+			return ret;
 	}
 
 	/*

Re: [PATCH v7 13/17] xfs: refactor xfs_parseags()

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:43PM +0800, Ian Kent wrote:
> Refactor xfs_parseags(), move the entire token case block to a separate
> function in an attempt to highlight the code that actually changes in
> converting to use the new mount api.
> 
> The only changes are what's needed to communicate the variables dsunit,
> dswidth and iosizelog back to xfs_parseags().
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

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

--D

> ---
>  fs/xfs/xfs_super.c |  290 ++++++++++++++++++++++++++++------------------------
>  1 file changed, 155 insertions(+), 135 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 92a37ac0b907..de0ab79536b3 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -164,6 +164,156 @@ match_kstrtoint(
>  	return ret;
>  }
>  
> +static int
> +xfs_fc_parse_param(
> +	int			token,
> +	char			*p,
> +	substring_t		*args,
> +	struct xfs_mount	*mp,
> +	int			*dsunit,
> +	int			*dswidth,
> +	uint8_t			*iosizelog)
> +{
> +	int			iosize = 0;
> +
> +	switch (token) {
> +	case Opt_logbufs:
> +		if (match_int(args, &mp->m_logbufs))
> +			return -EINVAL;
> +		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, &iosize))
> +			return -EINVAL;
> +		*iosizelog = ffs(iosize) - 1;
> +		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, dsunit))
> +			return -EINVAL;
> +		return 0;
> +	case Opt_swidth:
> +		if (match_int(args, dswidth))
> +			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_COMPAT_IOSIZE;
> +		return 0;
> +	case Opt_nolargeio:
> +		mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> +		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.
> @@ -185,7 +335,6 @@ xfs_parseargs(
>  	substring_t		args[MAX_OPT_ARGS];
>  	int			dsunit = 0;
>  	int			dswidth = 0;
> -	int			iosize = 0;
>  	uint8_t			iosizelog = 0;
>  
>  	/*
> @@ -215,145 +364,16 @@ xfs_parseargs(
>  
>  	while ((p = strsep(&options, ",")) != NULL) {
>  		int		token;
> +		int		ret;
>  
>  		if (!*p)
>  			continue;
>  
>  		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case Opt_logbufs:
> -			if (match_int(args, &mp->m_logbufs))
> -				return -EINVAL;
> -			break;
> -		case Opt_logbsize:
> -			if (match_kstrtoint(args, 10, &mp->m_logbsize))
> -				return -EINVAL;
> -			break;
> -		case Opt_logdev:
> -			kfree(mp->m_logname);
> -			mp->m_logname = match_strdup(args);
> -			if (!mp->m_logname)
> -				return -ENOMEM;
> -			break;
> -		case Opt_rtdev:
> -			kfree(mp->m_rtname);
> -			mp->m_rtname = match_strdup(args);
> -			if (!mp->m_rtname)
> -				return -ENOMEM;
> -			break;
> -		case Opt_allocsize:
> -			if (match_kstrtoint(args, 10, &iosize))
> -				return -EINVAL;
> -			iosizelog = ffs(iosize) - 1;
> -			break;
> -		case Opt_grpid:
> -		case Opt_bsdgroups:
> -			mp->m_flags |= XFS_MOUNT_GRPID;
> -			break;
> -		case Opt_nogrpid:
> -		case Opt_sysvgroups:
> -			mp->m_flags &= ~XFS_MOUNT_GRPID;
> -			break;
> -		case Opt_wsync:
> -			mp->m_flags |= XFS_MOUNT_WSYNC;
> -			break;
> -		case Opt_norecovery:
> -			mp->m_flags |= XFS_MOUNT_NORECOVERY;
> -			break;
> -		case Opt_noalign:
> -			mp->m_flags |= XFS_MOUNT_NOALIGN;
> -			break;
> -		case Opt_swalloc:
> -			mp->m_flags |= XFS_MOUNT_SWALLOC;
> -			break;
> -		case Opt_sunit:
> -			if (match_int(args, &dsunit))
> -				return -EINVAL;
> -			break;
> -		case Opt_swidth:
> -			if (match_int(args, &dswidth))
> -				return -EINVAL;
> -			break;
> -		case Opt_inode32:
> -			mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> -			break;
> -		case Opt_inode64:
> -			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> -			break;
> -		case Opt_nouuid:
> -			mp->m_flags |= XFS_MOUNT_NOUUID;
> -			break;
> -		case Opt_ikeep:
> -			mp->m_flags |= XFS_MOUNT_IKEEP;
> -			break;
> -		case Opt_noikeep:
> -			mp->m_flags &= ~XFS_MOUNT_IKEEP;
> -			break;
> -		case Opt_largeio:
> -			mp->m_flags &= ~XFS_MOUNT_COMPAT_IOSIZE;
> -			break;
> -		case Opt_nolargeio:
> -			mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> -			break;
> -		case Opt_attr2:
> -			mp->m_flags |= XFS_MOUNT_ATTR2;
> -			break;
> -		case Opt_noattr2:
> -			mp->m_flags &= ~XFS_MOUNT_ATTR2;
> -			mp->m_flags |= XFS_MOUNT_NOATTR2;
> -			break;
> -		case Opt_filestreams:
> -			mp->m_flags |= XFS_MOUNT_FILESTREAMS;
> -			break;
> -		case Opt_noquota:
> -			mp->m_qflags &= ~XFS_ALL_QUOTA_ACCT;
> -			mp->m_qflags &= ~XFS_ALL_QUOTA_ENFD;
> -			mp->m_qflags &= ~XFS_ALL_QUOTA_ACTIVE;
> -			break;
> -		case Opt_quota:
> -		case Opt_uquota:
> -		case Opt_usrquota:
> -			mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE |
> -					 XFS_UQUOTA_ENFD);
> -			break;
> -		case Opt_qnoenforce:
> -		case Opt_uqnoenforce:
> -			mp->m_qflags |= (XFS_UQUOTA_ACCT | XFS_UQUOTA_ACTIVE);
> -			mp->m_qflags &= ~XFS_UQUOTA_ENFD;
> -			break;
> -		case Opt_pquota:
> -		case Opt_prjquota:
> -			mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE |
> -					 XFS_PQUOTA_ENFD);
> -			break;
> -		case Opt_pqnoenforce:
> -			mp->m_qflags |= (XFS_PQUOTA_ACCT | XFS_PQUOTA_ACTIVE);
> -			mp->m_qflags &= ~XFS_PQUOTA_ENFD;
> -			break;
> -		case Opt_gquota:
> -		case Opt_grpquota:
> -			mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE |
> -					 XFS_GQUOTA_ENFD);
> -			break;
> -		case Opt_gqnoenforce:
> -			mp->m_qflags |= (XFS_GQUOTA_ACCT | XFS_GQUOTA_ACTIVE);
> -			mp->m_qflags &= ~XFS_GQUOTA_ENFD;
> -			break;
> -		case Opt_discard:
> -			mp->m_flags |= XFS_MOUNT_DISCARD;
> -			break;
> -		case Opt_nodiscard:
> -			mp->m_flags &= ~XFS_MOUNT_DISCARD;
> -			break;
> -#ifdef CONFIG_FS_DAX
> -		case Opt_dax:
> -			mp->m_flags |= XFS_MOUNT_DAX;
> -			break;
> -#endif
> -		default:
> -			xfs_warn(mp, "unknown mount option [%s].", p);
> -			return -EINVAL;
> -		}
> +		ret = xfs_fc_parse_param(token, p, args, mp, &dsunit, &dswidth,
> +					 &iosizelog);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	/*
> 

[PATCH v7 14/17] xfs: move xfs_parseargs() validation to a helper

[Ian Kent]

Move the validation code of xfs_parseargs() into a helper for later
use within the mount context methods.

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

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index de0ab79536b3..b3c27a0781ed 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -314,68 +314,13 @@ 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(
+xfs_fc_validate_params(
 	struct xfs_mount	*mp,
-	char			*options)
+	int			dsunit,
+	int			dswidth,
+	uint8_t			iosizelog)
 {
-	const struct super_block *sb = mp->m_super;
-	char			*p;
-	substring_t		args[MAX_OPT_ARGS];
-	int			dsunit = 0;
-	int			dswidth = 0;
-	uint8_t			iosizelog = 0;
-
-	/*
-	 * Copy binary VFS mount flags we are interested in.
-	 */
-	if (sb_rdonly(sb))
-		mp->m_flags |= XFS_MOUNT_RDONLY;
-	if (sb->s_flags & SB_DIRSYNC)
-		mp->m_flags |= XFS_MOUNT_DIRSYNC;
-	if (sb->s_flags & SB_SYNCHRONOUS)
-		mp->m_flags |= XFS_MOUNT_WSYNC;
-
-	/*
-	 * Set some default flags that could be cleared by the mount option
-	 * parsing.
-	 */
-	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
-
-	/*
-	 * These can be overridden by the mount option parsing.
-	 */
-	mp->m_logbufs = -1;
-	mp->m_logbsize = -1;
-
-	if (!options)
-		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, &dsunit, &dswidth,
-					 &iosizelog);
-		if (ret)
-			return ret;
-	}
-
 	/*
 	 * no recovery flag requires a read-only mount
 	 */
@@ -600,6 +545,71 @@ xfs_remount_ro(
 	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];
+	int			dsunit = 0;
+	int			dswidth = 0;
+	uint8_t			iosizelog = 0;
+
+	/*
+	 * Copy binary VFS mount flags we are interested in.
+	 */
+	if (sb_rdonly(sb))
+		mp->m_flags |= XFS_MOUNT_RDONLY;
+	if (sb->s_flags & SB_DIRSYNC)
+		mp->m_flags |= XFS_MOUNT_DIRSYNC;
+	if (sb->s_flags & SB_SYNCHRONOUS)
+		mp->m_flags |= XFS_MOUNT_WSYNC;
+
+	/*
+	 * Set some default flags that could be cleared by the mount option
+	 * parsing.
+	 */
+	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
+
+	/*
+	 * These can be overridden by the mount option parsing.
+	 */
+	mp->m_logbufs = -1;
+	mp->m_logbsize = -1;
+
+	if (!options)
+		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, &dsunit, &dswidth,
+					 &iosizelog);
+		if (ret)
+			return ret;
+	}
+
+	return xfs_fc_validate_params(mp, dsunit, dswidth, iosizelog);
+}
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;

Re: [PATCH v7 14/17] xfs: move xfs_parseargs() validation to a helper

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:51:48PM +0800, Ian Kent wrote:
> Move the validation code of xfs_parseargs() into a helper for later
> use within the mount context methods.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>

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

--D

> ---
>  fs/xfs/xfs_super.c |  128 ++++++++++++++++++++++++++++------------------------
>  1 file changed, 69 insertions(+), 59 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index de0ab79536b3..b3c27a0781ed 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -314,68 +314,13 @@ 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(
> +xfs_fc_validate_params(
>  	struct xfs_mount	*mp,
> -	char			*options)
> +	int			dsunit,
> +	int			dswidth,
> +	uint8_t			iosizelog)
>  {
> -	const struct super_block *sb = mp->m_super;
> -	char			*p;
> -	substring_t		args[MAX_OPT_ARGS];
> -	int			dsunit = 0;
> -	int			dswidth = 0;
> -	uint8_t			iosizelog = 0;
> -
> -	/*
> -	 * Copy binary VFS mount flags we are interested in.
> -	 */
> -	if (sb_rdonly(sb))
> -		mp->m_flags |= XFS_MOUNT_RDONLY;
> -	if (sb->s_flags & SB_DIRSYNC)
> -		mp->m_flags |= XFS_MOUNT_DIRSYNC;
> -	if (sb->s_flags & SB_SYNCHRONOUS)
> -		mp->m_flags |= XFS_MOUNT_WSYNC;
> -
> -	/*
> -	 * Set some default flags that could be cleared by the mount option
> -	 * parsing.
> -	 */
> -	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> -
> -	/*
> -	 * These can be overridden by the mount option parsing.
> -	 */
> -	mp->m_logbufs = -1;
> -	mp->m_logbsize = -1;
> -
> -	if (!options)
> -		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, &dsunit, &dswidth,
> -					 &iosizelog);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	/*
>  	 * no recovery flag requires a read-only mount
>  	 */
> @@ -600,6 +545,71 @@ xfs_remount_ro(
>  	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];
> +	int			dsunit = 0;
> +	int			dswidth = 0;
> +	uint8_t			iosizelog = 0;
> +
> +	/*
> +	 * Copy binary VFS mount flags we are interested in.
> +	 */
> +	if (sb_rdonly(sb))
> +		mp->m_flags |= XFS_MOUNT_RDONLY;
> +	if (sb->s_flags & SB_DIRSYNC)
> +		mp->m_flags |= XFS_MOUNT_DIRSYNC;
> +	if (sb->s_flags & SB_SYNCHRONOUS)
> +		mp->m_flags |= XFS_MOUNT_WSYNC;
> +
> +	/*
> +	 * Set some default flags that could be cleared by the mount option
> +	 * parsing.
> +	 */
> +	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> +
> +	/*
> +	 * These can be overridden by the mount option parsing.
> +	 */
> +	mp->m_logbufs = -1;
> +	mp->m_logbsize = -1;
> +
> +	if (!options)
> +		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, &dsunit, &dswidth,
> +					 &iosizelog);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return xfs_fc_validate_params(mp, dsunit, dswidth, iosizelog);
> +}
> +
>  struct proc_xfs_info {
>  	uint64_t	flag;
>  	char		*str;
> 

[PATCH v7 15/17] xfs: dont set sb in xfs_mount_alloc()

[Ian Kent]

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

Also change xfs_mount_alloc() decalaration static -> STATIC.

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 b3c27a0781ed..24eec22bcac1 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -403,8 +403,7 @@ xfs_fc_validate_params(
 }
 
 static struct xfs_mount *
-xfs_mount_alloc(
-	struct super_block	*sb)
+xfs_mount_alloc(void)
 {
 	struct xfs_mount	*mp;
 
@@ -412,7 +411,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);
@@ -1652,9 +1650,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);

[PATCH v7 16/17] xfs: move xfs_fs_fill_super() to be with parsing code

[Ian Kent]

The function xfs_fs_fill_super() is only used by the mount code, so move
it to the same area as the option handling code (as part the work to
locate the mount code together).

Unfortunately some forward declarations are needed as several functions
called by xfs_fs_fill_super() ahlready have a sensible ordering and are
grouped with other related functions.

Change STATIC -> static for referenced functions while we're at it.

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

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 24eec22bcac1..dd019be3fa72 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -47,8 +47,17 @@ static struct kset *xfs_kset;		/* top-level xfs sysfs dir */
 static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
 #endif
 
+static int xfs_setup_devices(struct xfs_mount *);
+static int xfs_open_devices(struct xfs_mount *);
+static void xfs_close_devices(struct xfs_mount *);
+static int xfs_init_mount_workqueues(struct xfs_mount *);
+static void xfs_destroy_mount_workqueues(struct xfs_mount *);
+static int xfs_init_percpu_counters(struct xfs_mount *);
+static void xfs_destroy_percpu_counters(struct xfs_mount *);
 static void xfs_restore_resvblks(struct xfs_mount *mp);
 static void xfs_save_resvblks(struct xfs_mount *mp);
+static int xfs_finish_flags(struct xfs_mount *);
+static uint64_t xfs_max_file_offset(unsigned int);
 
 /*
  * Table driven mount option parser.
@@ -608,6 +617,204 @@ xfs_parseargs(
 	return xfs_fc_validate_params(mp, dsunit, dswidth, iosizelog);
 }
 
+static int
+xfs_fs_fill_super(
+	struct super_block	*sb,
+	void			*data,
+	int			silent)
+{
+	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);
+	if (error)
+		goto out_free_names;
+
+	sb_min_blocksize(sb, BBSIZE);
+	sb->s_xattr = xfs_xattr_handlers;
+	sb->s_export_op = &xfs_export_operations;
+#ifdef CONFIG_XFS_QUOTA
+	sb->s_qcop = &xfs_quotactl_operations;
+	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
+#endif
+	sb->s_op = &xfs_super_operations;
+
+	/*
+	 * Delay mount work if the debug hook is set. This is debug
+	 * instrumention to coordinate simulation of xfs mount failures with
+	 * VFS superblock operations
+	 */
+	if (xfs_globals.mount_delay) {
+		xfs_notice(mp, "Delaying mount for %d seconds.",
+			xfs_globals.mount_delay);
+		msleep(xfs_globals.mount_delay * 1000);
+	}
+
+	if (silent)
+		flags |= XFS_MFSI_QUIET;
+
+	error = xfs_open_devices(mp);
+	if (error)
+		goto out_free_names;
+
+	error = xfs_init_mount_workqueues(mp);
+	if (error)
+		goto out_close_devices;
+
+	error = xfs_init_percpu_counters(mp);
+	if (error)
+		goto out_destroy_workqueues;
+
+	/* Allocate stats memory before we do operations that might use it */
+	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
+	if (!mp->m_stats.xs_stats) {
+		error = -ENOMEM;
+		goto out_destroy_counters;
+	}
+
+	error = xfs_readsb(mp, flags);
+	if (error)
+		goto out_free_stats;
+
+	error = xfs_finish_flags(mp);
+	if (error)
+		goto out_free_sb;
+
+	error = xfs_setup_devices(mp);
+	if (error)
+		goto out_free_sb;
+
+	error = xfs_filestream_mount(mp);
+	if (error)
+		goto out_free_sb;
+
+	/*
+	 * we must configure the block size in the superblock before we run the
+	 * full mount process as the mount process can lookup and cache inodes.
+	 */
+	sb->s_magic = XFS_SUPER_MAGIC;
+	sb->s_blocksize = mp->m_sb.sb_blocksize;
+	sb->s_blocksize_bits = ffs(sb->s_blocksize) - 1;
+	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
+	sb->s_max_links = XFS_MAXLINK;
+	sb->s_time_gran = 1;
+	sb->s_time_min = S32_MIN;
+	sb->s_time_max = S32_MAX;
+	sb->s_iflags |= SB_I_CGROUPWB;
+
+	set_posix_acl_flag(sb);
+
+	/* version 5 superblocks support inode version counters. */
+	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
+		sb->s_flags |= SB_I_VERSION;
+
+	if (mp->m_flags & XFS_MOUNT_DAX) {
+		bool rtdev_is_dax = false, datadev_is_dax;
+
+		xfs_warn(mp,
+		"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
+
+		datadev_is_dax = bdev_dax_supported(mp->m_ddev_targp->bt_bdev,
+			sb->s_blocksize);
+		if (mp->m_rtdev_targp)
+			rtdev_is_dax = bdev_dax_supported(
+				mp->m_rtdev_targp->bt_bdev, sb->s_blocksize);
+		if (!rtdev_is_dax && !datadev_is_dax) {
+			xfs_alert(mp,
+			"DAX unsupported by block device. Turning off DAX.");
+			mp->m_flags &= ~XFS_MOUNT_DAX;
+		}
+		if (xfs_sb_version_hasreflink(&mp->m_sb)) {
+			xfs_alert(mp,
+		"DAX and reflink cannot be used together!");
+			error = -EINVAL;
+			goto out_filestream_unmount;
+		}
+	}
+
+	if (mp->m_flags & XFS_MOUNT_DISCARD) {
+		struct request_queue *q = bdev_get_queue(sb->s_bdev);
+
+		if (!blk_queue_discard(q)) {
+			xfs_warn(mp, "mounting with \"discard\" option, but "
+					"the device does not support discard");
+			mp->m_flags &= ~XFS_MOUNT_DISCARD;
+		}
+	}
+
+	if (xfs_sb_version_hasreflink(&mp->m_sb)) {
+		if (mp->m_sb.sb_rblocks) {
+			xfs_alert(mp,
+	"reflink not compatible with realtime device!");
+			error = -EINVAL;
+			goto out_filestream_unmount;
+		}
+
+		if (xfs_globals.always_cow) {
+			xfs_info(mp, "using DEBUG-only always_cow mode.");
+			mp->m_always_cow = true;
+		}
+	}
+
+	if (xfs_sb_version_hasrmapbt(&mp->m_sb) && mp->m_sb.sb_rblocks) {
+		xfs_alert(mp,
+	"reverse mapping btree not compatible with realtime device!");
+		error = -EINVAL;
+		goto out_filestream_unmount;
+	}
+
+	error = xfs_mountfs(mp);
+	if (error)
+		goto out_filestream_unmount;
+
+	root = igrab(VFS_I(mp->m_rootip));
+	if (!root) {
+		error = -ENOENT;
+		goto out_unmount;
+	}
+	sb->s_root = d_make_root(root);
+	if (!sb->s_root) {
+		error = -ENOMEM;
+		goto out_unmount;
+	}
+
+	return 0;
+
+ out_filestream_unmount:
+	xfs_filestream_unmount(mp);
+ out_free_sb:
+	xfs_freesb(mp);
+ out_free_stats:
+	free_percpu(mp->m_stats.xs_stats);
+ out_destroy_counters:
+	xfs_destroy_percpu_counters(mp);
+ out_destroy_workqueues:
+	xfs_destroy_mount_workqueues(mp);
+ out_close_devices:
+	xfs_close_devices(mp);
+ out_free_names:
+	sb->s_fs_info = NULL;
+	xfs_mount_free(mp);
+ out:
+	return error;
+
+ out_unmount:
+	xfs_filestream_unmount(mp);
+	xfs_unmountfs(mp);
+	goto out_free_sb;
+}
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -840,7 +1047,7 @@ xfs_blkdev_issue_flush(
 	blkdev_issue_flush(buftarg->bt_bdev, GFP_NOFS, NULL);
 }
 
-STATIC void
+static void
 xfs_close_devices(
 	struct xfs_mount	*mp)
 {
@@ -876,7 +1083,7 @@ xfs_close_devices(
  * they are present.  The data subvolume has already been opened by
  * get_sb_bdev() and is stored in sb->s_bdev.
  */
-STATIC int
+static int
 xfs_open_devices(
 	struct xfs_mount	*mp)
 {
@@ -955,7 +1162,7 @@ xfs_open_devices(
 /*
  * Setup xfs_mount buffer target pointers based on superblock
  */
-STATIC int
+static int
 xfs_setup_devices(
 	struct xfs_mount	*mp)
 {
@@ -985,7 +1192,7 @@ xfs_setup_devices(
 	return 0;
 }
 
-STATIC int
+static int
 xfs_init_mount_workqueues(
 	struct xfs_mount	*mp)
 {
@@ -1036,7 +1243,7 @@ xfs_init_mount_workqueues(
 	return -ENOMEM;
 }
 
-STATIC void
+static void
 xfs_destroy_mount_workqueues(
 	struct xfs_mount	*mp)
 {
@@ -1518,7 +1725,7 @@ xfs_fs_show_options(
  * This function fills in xfs_mount_t fields based on mount args.
  * Note: the superblock _has_ now been read in.
  */
-STATIC int
+static int
 xfs_finish_flags(
 	struct xfs_mount	*mp)
 {
@@ -1636,204 +1843,6 @@ xfs_destroy_percpu_counters(
 	percpu_counter_destroy(&mp->m_delalloc_blks);
 }
 
-STATIC int
-xfs_fs_fill_super(
-	struct super_block	*sb,
-	void			*data,
-	int			silent)
-{
-	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);
-	if (error)
-		goto out_free_names;
-
-	sb_min_blocksize(sb, BBSIZE);
-	sb->s_xattr = xfs_xattr_handlers;
-	sb->s_export_op = &xfs_export_operations;
-#ifdef CONFIG_XFS_QUOTA
-	sb->s_qcop = &xfs_quotactl_operations;
-	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
-#endif
-	sb->s_op = &xfs_super_operations;
-
-	/*
-	 * Delay mount work if the debug hook is set. This is debug
-	 * instrumention to coordinate simulation of xfs mount failures with
-	 * VFS superblock operations
-	 */
-	if (xfs_globals.mount_delay) {
-		xfs_notice(mp, "Delaying mount for %d seconds.",
-			xfs_globals.mount_delay);
-		msleep(xfs_globals.mount_delay * 1000);
-	}
-
-	if (silent)
-		flags |= XFS_MFSI_QUIET;
-
-	error = xfs_open_devices(mp);
-	if (error)
-		goto out_free_names;
-
-	error = xfs_init_mount_workqueues(mp);
-	if (error)
-		goto out_close_devices;
-
-	error = xfs_init_percpu_counters(mp);
-	if (error)
-		goto out_destroy_workqueues;
-
-	/* Allocate stats memory before we do operations that might use it */
-	mp->m_stats.xs_stats = alloc_percpu(struct xfsstats);
-	if (!mp->m_stats.xs_stats) {
-		error = -ENOMEM;
-		goto out_destroy_counters;
-	}
-
-	error = xfs_readsb(mp, flags);
-	if (error)
-		goto out_free_stats;
-
-	error = xfs_finish_flags(mp);
-	if (error)
-		goto out_free_sb;
-
-	error = xfs_setup_devices(mp);
-	if (error)
-		goto out_free_sb;
-
-	error = xfs_filestream_mount(mp);
-	if (error)
-		goto out_free_sb;
-
-	/*
-	 * we must configure the block size in the superblock before we run the
-	 * full mount process as the mount process can lookup and cache inodes.
-	 */
-	sb->s_magic = XFS_SUPER_MAGIC;
-	sb->s_blocksize = mp->m_sb.sb_blocksize;
-	sb->s_blocksize_bits = ffs(sb->s_blocksize) - 1;
-	sb->s_maxbytes = xfs_max_file_offset(sb->s_blocksize_bits);
-	sb->s_max_links = XFS_MAXLINK;
-	sb->s_time_gran = 1;
-	sb->s_time_min = S32_MIN;
-	sb->s_time_max = S32_MAX;
-	sb->s_iflags |= SB_I_CGROUPWB;
-
-	set_posix_acl_flag(sb);
-
-	/* version 5 superblocks support inode version counters. */
-	if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
-		sb->s_flags |= SB_I_VERSION;
-
-	if (mp->m_flags & XFS_MOUNT_DAX) {
-		bool rtdev_is_dax = false, datadev_is_dax;
-
-		xfs_warn(mp,
-		"DAX enabled. Warning: EXPERIMENTAL, use at your own risk");
-
-		datadev_is_dax = bdev_dax_supported(mp->m_ddev_targp->bt_bdev,
-			sb->s_blocksize);
-		if (mp->m_rtdev_targp)
-			rtdev_is_dax = bdev_dax_supported(
-				mp->m_rtdev_targp->bt_bdev, sb->s_blocksize);
-		if (!rtdev_is_dax && !datadev_is_dax) {
-			xfs_alert(mp,
-			"DAX unsupported by block device. Turning off DAX.");
-			mp->m_flags &= ~XFS_MOUNT_DAX;
-		}
-		if (xfs_sb_version_hasreflink(&mp->m_sb)) {
-			xfs_alert(mp,
-		"DAX and reflink cannot be used together!");
-			error = -EINVAL;
-			goto out_filestream_unmount;
-		}
-	}
-
-	if (mp->m_flags & XFS_MOUNT_DISCARD) {
-		struct request_queue *q = bdev_get_queue(sb->s_bdev);
-
-		if (!blk_queue_discard(q)) {
-			xfs_warn(mp, "mounting with \"discard\" option, but "
-					"the device does not support discard");
-			mp->m_flags &= ~XFS_MOUNT_DISCARD;
-		}
-	}
-
-	if (xfs_sb_version_hasreflink(&mp->m_sb)) {
-		if (mp->m_sb.sb_rblocks) {
-			xfs_alert(mp,
-	"reflink not compatible with realtime device!");
-			error = -EINVAL;
-			goto out_filestream_unmount;
-		}
-
-		if (xfs_globals.always_cow) {
-			xfs_info(mp, "using DEBUG-only always_cow mode.");
-			mp->m_always_cow = true;
-		}
-	}
-
-	if (xfs_sb_version_hasrmapbt(&mp->m_sb) && mp->m_sb.sb_rblocks) {
-		xfs_alert(mp,
-	"reverse mapping btree not compatible with realtime device!");
-		error = -EINVAL;
-		goto out_filestream_unmount;
-	}
-
-	error = xfs_mountfs(mp);
-	if (error)
-		goto out_filestream_unmount;
-
-	root = igrab(VFS_I(mp->m_rootip));
-	if (!root) {
-		error = -ENOENT;
-		goto out_unmount;
-	}
-	sb->s_root = d_make_root(root);
-	if (!sb->s_root) {
-		error = -ENOMEM;
-		goto out_unmount;
-	}
-
-	return 0;
-
- out_filestream_unmount:
-	xfs_filestream_unmount(mp);
- out_free_sb:
-	xfs_freesb(mp);
- out_free_stats:
-	free_percpu(mp->m_stats.xs_stats);
- out_destroy_counters:
-	xfs_destroy_percpu_counters(mp);
- out_destroy_workqueues:
-	xfs_destroy_mount_workqueues(mp);
- out_close_devices:
-	xfs_close_devices(mp);
- out_free_names:
-	sb->s_fs_info = NULL;
-	xfs_mount_free(mp);
- out:
-	return error;
-
- out_unmount:
-	xfs_filestream_unmount(mp);
-	xfs_unmountfs(mp);
-	goto out_free_sb;
-}
-
 STATIC void
 xfs_fs_put_super(
 	struct super_block	*sb)

[PATCH v7 17/17] xfs: switch to use the new mount-api

[Ian Kent]

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.

With the new mount api the options parsing and the filling of the super
block are done seperately. Becuase of this it's sometimes necessary to
communicate intermediate values between the options parsing and the
fill super functions.

Define struct xfs_fc_context that holds intermediate values set from the
passed options. The fields dsunit and dswidth depend on one another so
the checks and setting of struct xfs_mount fields drom them need to be
done after options parsing. The iosizelog field could be set in the
options parsing function but the check used before setting the struct
xfs_mount field is a little more than trivial and would reduce the
readabiliy of the options parsing function so it's also added to the
struct xfs_fc_context.

I could have moved the xfs_fs_remount() function up to be with the
other mount related code to try and highlight what actually changed
when converting it to xfs_fc_reconfigure(). But the function is fairly
short and the gain in patch readability didn't appear to be worth the
extra code churn.

Finally remove unused code.

And rename xfs_fs_fill_super() to xfs_fc_fill_super() for consistency.

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

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index dd019be3fa72..3046aba9b058 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -38,6 +38,8 @@
 
 #include <linux/magic.h>
 #include <linux/parser.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 
 static const struct super_operations xfs_super_operations;
 struct bio_set xfs_ioend_bioset;
@@ -71,55 +73,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_fc_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_fc_parameters = {
+	.name		= "xfs",
+	.specs		= xfs_fc_param_specs,
+};
 
 static int
 suffix_kstrtoint(
@@ -156,60 +160,51 @@ 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;
-}
+struct xfs_fc_context {
+	int     dsunit;
+	int     dswidth;
+	uint8_t iosizelog;
+};
 
 static int
 xfs_fc_parse_param(
-	int			token,
-	char			*p,
-	substring_t		*args,
-	struct xfs_mount	*mp,
-	int			*dsunit,
-	int			*dswidth,
-	uint8_t			*iosizelog)
+	struct fs_context	*fc,
+	struct fs_parameter	*param)
 {
+	struct xfs_fc_context	*ctx = fc->fs_private;
+	struct xfs_mount	*mp = fc->s_fs_info;
+	struct fs_parse_result	result;
 	int			iosize = 0;
+	int			opt;
+
+	opt = fs_parse(fc, &xfs_fc_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, &iosize))
+		if (suffix_kstrtoint(param->string, 10, &iosize))
 			return -EINVAL;
-		*iosizelog = ffs(iosize) - 1;
+		ctx->iosizelog = ffs(iosize) - 1;
 		return 0;
 	case Opt_grpid:
 	case Opt_bsdgroups:
@@ -232,13 +227,11 @@ xfs_fc_parse_param(
 		mp->m_flags |= XFS_MOUNT_SWALLOC;
 		return 0;
 	case Opt_sunit:
-		if (match_int(args, dsunit))
-			return -EINVAL;
+		ctx->dsunit = result.uint_32;
 		return 0;
 	case Opt_swidth:
-		if (match_int(args, dswidth))
-			return -EINVAL;
-		return 0;
+		ctx->dswidth = result.uint_32;
+		return 0;;
 	case Opt_inode32:
 		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
 		return 0;
@@ -316,7 +309,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;
 	}
 
@@ -326,9 +319,7 @@ xfs_fc_parse_param(
 static int
 xfs_fc_validate_params(
 	struct xfs_mount	*mp,
-	int			dsunit,
-	int			dswidth,
-	uint8_t			iosizelog)
+	struct xfs_fc_context	*ctx)
 {
 	/*
 	 * no recovery flag requires a read-only mount
@@ -339,7 +330,8 @@ xfs_fc_validate_params(
 		return -EINVAL;
 	}
 
-	if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (dsunit || dswidth)) {
+	if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
+	    (ctx->dsunit || ctx->dswidth)) {
 		xfs_warn(mp,
 	"sunit and swidth options incompatible with the noalign option");
 		return -EINVAL;
@@ -352,27 +344,27 @@ xfs_fc_validate_params(
 	}
 #endif
 
-	if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
+	if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
 		xfs_warn(mp, "sunit and swidth must be specified together");
 		return -EINVAL;
 	}
 
-	if (dsunit && (dswidth % dsunit != 0)) {
+	if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
 		xfs_warn(mp,
 	"stripe width (%d) must be a multiple of the stripe unit (%d)",
-			dswidth, dsunit);
+			ctx->dswidth, ctx->dsunit);
 		return -EINVAL;
 	}
 
-	if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
+	if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
 		/*
 		 * At this point the superblock has not been read
 		 * in, therefore we do not know the block size.
 		 * Before the mount call ends we will convert
 		 * these to FSBs.
 		 */
-		mp->m_dalign = dsunit;
-		mp->m_swidth = dswidth;
+		mp->m_dalign = ctx->dsunit;
+		mp->m_swidth = ctx->dswidth;
 	}
 
 	if (mp->m_logbufs != -1 &&
@@ -394,18 +386,18 @@ xfs_fc_validate_params(
 		return -EINVAL;
 	}
 
-	if (iosizelog) {
-		if (iosizelog > XFS_MAX_IO_LOG ||
-		    iosizelog < XFS_MIN_IO_LOG) {
+	if (ctx->iosizelog) {
+		if (ctx->iosizelog > XFS_MAX_IO_LOG ||
+		    ctx->iosizelog < XFS_MIN_IO_LOG) {
 			xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
-				iosizelog, XFS_MIN_IO_LOG,
+				ctx->iosizelog, XFS_MIN_IO_LOG,
 				XFS_MAX_IO_LOG);
 			return -EINVAL;
 		}
 
 		mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
-		mp->m_readio_log = iosizelog;
-		mp->m_writeio_log = iosizelog;
+		mp->m_readio_log = ctx->iosizelog;
+		mp->m_writeio_log = ctx->iosizelog;
 	}
 
 	return 0;
@@ -552,92 +544,19 @@ xfs_remount_ro(
 	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];
-	int			dsunit = 0;
-	int			dswidth = 0;
-	uint8_t			iosizelog = 0;
-
-	/*
-	 * Copy binary VFS mount flags we are interested in.
-	 */
-	if (sb_rdonly(sb))
-		mp->m_flags |= XFS_MOUNT_RDONLY;
-	if (sb->s_flags & SB_DIRSYNC)
-		mp->m_flags |= XFS_MOUNT_DIRSYNC;
-	if (sb->s_flags & SB_SYNCHRONOUS)
-		mp->m_flags |= XFS_MOUNT_WSYNC;
-
-	/*
-	 * Set some default flags that could be cleared by the mount option
-	 * parsing.
-	 */
-	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
-
-	/*
-	 * These can be overridden by the mount option parsing.
-	 */
-	mp->m_logbufs = -1;
-	mp->m_logbsize = -1;
-
-	if (!options)
-		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, &dsunit, &dswidth,
-					 &iosizelog);
-		if (ret)
-			return ret;
-	}
-
-	return xfs_fc_validate_params(mp, dsunit, dswidth, iosizelog);
-}
-
-static int
-xfs_fs_fill_super(
+xfs_fc_fill_super(
 	struct super_block	*sb,
-	void			*data,
-	int			silent)
+	struct fs_context       *fc)
 {
+	struct xfs_fc_context	*ctx = fc->fs_private;
+	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, ctx);
 	if (error)
 		goto out_free_names;
 
@@ -661,7 +580,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);
@@ -806,7 +725,6 @@ xfs_fs_fill_super(
  out_free_names:
 	sb->s_fs_info = NULL;
 	xfs_mount_free(mp);
- out:
 	return error;
 
  out_unmount:
@@ -815,6 +733,147 @@ 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);
+}
+
+/*
+ * 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_fc_context	*ctx = fc->fs_private;
+	struct xfs_mount	*mp = XFS_M(fc->root->d_sb);
+	struct xfs_mount        *new_mp = fc->s_fs_info;
+	xfs_sb_t		*sbp = &mp->m_sb;
+	int			flags = fc->sb_flags;
+	int			error;
+
+	error = xfs_fc_validate_params(new_mp, ctx);
+	if (error)
+		return error;
+
+	/* inode32 -> inode64 */
+	if ((mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
+	    !(new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
+		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
+		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
+	}
+
+	/* inode64 -> inode32 */
+	if (!(mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
+	    (new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
+		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
+	}
+
+	/* ro -> rw */
+	if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(flags & SB_RDONLY)) {
+		error = xfs_remount_rw(mp);
+		if (error)
+			return error;
+	}
+
+	/* rw -> ro */
+	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
+		error = xfs_remount_ro(mp);
+		if (error)
+			return error;
+	}
+
+	return 0;
+}
+
+static void xfs_fc_free(struct fs_context *fc)
+{
+	struct xfs_fc_context	*ctx = fc->fs_private;
+	struct xfs_mount	*mp = fc->s_fs_info;
+
+	/*
+	 * mp and ctx are 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);
+	kfree(ctx);
+}
+
+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_fc_init_context(struct fs_context *fc)
+{
+	struct xfs_fc_context	*ctx;
+	struct xfs_mount	*mp;
+
+	ctx = kzalloc(sizeof(struct xfs_fc_context), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	mp = xfs_mount_alloc();
+	if (!mp) {
+		kfree(ctx);
+		return -ENOMEM;
+	}
+
+	/*
+	 * Set some default flags that could be cleared by the mount option
+	 * parsing.
+	 */
+	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
+
+	/*
+	 * These can be overridden by the mount option parsing.
+	 */
+	mp->m_logbufs = -1;
+	mp->m_logbsize = -1;
+
+	/*
+	 * Copy binary VFS mount flags we are interested in.
+	 */
+	if (fc->sb_flags & SB_RDONLY)
+		mp->m_flags |= XFS_MOUNT_RDONLY;
+	if (fc->sb_flags & SB_DIRSYNC)
+		mp->m_flags |= XFS_MOUNT_DIRSYNC;
+	if (fc->sb_flags & SB_SYNCHRONOUS)
+		mp->m_flags |= XFS_MOUNT_WSYNC;
+
+	fc->fs_private = ctx;
+	fc->s_fs_info = mp;
+	fc->ops = &xfs_context_ops;
+
+	return 0;
+}
+
+static struct file_system_type xfs_fs_type = {
+	.owner			= THIS_MODULE,
+	.name			= "xfs",
+	.init_fs_context	= xfs_fc_init_context,
+	.parameters		= &xfs_fc_parameters,
+	.kill_sb		= kill_block_super,
+	.fs_flags		= FS_REQUIRES_DEV,
+};
+
 struct proc_xfs_info {
 	uint64_t	flag;
 	char		*str;
@@ -1585,103 +1644,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_fs_remount(
-	struct super_block	*sb,
-	int			*flags,
-	char			*options)
-{
-	struct xfs_mount	*mp = XFS_M(sb);
-	xfs_sb_t		*sbp = &mp->m_sb;
-	substring_t		args[MAX_OPT_ARGS];
-	char			*p;
-	int			error;
-
-	/* First, check for complete junk; i.e. invalid options */
-	error = xfs_test_remount_options(sb, options);
-	if (error)
-		return error;
-
-	sync_filesystem(sb);
-	while ((p = strsep(&options, ",")) != NULL) {
-		int token;
-
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_inode64:
-			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
-			mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
-			break;
-		case Opt_inode32:
-			mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
-			mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
-			break;
-		default:
-			/*
-			 * Logically we would return an error here to prevent
-			 * users from believing they might have changed
-			 * mount options using remount which can't be changed.
-			 *
-			 * But unfortunately mount(8) adds all options from
-			 * mtab and fstab to the mount arguments in some cases
-			 * so we can't blindly reject options, but have to
-			 * check for each specified option if it actually
-			 * differs from the currently set option and only
-			 * reject it if that's the case.
-			 *
-			 * Until that is implemented we return success for
-			 * every remount request, and silently ignore all
-			 * options that we can't actually change.
-			 */
-#if 0
-			xfs_info(mp,
-		"mount option \"%s\" not supported for remount", p);
-			return -EINVAL;
-#else
-			break;
-#endif
-		}
-	}
-
-	/* ro -> rw */
-	if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(*flags & SB_RDONLY)) {
-		error = xfs_remount_rw(mp);
-		if (error)
-			return error;
-	}
-
-	/* rw -> ro */
-	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (*flags & SB_RDONLY)) {
-		error = xfs_remount_ro(mp);
-		if (error)
-			return error;
-	}
-
-	return 0;
-}
-
 /*
  * 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
@@ -1867,16 +1829,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,
@@ -1906,19 +1858,11 @@ 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 struct file_system_type xfs_fs_type = {
-	.owner			= THIS_MODULE,
-	.name			= "xfs",
-	.mount			= xfs_fs_mount,
-	.kill_sb		= kill_block_super,
-	.fs_flags		= FS_REQUIRES_DEV,
-};
 MODULE_ALIAS_FS("xfs");
 
 STATIC int __init

Re: [PATCH v7 17/17] xfs: switch to use the new mount-api

[Darrick J. Wong]

On Thu, Oct 24, 2019 at 03:52:04PM +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.
> 
> With the new mount api the options parsing and the filling of the super
> block are done seperately. Becuase of this it's sometimes necessary to
> communicate intermediate values between the options parsing and the
> fill super functions.
> 
> Define struct xfs_fc_context that holds intermediate values set from the
> passed options. The fields dsunit and dswidth depend on one another so
> the checks and setting of struct xfs_mount fields drom them need to be
> done after options parsing. The iosizelog field could be set in the
> options parsing function but the check used before setting the struct
> xfs_mount field is a little more than trivial and would reduce the
> readabiliy of the options parsing function so it's also added to the
> struct xfs_fc_context.
> 
> I could have moved the xfs_fs_remount() function up to be with the
> other mount related code to try and highlight what actually changed
> when converting it to xfs_fc_reconfigure(). But the function is fairly
> short and the gain in patch readability didn't appear to be worth the
> extra code churn.
> 
> Finally remove unused code.
> 
> And rename xfs_fs_fill_super() to xfs_fc_fill_super() for consistency.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>

Dunno what's up with this particular patch, but I see regressions on
generic/361 (and similar asserts on a few others).  The patches leading
up to this patch do not generate this error.

--D

[  430.789299] XFS (loop0): ino 83 data fork has delalloc extent at [0x1f7f0:0x1010]
[  430.790206] XFS: Assertion failed: 0, file: fs/xfs/xfs_super.c, line: 1390
[  430.791010] ------------[ cut here ]------------
[  430.791612] WARNING: CPU: 1 PID: 10107 at fs/xfs/xfs_message.c:104 assfail+0x32/0x50 [xfs]
[  430.792542] Modules linked in: xfs libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink bfq ip6table_filter ip6_tables iptable_filter sch_fq_codel ip_tables x_tables nfsv4 af_packet [last unloaded: xfs]
[  430.795317] CPU: 1 PID: 10107 Comm: umount Not tainted 5.4.0-rc3-djw #rc3
[  430.796081] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1 04/01/2014
[  430.797080] RIP: 0010:assfail+0x32/0x50 [xfs]
[  430.797597] Code: f1 41 89 d0 48 c7 c6 00 65 3a a0 48 89 fa 31 ff e8 13 f8 ff ff 0f b6 1d 88 65 11 00 80 fb 01 0f 87 a4 32 06 00 83 e3 01 75 04 <0f> 0b 5b c3 0f 0b 48 c7 c7 c0 a3 43 a0 e8 4c c1 09 e1 66 90 66 2e
[  430.799610] RSP: 0018:ffffc90002ddbd90 EFLAGS: 00010246
[  430.800204] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[  430.801003] RDX: 00000000ffffffc0 RSI: 000000000000000a RDI: ffffffffa0395dcf
[  430.801804] RBP: ffffe8ffc0405d00 R08: 0000000000000000 R09: 0000000000000000
[  430.802601] R10: 000000000000000a R11: f000000000000000 R12: ffff88803bc2b480
[  430.803393] R13: ffff88803bc2b718 R14: 0000000000000001 R15: 0000000000000001
[  430.804194] FS:  00007f8f354f9080(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
[  430.805097] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  430.805753] CR2: 0000561606b9bbd0 CR3: 000000002f1ce003 CR4: 00000000001606a0
[  430.806551] Call Trace:
[  430.806901]  xfs_fs_destroy_inode+0x2a3/0x4e0 [xfs]
[  430.807478]  destroy_inode+0x3b/0x80
[  430.807911]  dispose_list+0x51/0x80
[  430.808332]  evict_inodes+0x15b/0x1b0
[  430.808778]  generic_shutdown_super+0x3a/0x100
[  430.809293]  kill_block_super+0x21/0x50
[  430.809752]  deactivate_locked_super+0x2f/0x70
[  430.810274]  cleanup_mnt+0xb4/0x140
[  430.810698]  task_work_run+0x9e/0xd0
[  430.811124]  exit_to_usermode_loop+0x83/0x90
[  430.811627]  do_syscall_64+0x16d/0x180
[  430.812077]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  430.812662] RIP: 0033:0x7f8f34dbb8c7
[  430.813089] Code: 95 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 91 95 2c 00 f7 d8 64 89 01 48
[  430.815104] RSP: 002b:00007fff01ee9f98 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
[  430.815947] RAX: 0000000000000000 RBX: 000055bd5869f970 RCX: 00007f8f34dbb8c7
[  430.816743] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 000055bd5869fb50
[  430.817537] RBP: 0000000000000000 R08: 000055bd586a0900 R09: 0000000000000003
[  430.818331] R10: 000000000000000b R11: 0000000000000246 R12: 000055bd5869fb50
[  430.819131] R13: 00007f8f352dd8a4 R14: 0000000000000000 R15: 0000000000000000
[  430.819943] irq event stamp: 51598
[  430.820351] hardirqs last  enabled at (51597): [<ffffffff810d6087>] console_unlock+0x437/0x590
[  430.821300] hardirqs last disabled at (51598): [<ffffffff81001d8a>] trace_hardirqs_off_thunk+0x1a/0x20
[  430.822331] softirqs last  enabled at (51586): [<ffffffff81a003af>] __do_softirq+0x3af/0x4a4
[  430.823269] softirqs last disabled at (51579): [<ffffffff8106528c>] irq_exit+0xbc/0xe0
[  430.824152] ---[ end trace 27f9fee2eb75762a ]---
[  430.824737] XFS (loop0): Unmounting Filesystem
[  430.854250] XFS: Assertion failed: XFS_FORCED_SHUTDOWN(mp) || percpu_counter_sum(&mp->m_delalloc_blks) == 0, file: fs/xfs/xfs_super.c, line: 1803
[  430.855789] ------------[ cut here ]------------
[  430.856507] WARNING: CPU: 1 PID: 10107 at fs/xfs/xfs_message.c:104 assfail+0x32/0x50 [xfs]
[  430.857782] Modules linked in: xfs libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink bfq ip6table_filter ip6_tables iptable_filter sch_fq_codel ip_tables x_tables nfsv4 af_packet [last unloaded: xfs]
[  430.860781] CPU: 1 PID: 10107 Comm: umount Tainted: G        W         5.4.0-rc3-djw #rc3
[  430.861758] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1 04/01/2014
[  430.862832] RIP: 0010:assfail+0x32/0x50 [xfs]
[  430.863384] Code: f1 41 89 d0 48 c7 c6 00 65 3a a0 48 89 fa 31 ff e8 13 f8 ff ff 0f b6 1d 88 65 11 00 80 fb 01 0f 87 a4 32 06 00 83 e3 01 75 04 <0f> 0b 5b c3 0f 0b 48 c7 c7 c0 a3 43 a0 e8 4c c1 09 e1 66 90 66 2e
[  430.865561] RSP: 0018:ffffc90002ddbe30 EFLAGS: 00010246
[  430.866201] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[  430.867074] RDX: 00000000ffffffc0 RSI: 000000000000000a RDI: ffffffffa0395dcf
[  430.867877] RBP: ffff88807faa42a0 R08: 0000000000000000 R09: 0000000000000000
[  430.868671] R10: 000000000000000a R11: f000000000000000 R12: ffff88802f28d400
[  430.869465] R13: ffffffff81db7286 R14: ffffffff826d3560 R15: 0000000000000000
[  430.870269] FS:  00007f8f354f9080(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
[  430.871169] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  430.871833] CR2: 0000561606b9bbd0 CR3: 000000002f1ce003 CR4: 00000000001606a0
[  430.872644] Call Trace:
[  430.872994]  xfs_destroy_percpu_counters+0x6d/0x70 [xfs]
[  430.873650]  xfs_fs_put_super+0x51/0x80 [xfs]
[  430.874163]  generic_shutdown_super+0x67/0x100
[  430.874690]  kill_block_super+0x21/0x50
[  430.875146]  deactivate_locked_super+0x2f/0x70
[  430.875666]  cleanup_mnt+0xb4/0x140
[  430.876093]  task_work_run+0x9e/0xd0
[  430.876531]  exit_to_usermode_loop+0x83/0x90
[  430.877030]  do_syscall_64+0x16d/0x180
[  430.877479]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  430.878065] RIP: 0033:0x7f8f34dbb8c7
[  430.878495] Code: 95 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 91 95 2c 00 f7 d8 64 89 01 48
[  430.880503] RSP: 002b:00007fff01ee9f98 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
[  430.881347] RAX: 0000000000000000 RBX: 000055bd5869f970 RCX: 00007f8f34dbb8c7
[  430.882147] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 000055bd5869fb50
[  430.882949] RBP: 0000000000000000 R08: 000055bd586a0900 R09: 0000000000000003
[  430.883753] R10: 000000000000000b R11: 0000000000000246 R12: 000055bd5869fb50
[  430.884559] R13: 00007f8f352dd8a4 R14: 0000000000000000 R15: 0000000000000000
[  430.885361] irq event stamp: 52232
[  430.885773] hardirqs last  enabled at (52231): [<ffffffff810d6087>] console_unlock+0x437/0x590
[  430.886738] hardirqs last disabled at (52232): [<ffffffff81001d8a>] trace_hardirqs_off_thunk+0x1a/0x20
[  430.887763] softirqs last  enabled at (51614): [<ffffffff81a003af>] __do_softirq+0x3af/0x4a4
[  430.888701] softirqs last disabled at (51601): [<ffffffff8106528c>] irq_exit+0xbc/0xe0
[  430.889591] ---[ end trace 27f9fee2eb75762b ]---

> ---
>  fs/xfs/xfs_super.c |  530 +++++++++++++++++++++++-----------------------------
>  1 file changed, 237 insertions(+), 293 deletions(-)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index dd019be3fa72..3046aba9b058 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -38,6 +38,8 @@
>  
>  #include <linux/magic.h>
>  #include <linux/parser.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>  
>  static const struct super_operations xfs_super_operations;
>  struct bio_set xfs_ioend_bioset;
> @@ -71,55 +73,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_fc_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_fc_parameters = {
> +	.name		= "xfs",
> +	.specs		= xfs_fc_param_specs,
> +};
>  
>  static int
>  suffix_kstrtoint(
> @@ -156,60 +160,51 @@ 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;
> -}
> +struct xfs_fc_context {
> +	int     dsunit;
> +	int     dswidth;
> +	uint8_t iosizelog;
> +};
>  
>  static int
>  xfs_fc_parse_param(
> -	int			token,
> -	char			*p,
> -	substring_t		*args,
> -	struct xfs_mount	*mp,
> -	int			*dsunit,
> -	int			*dswidth,
> -	uint8_t			*iosizelog)
> +	struct fs_context	*fc,
> +	struct fs_parameter	*param)
>  {
> +	struct xfs_fc_context	*ctx = fc->fs_private;
> +	struct xfs_mount	*mp = fc->s_fs_info;
> +	struct fs_parse_result	result;
>  	int			iosize = 0;
> +	int			opt;
> +
> +	opt = fs_parse(fc, &xfs_fc_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, &iosize))
> +		if (suffix_kstrtoint(param->string, 10, &iosize))
>  			return -EINVAL;
> -		*iosizelog = ffs(iosize) - 1;
> +		ctx->iosizelog = ffs(iosize) - 1;
>  		return 0;
>  	case Opt_grpid:
>  	case Opt_bsdgroups:
> @@ -232,13 +227,11 @@ xfs_fc_parse_param(
>  		mp->m_flags |= XFS_MOUNT_SWALLOC;
>  		return 0;
>  	case Opt_sunit:
> -		if (match_int(args, dsunit))
> -			return -EINVAL;
> +		ctx->dsunit = result.uint_32;
>  		return 0;
>  	case Opt_swidth:
> -		if (match_int(args, dswidth))
> -			return -EINVAL;
> -		return 0;
> +		ctx->dswidth = result.uint_32;
> +		return 0;;
>  	case Opt_inode32:
>  		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
>  		return 0;
> @@ -316,7 +309,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;
>  	}
>  
> @@ -326,9 +319,7 @@ xfs_fc_parse_param(
>  static int
>  xfs_fc_validate_params(
>  	struct xfs_mount	*mp,
> -	int			dsunit,
> -	int			dswidth,
> -	uint8_t			iosizelog)
> +	struct xfs_fc_context	*ctx)
>  {
>  	/*
>  	 * no recovery flag requires a read-only mount
> @@ -339,7 +330,8 @@ xfs_fc_validate_params(
>  		return -EINVAL;
>  	}
>  
> -	if ((mp->m_flags & XFS_MOUNT_NOALIGN) && (dsunit || dswidth)) {
> +	if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
> +	    (ctx->dsunit || ctx->dswidth)) {
>  		xfs_warn(mp,
>  	"sunit and swidth options incompatible with the noalign option");
>  		return -EINVAL;
> @@ -352,27 +344,27 @@ xfs_fc_validate_params(
>  	}
>  #endif
>  
> -	if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
> +	if ((ctx->dsunit && !ctx->dswidth) || (!ctx->dsunit && ctx->dswidth)) {
>  		xfs_warn(mp, "sunit and swidth must be specified together");
>  		return -EINVAL;
>  	}
>  
> -	if (dsunit && (dswidth % dsunit != 0)) {
> +	if (ctx->dsunit && (ctx->dswidth % ctx->dsunit != 0)) {
>  		xfs_warn(mp,
>  	"stripe width (%d) must be a multiple of the stripe unit (%d)",
> -			dswidth, dsunit);
> +			ctx->dswidth, ctx->dsunit);
>  		return -EINVAL;
>  	}
>  
> -	if (dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
> +	if (ctx->dsunit && !(mp->m_flags & XFS_MOUNT_NOALIGN)) {
>  		/*
>  		 * At this point the superblock has not been read
>  		 * in, therefore we do not know the block size.
>  		 * Before the mount call ends we will convert
>  		 * these to FSBs.
>  		 */
> -		mp->m_dalign = dsunit;
> -		mp->m_swidth = dswidth;
> +		mp->m_dalign = ctx->dsunit;
> +		mp->m_swidth = ctx->dswidth;
>  	}
>  
>  	if (mp->m_logbufs != -1 &&
> @@ -394,18 +386,18 @@ xfs_fc_validate_params(
>  		return -EINVAL;
>  	}
>  
> -	if (iosizelog) {
> -		if (iosizelog > XFS_MAX_IO_LOG ||
> -		    iosizelog < XFS_MIN_IO_LOG) {
> +	if (ctx->iosizelog) {
> +		if (ctx->iosizelog > XFS_MAX_IO_LOG ||
> +		    ctx->iosizelog < XFS_MIN_IO_LOG) {
>  			xfs_warn(mp, "invalid log iosize: %d [not %d-%d]",
> -				iosizelog, XFS_MIN_IO_LOG,
> +				ctx->iosizelog, XFS_MIN_IO_LOG,
>  				XFS_MAX_IO_LOG);
>  			return -EINVAL;
>  		}
>  
>  		mp->m_flags |= XFS_MOUNT_DFLT_IOSIZE;
> -		mp->m_readio_log = iosizelog;
> -		mp->m_writeio_log = iosizelog;
> +		mp->m_readio_log = ctx->iosizelog;
> +		mp->m_writeio_log = ctx->iosizelog;
>  	}
>  
>  	return 0;
> @@ -552,92 +544,19 @@ xfs_remount_ro(
>  	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];
> -	int			dsunit = 0;
> -	int			dswidth = 0;
> -	uint8_t			iosizelog = 0;
> -
> -	/*
> -	 * Copy binary VFS mount flags we are interested in.
> -	 */
> -	if (sb_rdonly(sb))
> -		mp->m_flags |= XFS_MOUNT_RDONLY;
> -	if (sb->s_flags & SB_DIRSYNC)
> -		mp->m_flags |= XFS_MOUNT_DIRSYNC;
> -	if (sb->s_flags & SB_SYNCHRONOUS)
> -		mp->m_flags |= XFS_MOUNT_WSYNC;
> -
> -	/*
> -	 * Set some default flags that could be cleared by the mount option
> -	 * parsing.
> -	 */
> -	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> -
> -	/*
> -	 * These can be overridden by the mount option parsing.
> -	 */
> -	mp->m_logbufs = -1;
> -	mp->m_logbsize = -1;
> -
> -	if (!options)
> -		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, &dsunit, &dswidth,
> -					 &iosizelog);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	return xfs_fc_validate_params(mp, dsunit, dswidth, iosizelog);
> -}
> -
> -static int
> -xfs_fs_fill_super(
> +xfs_fc_fill_super(
>  	struct super_block	*sb,
> -	void			*data,
> -	int			silent)
> +	struct fs_context       *fc)
>  {
> +	struct xfs_fc_context	*ctx = fc->fs_private;
> +	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, ctx);
>  	if (error)
>  		goto out_free_names;
>  
> @@ -661,7 +580,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);
> @@ -806,7 +725,6 @@ xfs_fs_fill_super(
>   out_free_names:
>  	sb->s_fs_info = NULL;
>  	xfs_mount_free(mp);
> - out:
>  	return error;
>  
>   out_unmount:
> @@ -815,6 +733,147 @@ 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);
> +}
> +
> +/*
> + * 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_fc_context	*ctx = fc->fs_private;
> +	struct xfs_mount	*mp = XFS_M(fc->root->d_sb);
> +	struct xfs_mount        *new_mp = fc->s_fs_info;
> +	xfs_sb_t		*sbp = &mp->m_sb;
> +	int			flags = fc->sb_flags;
> +	int			error;
> +
> +	error = xfs_fc_validate_params(new_mp, ctx);
> +	if (error)
> +		return error;
> +
> +	/* inode32 -> inode64 */
> +	if ((mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
> +	    !(new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
> +		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> +		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
> +	}
> +
> +	/* inode64 -> inode32 */
> +	if (!(mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
> +	    (new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
> +		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> +		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
> +	}
> +
> +	/* ro -> rw */
> +	if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(flags & SB_RDONLY)) {
> +		error = xfs_remount_rw(mp);
> +		if (error)
> +			return error;
> +	}
> +
> +	/* rw -> ro */
> +	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
> +		error = xfs_remount_ro(mp);
> +		if (error)
> +			return error;
> +	}
> +
> +	return 0;
> +}
> +
> +static void xfs_fc_free(struct fs_context *fc)
> +{
> +	struct xfs_fc_context	*ctx = fc->fs_private;
> +	struct xfs_mount	*mp = fc->s_fs_info;
> +
> +	/*
> +	 * mp and ctx are 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);
> +	kfree(ctx);
> +}
> +
> +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_fc_init_context(struct fs_context *fc)
> +{
> +	struct xfs_fc_context	*ctx;
> +	struct xfs_mount	*mp;
> +
> +	ctx = kzalloc(sizeof(struct xfs_fc_context), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mp = xfs_mount_alloc();
> +	if (!mp) {
> +		kfree(ctx);
> +		return -ENOMEM;
> +	}
> +
> +	/*
> +	 * Set some default flags that could be cleared by the mount option
> +	 * parsing.
> +	 */
> +	mp->m_flags |= XFS_MOUNT_COMPAT_IOSIZE;
> +
> +	/*
> +	 * These can be overridden by the mount option parsing.
> +	 */
> +	mp->m_logbufs = -1;
> +	mp->m_logbsize = -1;
> +
> +	/*
> +	 * Copy binary VFS mount flags we are interested in.
> +	 */
> +	if (fc->sb_flags & SB_RDONLY)
> +		mp->m_flags |= XFS_MOUNT_RDONLY;
> +	if (fc->sb_flags & SB_DIRSYNC)
> +		mp->m_flags |= XFS_MOUNT_DIRSYNC;
> +	if (fc->sb_flags & SB_SYNCHRONOUS)
> +		mp->m_flags |= XFS_MOUNT_WSYNC;
> +
> +	fc->fs_private = ctx;
> +	fc->s_fs_info = mp;
> +	fc->ops = &xfs_context_ops;
> +
> +	return 0;
> +}
> +
> +static struct file_system_type xfs_fs_type = {
> +	.owner			= THIS_MODULE,
> +	.name			= "xfs",
> +	.init_fs_context	= xfs_fc_init_context,
> +	.parameters		= &xfs_fc_parameters,
> +	.kill_sb		= kill_block_super,
> +	.fs_flags		= FS_REQUIRES_DEV,
> +};
> +
>  struct proc_xfs_info {
>  	uint64_t	flag;
>  	char		*str;
> @@ -1585,103 +1644,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_fs_remount(
> -	struct super_block	*sb,
> -	int			*flags,
> -	char			*options)
> -{
> -	struct xfs_mount	*mp = XFS_M(sb);
> -	xfs_sb_t		*sbp = &mp->m_sb;
> -	substring_t		args[MAX_OPT_ARGS];
> -	char			*p;
> -	int			error;
> -
> -	/* First, check for complete junk; i.e. invalid options */
> -	error = xfs_test_remount_options(sb, options);
> -	if (error)
> -		return error;
> -
> -	sync_filesystem(sb);
> -	while ((p = strsep(&options, ",")) != NULL) {
> -		int token;
> -
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case Opt_inode64:
> -			mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> -			mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
> -			break;
> -		case Opt_inode32:
> -			mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> -			mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
> -			break;
> -		default:
> -			/*
> -			 * Logically we would return an error here to prevent
> -			 * users from believing they might have changed
> -			 * mount options using remount which can't be changed.
> -			 *
> -			 * But unfortunately mount(8) adds all options from
> -			 * mtab and fstab to the mount arguments in some cases
> -			 * so we can't blindly reject options, but have to
> -			 * check for each specified option if it actually
> -			 * differs from the currently set option and only
> -			 * reject it if that's the case.
> -			 *
> -			 * Until that is implemented we return success for
> -			 * every remount request, and silently ignore all
> -			 * options that we can't actually change.
> -			 */
> -#if 0
> -			xfs_info(mp,
> -		"mount option \"%s\" not supported for remount", p);
> -			return -EINVAL;
> -#else
> -			break;
> -#endif
> -		}
> -	}
> -
> -	/* ro -> rw */
> -	if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(*flags & SB_RDONLY)) {
> -		error = xfs_remount_rw(mp);
> -		if (error)
> -			return error;
> -	}
> -
> -	/* rw -> ro */
> -	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (*flags & SB_RDONLY)) {
> -		error = xfs_remount_ro(mp);
> -		if (error)
> -			return error;
> -	}
> -
> -	return 0;
> -}
> -
>  /*
>   * 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
> @@ -1867,16 +1829,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,
> @@ -1906,19 +1858,11 @@ 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 struct file_system_type xfs_fs_type = {
> -	.owner			= THIS_MODULE,
> -	.name			= "xfs",
> -	.mount			= xfs_fs_mount,
> -	.kill_sb		= kill_block_super,
> -	.fs_flags		= FS_REQUIRES_DEV,
> -};
>  MODULE_ALIAS_FS("xfs");
>  
>  STATIC int __init
>