All of lore.kernel.org
 help / color / mirror / Atom feed
* [lustre-devel] [PATCH 0/3] lustre: llite: rework client mount code
@ 2018-07-30 23:16 James Simmons
  2018-07-30 23:16 ` [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite James Simmons
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: James Simmons @ 2018-07-30 23:16 UTC (permalink / raw)
  To: lustre-devel

Most of the lustre mount code is stuffed in obd_mount.c. That code
was designed with support for both servers and clients. Move the
code that is specific to just the client to the llite layer which
makes the code simpler.

James Simmons (3):
  lustre: obd: migrate lustre_fill_super() to llite
  lustre: llite: handle registering file system
  lustre: llite: simplify lustre_fill_super()

 .../staging/lustre/lustre/include/lustre_disk.h    |   6 +-
 drivers/staging/lustre/lustre/llite/llite_lib.c    |   3 +
 drivers/staging/lustre/lustre/llite/super25.c      |  98 +++++++++++-
 drivers/staging/lustre/lustre/obdclass/class_obd.c |   6 -
 drivers/staging/lustre/lustre/obdclass/obd_mount.c | 165 ++-------------------
 5 files changed, 115 insertions(+), 163 deletions(-)

-- 
1.8.3.1

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

* [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite
  2018-07-30 23:16 [lustre-devel] [PATCH 0/3] lustre: llite: rework client mount code James Simmons
@ 2018-07-30 23:16 ` James Simmons
  2018-07-31  3:13   ` NeilBrown
  2018-07-30 23:16 ` [lustre-devel] [PATCH 2/3] lustre: llite: handle registering file system James Simmons
  2018-07-30 23:16 ` [lustre-devel] [PATCH 3/3] lustre: llite: simplify lustre_fill_super() James Simmons
  2 siblings, 1 reply; 8+ messages in thread
From: James Simmons @ 2018-07-30 23:16 UTC (permalink / raw)
  To: lustre-devel

Currently both obdclass and ptlrpc are built as separate modules
with ptlrpc being a child of obdclass. A recent change placed
ptlrpc_[inc|dec]_ref() into the obdclass code i.e obd_mount.c
which now causes a curricular module dependency. Examining the
code the bulk use of these functions are in lustre_fill_super().
So move lustre_fill_super() to the llite layer where it really
belongs. This change also allows lustre_fill_super() to be
simplified and we can push lustre_fill_super() instead of
ll_fill_super(). Also ptlrpc_dec_ref() needed to be pulled out
of lustre_common_put_super() as well. Once obdclass and ptlrpc
are combined into one module we can restore ptlrpc_dec_rec()
in lustre_common_put_super().

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 .../staging/lustre/lustre/include/lustre_disk.h    |   9 +-
 drivers/staging/lustre/lustre/llite/llite_lib.c    |   3 +
 drivers/staging/lustre/lustre/llite/super25.c      |  83 +++++++++++++-
 drivers/staging/lustre/lustre/obdclass/obd_mount.c | 125 +++++----------------
 4 files changed, 115 insertions(+), 105 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h
index 1a6d421..772ecc9 100644
--- a/drivers/staging/lustre/lustre/include/lustre_disk.h
+++ b/drivers/staging/lustre/lustre/include/lustre_disk.h
@@ -141,10 +141,13 @@ struct lustre_sb_info {
 
 /* obd_mount.c */
 
+struct lustre_sb_info *lustre_init_lsi(struct super_block *sb);
+int lustre_put_lsi(struct super_block *sb);
+int lmd_parse(char *options, struct lustre_mount_data *lmd);
 int lustre_start_mgc(struct super_block *sb);
-void lustre_register_super_ops(struct module *mod,
-			       int (*cfs)(struct super_block *sb),
-			       void (*ksc)(struct super_block *sb));
+void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
+					  int silent),
+                               void (*ksc)(struct super_block *sb));
 int lustre_common_put_super(struct super_block *sb);
 
 int mgc_fsname2resid(char *fsname, struct ldlm_res_id *res_id, int type);
diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 5c8d0fe..87a929c 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -914,6 +914,7 @@ int ll_fill_super(struct super_block *sb)
 	cfg = kzalloc(sizeof(*cfg), GFP_NOFS);
 	if (!cfg) {
 		lustre_common_put_super(sb);
+		ptlrpc_dec_ref();
 		return -ENOMEM;
 	}
 
@@ -926,6 +927,7 @@ int ll_fill_super(struct super_block *sb)
 		module_put(THIS_MODULE);
 		kfree(cfg);
 		lustre_common_put_super(sb);
+		ptlrpc_dec_ref();
 		return -ENOMEM;
 	}
 
@@ -1059,6 +1061,7 @@ void ll_put_super(struct super_block *sb)
 	lsi->lsi_llsbi = NULL;
 
 	lustre_common_put_super(sb);
+	ptlrpc_dec_ref();
 
 	cl_env_cache_purge(~0);
 
diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
index d335f29..de43d58 100644
--- a/drivers/staging/lustre/lustre/llite/super25.c
+++ b/drivers/staging/lustre/lustre/llite/super25.c
@@ -83,6 +83,85 @@ struct super_operations lustre_super_operations = {
 };
 MODULE_ALIAS_FS("lustre");
 
+/** This is the entry point for the mount call into Lustre.
+ * This is called when a server or client is mounted,
+ * and this is where we start setting things up.
+ * @param data Mount options (e.g. -o flock,abort_recov)
+ */
+int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
+{
+	struct lustre_mount_data *lmd;
+	struct lustre_sb_info *lsi;
+	int rc;
+
+	CDEBUG(D_SUPER | D_CONFIG | D_VFSTRACE, "VFS Op: sb %p\n", sb);
+
+	lsi = lustre_init_lsi(sb);
+	if (!lsi)
+		return -ENOMEM;
+	lmd = lsi->lsi_lmd;
+
+	/*
+	 * Disable lockdep during mount, because mount locking patterns are
+	 * `special'.
+	 */
+	lockdep_off();
+
+	/*
+	 * LU-639: the obd cleanup of last mount may not finish yet, wait here.
+	 */
+	obd_zombie_barrier();
+
+	/* Figure out the lmd from the mount options */
+	if (lmd_parse(lmd2_data, lmd)) {
+		rc = -EINVAL;
+		goto out_put_lsi;
+	}
+
+	if (lmd_is_client(lmd)) {
+		CDEBUG(D_SUPER | D_CONFIG, "Mounting client %s\n",
+		       lmd->lmd_profile);
+		rc = ptlrpc_inc_ref();
+		if (rc)
+			goto out_put_lsi;
+
+		rc = lustre_start_mgc(sb);
+		if (rc) {
+			/* This will put_lsi and ptlrpc_dec_ref() */
+			lustre_common_put_super(sb);
+			ptlrpc_dec_ref();
+			goto out;
+		}
+
+		/* Connect and start */
+		rc = ll_fill_super(sb);
+
+		/*
+		 * c_f_s will call lustre_common_put_super on failure, otherwise
+		 * c_f_s will have taken another reference to the module
+		 */
+	} else {
+		CERROR("This is client-side-only module, cannot handle server mount.\n");
+		rc = -EINVAL;
+	}
+	/* If error happens in fill_super() call, @lsi will be killed there.
+	 * This is why we do not put it here.
+	 */
+	goto out;
+out_put_lsi:
+	lustre_put_lsi(sb);
+out:
+	if (rc) {
+		CERROR("Unable to mount %s (%d)\n",
+		       s2lsi(sb) ? lmd->lmd_dev : "", rc);
+	} else {
+		CDEBUG(D_SUPER, "Mount %s complete\n",
+		       lmd->lmd_dev);
+	}
+	lockdep_on();
+	return rc;
+}
+
 static int __init lustre_init(void)
 {
 	int rc;
@@ -145,7 +224,7 @@ static int __init lustre_init(void)
 	if (rc != 0)
 		goto out_inode_fini_env;
 
-	lustre_register_super_ops(THIS_MODULE, ll_fill_super, ll_kill_super);
+	lustre_register_super_ops(lustre_fill_super, ll_kill_super);
 	lustre_register_client_process_config(ll_process_config);
 
 	return 0;
@@ -166,7 +245,7 @@ static int __init lustre_init(void)
 
 static void __exit lustre_exit(void)
 {
-	lustre_register_super_ops(NULL, NULL, NULL);
+	lustre_register_super_ops(NULL, NULL);
 	lustre_register_client_process_config(NULL);
 
 	debugfs_remove(llite_root);
diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
index 1bd5613..ac841f4 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
@@ -50,8 +50,8 @@
 #include <uapi/linux/lustre/lustre_param.h>
 
 static DEFINE_SPINLOCK(client_lock);
-static struct module *client_mod;
-static int (*client_fill_super)(struct super_block *sb);
+static int (*client_fill_super)(struct super_block *sb, void *data,
+                                int silent);
 static void (*kill_super_cb)(struct super_block *sb);
 
 /**************** config llog ********************/
@@ -430,6 +430,7 @@ int lustre_start_mgc(struct super_block *sb)
 	kfree(niduuid);
 	return rc;
 }
+EXPORT_SYMBOL(lustre_start_mgc);
 
 static int lustre_stop_mgc(struct super_block *sb)
 {
@@ -507,7 +508,7 @@ static int lustre_stop_mgc(struct super_block *sb)
 
 /***************** lustre superblock **************/
 
-static struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
+struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
 {
 	struct lustre_sb_info *lsi;
 
@@ -532,6 +533,7 @@ static struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
 
 	return lsi;
 }
+EXPORT_SYMBOL(lustre_init_lsi);
 
 static int lustre_free_lsi(struct super_block *sb)
 {
@@ -567,7 +569,7 @@ static int lustre_free_lsi(struct super_block *sb)
 /* The lsi has one reference for every server that is using the disk -
  * e.g. MDT, MGS, and potentially MGC
  */
-static int lustre_put_lsi(struct super_block *sb)
+int lustre_put_lsi(struct super_block *sb)
 {
 	struct lustre_sb_info *lsi = s2lsi(sb);
 
@@ -578,6 +580,7 @@ static int lustre_put_lsi(struct super_block *sb)
 	}
 	return 0;
 }
+EXPORT_SYMBOL(lustre_put_lsi);
 
 /*** SERVER NAME ***
  * <FSNAME><SEPARATOR><TYPE><INDEX>
@@ -686,7 +689,12 @@ int lustre_common_put_super(struct super_block *sb)
 	}
 	/* Drop a ref to the mounted disk */
 	lustre_put_lsi(sb);
-	ptlrpc_dec_ref();
+	/* !!! FIXME !!!!! */
+	/* ptlrpc_dec_ref() should go here but will
+	 * cause curricular module dependencies. Once
+	 * obdclass and ptlrpc are merged into one
+	 * module ptlrpc_dec_ref() can come back here
+	 */
 	return rc;
 }
 EXPORT_SYMBOL(lustre_common_put_super);
@@ -992,7 +1000,7 @@ static int lmd_parse_nidlist(char *buf, char **endh)
  * e.g. mount -v -t lustre -o abort_recov uml1:uml2:/lustre-client /mnt/lustre
  * dev is passed as device=uml1:/lustre by mount.lustre
  */
-static int lmd_parse(char *options, struct lustre_mount_data *lmd)
+int lmd_parse(char *options, struct lustre_mount_data *lmd)
 {
 	char *s1, *s2, *devname = NULL;
 	struct lustre_mount_data *raw = (struct lustre_mount_data *)options;
@@ -1216,106 +1224,16 @@ static int lmd_parse(char *options, struct lustre_mount_data *lmd)
 	CERROR("Bad mount options %s\n", options);
 	return -EINVAL;
 }
-
-/** This is the entry point for the mount call into Lustre.
- * This is called when a server or client is mounted,
- * and this is where we start setting things up.
- * @param data Mount options (e.g. -o flock,abort_recov)
- */
-static int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
-{
-	struct lustre_mount_data *lmd;
-	struct lustre_sb_info *lsi;
-	int rc;
-
-	CDEBUG(D_MOUNT | D_VFSTRACE, "VFS Op: sb %p\n", sb);
-
-	lsi = lustre_init_lsi(sb);
-	if (!lsi)
-		return -ENOMEM;
-	lmd = lsi->lsi_lmd;
-
-	/*
-	 * Disable lockdep during mount, because mount locking patterns are
-	 * `special'.
-	 */
-	lockdep_off();
-
-	/*
-	 * LU-639: the obd cleanup of last mount may not finish yet, wait here.
-	 */
-	obd_zombie_barrier();
-
-	/* Figure out the lmd from the mount options */
-	if (lmd_parse(lmd2_data, lmd)) {
-		rc = -EINVAL;
-		goto out_put_lsi;
-	}
-
-	if (lmd_is_client(lmd)) {
-		bool have_client = false;
-		CDEBUG(D_MOUNT, "Mounting client %s\n", lmd->lmd_profile);
-		if (!client_fill_super)
-			request_module("lustre");
-		rc = ptlrpc_inc_ref();
-		if (rc)
-			goto out_put_lsi;
-		spin_lock(&client_lock);
-		if (client_fill_super && try_module_get(client_mod))
-			have_client = true;
-		spin_unlock(&client_lock);
-		if (!have_client) {
-			LCONSOLE_ERROR_MSG(0x165, "Nothing registered for client mount! Is the 'lustre' module loaded?\n");
-			lustre_put_lsi(sb);
-			rc = -ENODEV;
-		} else {
-			rc = lustre_start_mgc(sb);
-			if (rc) {
-				/* This will put_lsi and ptlrpc_dec_ref() */
-				lustre_common_put_super(sb);
-				goto out;
-			}
-			/* Connect and start */
-			/* (should always be ll_fill_super) */
-			rc = (*client_fill_super)(sb);
-			/*
-			 * c_f_s will call lustre_common_put_super on failure, otherwise
-			 * c_f_s will have taken another reference to the module
-			 */
-			module_put(client_mod);
-		}
-	} else {
-		CERROR("This is client-side-only module, cannot handle server mount.\n");
-		rc = -EINVAL;
-	}
-
-	/* If error happens in fill_super() call, @lsi will be killed there.
-	 * This is why we do not put it here.
-	 */
-	goto out;
-out_put_lsi:
-	lustre_put_lsi(sb);
-out:
-	if (rc) {
-		CERROR("Unable to mount %s (%d)\n",
-		       s2lsi(sb) ? lmd->lmd_dev : "", rc);
-	} else {
-		CDEBUG(D_SUPER, "Mount %s complete\n",
-		       lmd->lmd_dev);
-	}
-	lockdep_on();
-	return rc;
-}
+EXPORT_SYMBOL(lmd_parse);
 
 /* We can't call ll_fill_super by name because it lives in a module that
  * must be loaded after this one.
  */
-void lustre_register_super_ops(struct module *mod,
-			       int (*cfs)(struct super_block *sb),
+void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
+					  int silent),
 			       void (*ksc)(struct super_block *sb))
 {
 	spin_lock(&client_lock);
-	client_mod = mod;
 	client_fill_super = cfs;
 	kill_super_cb = ksc;
 	spin_unlock(&client_lock);
@@ -1326,7 +1244,14 @@ void lustre_register_super_ops(struct module *mod,
 static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
 				   const char *devname, void *data)
 {
-	return mount_nodev(fs_type, flags, data, lustre_fill_super);
+	struct dentry *root;
+
+	spin_lock(&client_lock);
+	if (!client_fill_super)
+		request_module("lustre");
+	root = mount_nodev(fs_type, flags, data, client_fill_super);
+	spin_unlock(&client_lock);
+	return root;
 }
 
 static void lustre_kill_super(struct super_block *sb)
-- 
1.8.3.1

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

* [lustre-devel] [PATCH 2/3] lustre: llite: handle registering file system
  2018-07-30 23:16 [lustre-devel] [PATCH 0/3] lustre: llite: rework client mount code James Simmons
  2018-07-30 23:16 ` [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite James Simmons
@ 2018-07-30 23:16 ` James Simmons
  2018-08-01  0:13   ` Andreas Dilger
  2018-07-30 23:16 ` [lustre-devel] [PATCH 3/3] lustre: llite: simplify lustre_fill_super() James Simmons
  2 siblings, 1 reply; 8+ messages in thread
From: James Simmons @ 2018-07-30 23:16 UTC (permalink / raw)
  To: lustre-devel

Move the code that registers struct file_system_type for lustre
with the VFS layer to the llite layer where it belongs. This
removes the ugly function pointer passing between obdclass and
llite.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 .../staging/lustre/lustre/include/lustre_disk.h    |  3 -
 drivers/staging/lustre/lustre/llite/super25.c      | 34 +++++++++++-
 drivers/staging/lustre/lustre/obdclass/class_obd.c |  6 --
 drivers/staging/lustre/lustre/obdclass/obd_mount.c | 64 ----------------------
 4 files changed, 31 insertions(+), 76 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h
index 772ecc9..d5fadde 100644
--- a/drivers/staging/lustre/lustre/include/lustre_disk.h
+++ b/drivers/staging/lustre/lustre/include/lustre_disk.h
@@ -145,9 +145,6 @@ struct lustre_sb_info {
 int lustre_put_lsi(struct super_block *sb);
 int lmd_parse(char *options, struct lustre_mount_data *lmd);
 int lustre_start_mgc(struct super_block *sb);
-void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
-					  int silent),
-                               void (*ksc)(struct super_block *sb));
 int lustre_common_put_super(struct super_block *sb);
 
 int mgc_fsname2resid(char *fsname, struct ldlm_res_id *res_id, int type);
diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
index de43d58..ac8f6f1 100644
--- a/drivers/staging/lustre/lustre/llite/super25.c
+++ b/drivers/staging/lustre/lustre/llite/super25.c
@@ -81,7 +81,6 @@ struct super_operations lustre_super_operations = {
 	.remount_fs    = ll_remount_fs,
 	.show_options  = ll_show_options,
 };
-MODULE_ALIAS_FS("lustre");
 
 /** This is the entry point for the mount call into Lustre.
  * This is called when a server or client is mounted,
@@ -162,6 +161,30 @@ int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
 	return rc;
 }
 
+/***************** FS registration ******************/
+static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
+				   const char *devname, void *data)
+{
+	return mount_nodev(fs_type, flags, data, lustre_fill_super);
+}
+
+static void lustre_kill_super(struct super_block *sb)
+{
+	ll_kill_super(sb);
+	kill_anon_super(sb);
+}
+
+/** Register the "lustre" fs type
+ */
+static struct file_system_type lustre_fs_type = {
+	.owner		= THIS_MODULE,
+	.name		= "lustre",
+	.mount		= lustre_mount,
+	.kill_sb	= lustre_kill_super,
+	.fs_flags	= FS_RENAME_DOES_D_MOVE,
+};
+MODULE_ALIAS_FS("lustre");
+
 static int __init lustre_init(void)
 {
 	int rc;
@@ -224,11 +247,16 @@ static int __init lustre_init(void)
 	if (rc != 0)
 		goto out_inode_fini_env;
 
-	lustre_register_super_ops(lustre_fill_super, ll_kill_super);
+	rc = register_filesystem(&lustre_fs_type);
+	if (rc)
+		goto out_xattr_cache;
+
 	lustre_register_client_process_config(ll_process_config);
 
 	return 0;
 
+out_xattr_cache:
+	ll_xattr_fini();
 out_inode_fini_env:
 	cl_env_put(cl_inode_fini_env, &cl_inode_fini_refcheck);
 out_vvp:
@@ -245,8 +273,8 @@ static int __init lustre_init(void)
 
 static void __exit lustre_exit(void)
 {
-	lustre_register_super_ops(NULL, NULL);
 	lustre_register_client_process_config(NULL);
+	unregister_filesystem(&lustre_fs_type);
 
 	debugfs_remove(llite_root);
 	kset_unregister(llite_kset);
diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
index 81a4c66..cdaf729 100644
--- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
+++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
@@ -510,18 +510,12 @@ static int __init obdclass_init(void)
 		return err;
 
 	err = llog_info_init();
-	if (err)
-		return err;
-
-	err = lustre_register_fs();
 
 	return err;
 }
 
 static void obdclass_exit(void)
 {
-	lustre_unregister_fs();
-
 	misc_deregister(&obd_psdev);
 	llog_info_fini();
 	cl_global_fini();
diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
index ac841f4..b84bca4 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
@@ -49,11 +49,6 @@
 #include <lustre_disk.h>
 #include <uapi/linux/lustre/lustre_param.h>
 
-static DEFINE_SPINLOCK(client_lock);
-static int (*client_fill_super)(struct super_block *sb, void *data,
-                                int silent);
-static void (*kill_super_cb)(struct super_block *sb);
-
 /**************** config llog ********************/
 
 /** Get a config log from the MGS and process it.
@@ -1225,62 +1220,3 @@ int lmd_parse(char *options, struct lustre_mount_data *lmd)
 	return -EINVAL;
 }
 EXPORT_SYMBOL(lmd_parse);
-
-/* We can't call ll_fill_super by name because it lives in a module that
- * must be loaded after this one.
- */
-void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
-					  int silent),
-			       void (*ksc)(struct super_block *sb))
-{
-	spin_lock(&client_lock);
-	client_fill_super = cfs;
-	kill_super_cb = ksc;
-	spin_unlock(&client_lock);
-}
-EXPORT_SYMBOL(lustre_register_super_ops);
-
-/***************** FS registration ******************/
-static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
-				   const char *devname, void *data)
-{
-	struct dentry *root;
-
-	spin_lock(&client_lock);
-	if (!client_fill_super)
-		request_module("lustre");
-	root = mount_nodev(fs_type, flags, data, client_fill_super);
-	spin_unlock(&client_lock);
-	return root;
-}
-
-static void lustre_kill_super(struct super_block *sb)
-{
-	struct lustre_sb_info *lsi = s2lsi(sb);
-
-	if (kill_super_cb && lsi)
-		(*kill_super_cb)(sb);
-
-	kill_anon_super(sb);
-}
-
-/** Register the "lustre" fs type
- */
-static struct file_system_type lustre_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "lustre",
-	.mount		= lustre_mount,
-	.kill_sb	= lustre_kill_super,
-	.fs_flags	= FS_RENAME_DOES_D_MOVE,
-};
-MODULE_ALIAS_FS("lustre");
-
-int lustre_register_fs(void)
-{
-	return register_filesystem(&lustre_fs_type);
-}
-
-int lustre_unregister_fs(void)
-{
-	return unregister_filesystem(&lustre_fs_type);
-}
-- 
1.8.3.1

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

* [lustre-devel] [PATCH 3/3] lustre: llite: simplify lustre_fill_super()
  2018-07-30 23:16 [lustre-devel] [PATCH 0/3] lustre: llite: rework client mount code James Simmons
  2018-07-30 23:16 ` [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite James Simmons
  2018-07-30 23:16 ` [lustre-devel] [PATCH 2/3] lustre: llite: handle registering file system James Simmons
@ 2018-07-30 23:16 ` James Simmons
  2 siblings, 0 replies; 8+ messages in thread
From: James Simmons @ 2018-07-30 23:16 UTC (permalink / raw)
  To: lustre-devel

With lustre_fill_super() being client specific we can easily
simplify the code.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/super25.c | 51 ++++++++++-----------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
index ac8f6f1..1ada5a7 100644
--- a/drivers/staging/lustre/lustre/llite/super25.c
+++ b/drivers/staging/lustre/lustre/llite/super25.c
@@ -113,42 +113,29 @@ int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
 
 	/* Figure out the lmd from the mount options */
 	if (lmd_parse(lmd2_data, lmd)) {
+		lustre_put_lsi(sb);
 		rc = -EINVAL;
-		goto out_put_lsi;
+		goto out;
 	}
 
-	if (lmd_is_client(lmd)) {
-		CDEBUG(D_SUPER | D_CONFIG, "Mounting client %s\n",
-		       lmd->lmd_profile);
-		rc = ptlrpc_inc_ref();
-		if (rc)
-			goto out_put_lsi;
-
-		rc = lustre_start_mgc(sb);
-		if (rc) {
-			/* This will put_lsi and ptlrpc_dec_ref() */
-			lustre_common_put_super(sb);
-			ptlrpc_dec_ref();
-			goto out;
-		}
-
-		/* Connect and start */
-		rc = ll_fill_super(sb);
-
-		/*
-		 * c_f_s will call lustre_common_put_super on failure, otherwise
-		 * c_f_s will have taken another reference to the module
-		 */
-	} else {
-		CERROR("This is client-side-only module, cannot handle server mount.\n");
-		rc = -EINVAL;
+	CDEBUG(D_SUPER | D_CONFIG, "Mounting client %s\n",
+	       lmd->lmd_profile);
+	rc = ptlrpc_inc_ref();
+	if (rc) {
+		lustre_put_lsi(sb);
+		goto out;
 	}
-	/* If error happens in fill_super() call, @lsi will be killed there.
-	 * This is why we do not put it here.
-	 */
-	goto out;
-out_put_lsi:
-	lustre_put_lsi(sb);
+
+	rc = lustre_start_mgc(sb);
+	if (rc) {
+		/* This will put_lsi and ptlrpc_dec_ref() */
+		lustre_common_put_super(sb);
+		ptlrpc_dec_ref();
+		goto out;
+	}
+
+	/* Connect and start. If error happens, @lsi will be killed there */
+	rc = ll_fill_super(sb);
 out:
 	if (rc) {
 		CERROR("Unable to mount %s (%d)\n",
-- 
1.8.3.1

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

* [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite
  2018-07-30 23:16 ` [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite James Simmons
@ 2018-07-31  3:13   ` NeilBrown
  2018-08-01  3:17     ` NeilBrown
  0 siblings, 1 reply; 8+ messages in thread
From: NeilBrown @ 2018-07-31  3:13 UTC (permalink / raw)
  To: lustre-devel

On Mon, Jul 30 2018, James Simmons wrote:

> Currently both obdclass and ptlrpc are built as separate modules
> with ptlrpc being a child of obdclass. A recent change placed
> ptlrpc_[inc|dec]_ref() into the obdclass code i.e obd_mount.c
> which now causes a curricular module dependency. Examining the

Oops, that was careless - sorry about that.
Maybe I should just revert that patch .... but that probably
doesn't actually make things easier.  Let's go with your fix.


> code the bulk use of these functions are in lustre_fill_super().
> So move lustre_fill_super() to the llite layer where it really
> belongs. This change also allows lustre_fill_super() to be
> simplified and we can push lustre_fill_super() instead of
> ll_fill_super(). Also ptlrpc_dec_ref() needed to be pulled out
> of lustre_common_put_super() as well. Once obdclass and ptlrpc
> are combined into one module we can restore ptlrpc_dec_rec()
> in lustre_common_put_super().

The path forward for unify the modules is not as smooth as I hoped.
Masahiro Yamada (kbuild maintainer) doesn't like the idea of building
one modules from multiple directories at all.  I might try again,
but it isn't looking good.

Two further comments below...

>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  .../staging/lustre/lustre/include/lustre_disk.h    |   9 +-
>  drivers/staging/lustre/lustre/llite/llite_lib.c    |   3 +
>  drivers/staging/lustre/lustre/llite/super25.c      |  83 +++++++++++++-
>  drivers/staging/lustre/lustre/obdclass/obd_mount.c | 125 +++++----------------
>  4 files changed, 115 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h
> index 1a6d421..772ecc9 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_disk.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_disk.h
> @@ -141,10 +141,13 @@ struct lustre_sb_info {
>  
>  /* obd_mount.c */
>  
> +struct lustre_sb_info *lustre_init_lsi(struct super_block *sb);
> +int lustre_put_lsi(struct super_block *sb);
> +int lmd_parse(char *options, struct lustre_mount_data *lmd);
>  int lustre_start_mgc(struct super_block *sb);
> -void lustre_register_super_ops(struct module *mod,
> -			       int (*cfs)(struct super_block *sb),
> -			       void (*ksc)(struct super_block *sb));
> +void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
> +					  int silent),
> +                               void (*ksc)(struct super_block *sb));
>  int lustre_common_put_super(struct super_block *sb);
>  
>  int mgc_fsname2resid(char *fsname, struct ldlm_res_id *res_id, int type);
> diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
> index 5c8d0fe..87a929c 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_lib.c
> +++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
> @@ -914,6 +914,7 @@ int ll_fill_super(struct super_block *sb)
>  	cfg = kzalloc(sizeof(*cfg), GFP_NOFS);
>  	if (!cfg) {
>  		lustre_common_put_super(sb);
> +		ptlrpc_dec_ref();
>  		return -ENOMEM;
>  	}
>  
> @@ -926,6 +927,7 @@ int ll_fill_super(struct super_block *sb)
>  		module_put(THIS_MODULE);
>  		kfree(cfg);
>  		lustre_common_put_super(sb);
> +		ptlrpc_dec_ref();
>  		return -ENOMEM;
>  	}
>  
> @@ -1059,6 +1061,7 @@ void ll_put_super(struct super_block *sb)
>  	lsi->lsi_llsbi = NULL;
>  
>  	lustre_common_put_super(sb);
> +	ptlrpc_dec_ref();
>  
>  	cl_env_cache_purge(~0);
>  
> diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
> index d335f29..de43d58 100644
> --- a/drivers/staging/lustre/lustre/llite/super25.c
> +++ b/drivers/staging/lustre/lustre/llite/super25.c
> @@ -83,6 +83,85 @@ struct super_operations lustre_super_operations = {
>  };
>  MODULE_ALIAS_FS("lustre");
>  
> +/** This is the entry point for the mount call into Lustre.
> + * This is called when a server or client is mounted,
> + * and this is where we start setting things up.
> + * @param data Mount options (e.g. -o flock,abort_recov)
> + */
> +int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
> +{
> +	struct lustre_mount_data *lmd;
> +	struct lustre_sb_info *lsi;
> +	int rc;
> +
> +	CDEBUG(D_SUPER | D_CONFIG | D_VFSTRACE, "VFS Op: sb %p\n", sb);
> +
> +	lsi = lustre_init_lsi(sb);
> +	if (!lsi)
> +		return -ENOMEM;
> +	lmd = lsi->lsi_lmd;
> +
> +	/*
> +	 * Disable lockdep during mount, because mount locking patterns are
> +	 * `special'.
> +	 */
> +	lockdep_off();
> +
> +	/*
> +	 * LU-639: the obd cleanup of last mount may not finish yet, wait here.
> +	 */
> +	obd_zombie_barrier();
> +
> +	/* Figure out the lmd from the mount options */
> +	if (lmd_parse(lmd2_data, lmd)) {
> +		rc = -EINVAL;
> +		goto out_put_lsi;
> +	}
> +
> +	if (lmd_is_client(lmd)) {
> +		CDEBUG(D_SUPER | D_CONFIG, "Mounting client %s\n",
> +		       lmd->lmd_profile);
> +		rc = ptlrpc_inc_ref();
> +		if (rc)
> +			goto out_put_lsi;
> +
> +		rc = lustre_start_mgc(sb);
> +		if (rc) {
> +			/* This will put_lsi and ptlrpc_dec_ref() */

This comment is now wrong, or at least odd.

> +			lustre_common_put_super(sb);
> +			ptlrpc_dec_ref();
> +			goto out;
> +		}
> +
> +		/* Connect and start */
> +		rc = ll_fill_super(sb);
> +
> +		/*
> +		 * c_f_s will call lustre_common_put_super on failure, otherwise
> +		 * c_f_s will have taken another reference to the module
> +		 */
> +	} else {
> +		CERROR("This is client-side-only module, cannot handle server mount.\n");
> +		rc = -EINVAL;
> +	}
> +	/* If error happens in fill_super() call, @lsi will be killed there.
> +	 * This is why we do not put it here.
> +	 */
> +	goto out;
> +out_put_lsi:
> +	lustre_put_lsi(sb);
> +out:
> +	if (rc) {
> +		CERROR("Unable to mount %s (%d)\n",
> +		       s2lsi(sb) ? lmd->lmd_dev : "", rc);
> +	} else {
> +		CDEBUG(D_SUPER, "Mount %s complete\n",
> +		       lmd->lmd_dev);
> +	}
> +	lockdep_on();
> +	return rc;
> +}
> +
>  static int __init lustre_init(void)
>  {
>  	int rc;
> @@ -145,7 +224,7 @@ static int __init lustre_init(void)
>  	if (rc != 0)
>  		goto out_inode_fini_env;
>  
> -	lustre_register_super_ops(THIS_MODULE, ll_fill_super, ll_kill_super);
> +	lustre_register_super_ops(lustre_fill_super, ll_kill_super);
>  	lustre_register_client_process_config(ll_process_config);
>  
>  	return 0;
> @@ -166,7 +245,7 @@ static int __init lustre_init(void)
>  
>  static void __exit lustre_exit(void)
>  {
> -	lustre_register_super_ops(NULL, NULL, NULL);
> +	lustre_register_super_ops(NULL, NULL);
>  	lustre_register_client_process_config(NULL);
>  
>  	debugfs_remove(llite_root);
> diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> index 1bd5613..ac841f4 100644
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -50,8 +50,8 @@
>  #include <uapi/linux/lustre/lustre_param.h>
>  
>  static DEFINE_SPINLOCK(client_lock);
> -static struct module *client_mod;
> -static int (*client_fill_super)(struct super_block *sb);
> +static int (*client_fill_super)(struct super_block *sb, void *data,
> +                                int silent);
>  static void (*kill_super_cb)(struct super_block *sb);
>  
>  /**************** config llog ********************/
> @@ -430,6 +430,7 @@ int lustre_start_mgc(struct super_block *sb)
>  	kfree(niduuid);
>  	return rc;
>  }
> +EXPORT_SYMBOL(lustre_start_mgc);
>  
>  static int lustre_stop_mgc(struct super_block *sb)
>  {
> @@ -507,7 +508,7 @@ static int lustre_stop_mgc(struct super_block *sb)
>  
>  /***************** lustre superblock **************/
>  
> -static struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
> +struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
>  {
>  	struct lustre_sb_info *lsi;
>  
> @@ -532,6 +533,7 @@ static struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
>  
>  	return lsi;
>  }
> +EXPORT_SYMBOL(lustre_init_lsi);
>  
>  static int lustre_free_lsi(struct super_block *sb)
>  {
> @@ -567,7 +569,7 @@ static int lustre_free_lsi(struct super_block *sb)
>  /* The lsi has one reference for every server that is using the disk -
>   * e.g. MDT, MGS, and potentially MGC
>   */
> -static int lustre_put_lsi(struct super_block *sb)
> +int lustre_put_lsi(struct super_block *sb)
>  {
>  	struct lustre_sb_info *lsi = s2lsi(sb);
>  
> @@ -578,6 +580,7 @@ static int lustre_put_lsi(struct super_block *sb)
>  	}
>  	return 0;
>  }
> +EXPORT_SYMBOL(lustre_put_lsi);
>  
>  /*** SERVER NAME ***
>   * <FSNAME><SEPARATOR><TYPE><INDEX>
> @@ -686,7 +689,12 @@ int lustre_common_put_super(struct super_block *sb)
>  	}
>  	/* Drop a ref to the mounted disk */
>  	lustre_put_lsi(sb);
> -	ptlrpc_dec_ref();
> +	/* !!! FIXME !!!!! */
> +	/* ptlrpc_dec_ref() should go here but will
> +	 * cause curricular module dependencies. Once
> +	 * obdclass and ptlrpc are merged into one
> +	 * module ptlrpc_dec_ref() can come back here
> +	 */
>  	return rc;
>  }
>  EXPORT_SYMBOL(lustre_common_put_super);
> @@ -992,7 +1000,7 @@ static int lmd_parse_nidlist(char *buf, char **endh)
>   * e.g. mount -v -t lustre -o abort_recov uml1:uml2:/lustre-client /mnt/lustre
>   * dev is passed as device=uml1:/lustre by mount.lustre
>   */
> -static int lmd_parse(char *options, struct lustre_mount_data *lmd)
> +int lmd_parse(char *options, struct lustre_mount_data *lmd)
>  {
>  	char *s1, *s2, *devname = NULL;
>  	struct lustre_mount_data *raw = (struct lustre_mount_data *)options;
> @@ -1216,106 +1224,16 @@ static int lmd_parse(char *options, struct lustre_mount_data *lmd)
>  	CERROR("Bad mount options %s\n", options);
>  	return -EINVAL;
>  }
> -
> -/** This is the entry point for the mount call into Lustre.
> - * This is called when a server or client is mounted,
> - * and this is where we start setting things up.
> - * @param data Mount options (e.g. -o flock,abort_recov)
> - */
> -static int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
> -{
> -	struct lustre_mount_data *lmd;
> -	struct lustre_sb_info *lsi;
> -	int rc;
> -
> -	CDEBUG(D_MOUNT | D_VFSTRACE, "VFS Op: sb %p\n", sb);
> -
> -	lsi = lustre_init_lsi(sb);
> -	if (!lsi)
> -		return -ENOMEM;
> -	lmd = lsi->lsi_lmd;
> -
> -	/*
> -	 * Disable lockdep during mount, because mount locking patterns are
> -	 * `special'.
> -	 */
> -	lockdep_off();
> -
> -	/*
> -	 * LU-639: the obd cleanup of last mount may not finish yet, wait here.
> -	 */
> -	obd_zombie_barrier();
> -
> -	/* Figure out the lmd from the mount options */
> -	if (lmd_parse(lmd2_data, lmd)) {
> -		rc = -EINVAL;
> -		goto out_put_lsi;
> -	}
> -
> -	if (lmd_is_client(lmd)) {
> -		bool have_client = false;
> -		CDEBUG(D_MOUNT, "Mounting client %s\n", lmd->lmd_profile);
> -		if (!client_fill_super)
> -			request_module("lustre");
> -		rc = ptlrpc_inc_ref();
> -		if (rc)
> -			goto out_put_lsi;
> -		spin_lock(&client_lock);
> -		if (client_fill_super && try_module_get(client_mod))
> -			have_client = true;
> -		spin_unlock(&client_lock);
> -		if (!have_client) {
> -			LCONSOLE_ERROR_MSG(0x165, "Nothing registered for client mount! Is the 'lustre' module loaded?\n");
> -			lustre_put_lsi(sb);
> -			rc = -ENODEV;
> -		} else {
> -			rc = lustre_start_mgc(sb);
> -			if (rc) {
> -				/* This will put_lsi and ptlrpc_dec_ref() */
> -				lustre_common_put_super(sb);
> -				goto out;
> -			}
> -			/* Connect and start */
> -			/* (should always be ll_fill_super) */
> -			rc = (*client_fill_super)(sb);
> -			/*
> -			 * c_f_s will call lustre_common_put_super on failure, otherwise
> -			 * c_f_s will have taken another reference to the module
> -			 */
> -			module_put(client_mod);
> -		}
> -	} else {
> -		CERROR("This is client-side-only module, cannot handle server mount.\n");
> -		rc = -EINVAL;
> -	}
> -
> -	/* If error happens in fill_super() call, @lsi will be killed there.
> -	 * This is why we do not put it here.
> -	 */
> -	goto out;
> -out_put_lsi:
> -	lustre_put_lsi(sb);
> -out:
> -	if (rc) {
> -		CERROR("Unable to mount %s (%d)\n",
> -		       s2lsi(sb) ? lmd->lmd_dev : "", rc);
> -	} else {
> -		CDEBUG(D_SUPER, "Mount %s complete\n",
> -		       lmd->lmd_dev);
> -	}
> -	lockdep_on();
> -	return rc;
> -}
> +EXPORT_SYMBOL(lmd_parse);
>  
>  /* We can't call ll_fill_super by name because it lives in a module that
>   * must be loaded after this one.
>   */
> -void lustre_register_super_ops(struct module *mod,
> -			       int (*cfs)(struct super_block *sb),
> +void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
> +					  int silent),
>  			       void (*ksc)(struct super_block *sb))
>  {
>  	spin_lock(&client_lock);
> -	client_mod = mod;
>  	client_fill_super = cfs;
>  	kill_super_cb = ksc;
>  	spin_unlock(&client_lock);
> @@ -1326,7 +1244,14 @@ void lustre_register_super_ops(struct module *mod,
>  static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
>  				   const char *devname, void *data)
>  {
> -	return mount_nodev(fs_type, flags, data, lustre_fill_super);
> +	struct dentry *root;
> +
> +	spin_lock(&client_lock);
> +	if (!client_fill_super)
> +		request_module("lustre");
> +	root = mount_nodev(fs_type, flags, data, client_fill_super);
> +	spin_unlock(&client_lock);

You cannot call request_module() inside spinlock - it waits for
'modprobe' to run.

I think it is best to move all the bits that need to move between
modules all at once.

Thanks,
NeilBrown


> +	return root;
>  }
>  
>  static void lustre_kill_super(struct super_block *sb)
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180731/399c22aa/attachment.sig>

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

* [lustre-devel] [PATCH 2/3] lustre: llite: handle registering file system
  2018-07-30 23:16 ` [lustre-devel] [PATCH 2/3] lustre: llite: handle registering file system James Simmons
@ 2018-08-01  0:13   ` Andreas Dilger
  2018-08-01  1:14     ` NeilBrown
  0 siblings, 1 reply; 8+ messages in thread
From: Andreas Dilger @ 2018-08-01  0:13 UTC (permalink / raw)
  To: lustre-devel

On Jul 30, 2018, at 17:16, James Simmons <jsimmons@infradead.org> wrote:
> 
> Move the code that registers struct file_system_type for lustre
> with the VFS layer to the llite layer where it belongs. This
> removes the ugly function pointer passing between obdclass and
> llite.

One potential issue here is that "mount -t lustre" may get confused
because the mount(8) binary (or kernel, I forget where it is these days)
will modprobe "lustre" when trying to mount the filesystem.  This
can fail if some odd combinations of modules are loaded, since obdclass
used to register the "lustre" filesystem type, but the client needs
"llite" loaded to actually mount, while the server does not.  This
might end up causing "llite" to always be loaded on the server if this
patch were included into master.

One option that I've through for a while is to change the servers to
mount with "-t mdt" and "-t ost", or "-t lustre_mdt" or maybe just
"-t lustre_server" or something similar.  I don't have a great love
for any of these options, so I'm open for suggestions.  Doing this sooner
rather than later will ease the transition at some point in the future.

We would add this as an additional filesystem type for the modules, and
still allow "-t lustre" for some time for compatibility reasons.  I like
the "lustre_mdt" and "lustre_ost" filesystem types, but using just "mdt"
and "ost" might be the best options, since this would result in the
"mdt.ko" and "ost.ko" modules being auto-loaded at mount time.

Cheers, Andreas


> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
> .../staging/lustre/lustre/include/lustre_disk.h    |  3 -
> drivers/staging/lustre/lustre/llite/super25.c      | 34 +++++++++++-
> drivers/staging/lustre/lustre/obdclass/class_obd.c |  6 --
> drivers/staging/lustre/lustre/obdclass/obd_mount.c | 64 ----------------------
> 4 files changed, 31 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h
> index 772ecc9..d5fadde 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_disk.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_disk.h
> @@ -145,9 +145,6 @@ struct lustre_sb_info {
> int lustre_put_lsi(struct super_block *sb);
> int lmd_parse(char *options, struct lustre_mount_data *lmd);
> int lustre_start_mgc(struct super_block *sb);
> -void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
> -					  int silent),
> -                               void (*ksc)(struct super_block *sb));
> int lustre_common_put_super(struct super_block *sb);
> 
> int mgc_fsname2resid(char *fsname, struct ldlm_res_id *res_id, int type);
> diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
> index de43d58..ac8f6f1 100644
> --- a/drivers/staging/lustre/lustre/llite/super25.c
> +++ b/drivers/staging/lustre/lustre/llite/super25.c
> @@ -81,7 +81,6 @@ struct super_operations lustre_super_operations = {
> 	.remount_fs    = ll_remount_fs,
> 	.show_options  = ll_show_options,
> };
> -MODULE_ALIAS_FS("lustre");
> 
> /** This is the entry point for the mount call into Lustre.
>  * This is called when a server or client is mounted,
> @@ -162,6 +161,30 @@ int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
> 	return rc;
> }
> 
> +/***************** FS registration ******************/
> +static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
> +				   const char *devname, void *data)
> +{
> +	return mount_nodev(fs_type, flags, data, lustre_fill_super);
> +}
> +
> +static void lustre_kill_super(struct super_block *sb)
> +{
> +	ll_kill_super(sb);
> +	kill_anon_super(sb);
> +}
> +
> +/** Register the "lustre" fs type
> + */
> +static struct file_system_type lustre_fs_type = {
> +	.owner		= THIS_MODULE,
> +	.name		= "lustre",
> +	.mount		= lustre_mount,
> +	.kill_sb	= lustre_kill_super,
> +	.fs_flags	= FS_RENAME_DOES_D_MOVE,
> +};
> +MODULE_ALIAS_FS("lustre");
> +
> static int __init lustre_init(void)
> {
> 	int rc;
> @@ -224,11 +247,16 @@ static int __init lustre_init(void)
> 	if (rc != 0)
> 		goto out_inode_fini_env;
> 
> -	lustre_register_super_ops(lustre_fill_super, ll_kill_super);
> +	rc = register_filesystem(&lustre_fs_type);
> +	if (rc)
> +		goto out_xattr_cache;
> +
> 	lustre_register_client_process_config(ll_process_config);
> 
> 	return 0;
> 
> +out_xattr_cache:
> +	ll_xattr_fini();
> out_inode_fini_env:
> 	cl_env_put(cl_inode_fini_env, &cl_inode_fini_refcheck);
> out_vvp:
> @@ -245,8 +273,8 @@ static int __init lustre_init(void)
> 
> static void __exit lustre_exit(void)
> {
> -	lustre_register_super_ops(NULL, NULL);
> 	lustre_register_client_process_config(NULL);
> +	unregister_filesystem(&lustre_fs_type);
> 
> 	debugfs_remove(llite_root);
> 	kset_unregister(llite_kset);
> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> index 81a4c66..cdaf729 100644
> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> @@ -510,18 +510,12 @@ static int __init obdclass_init(void)
> 		return err;
> 
> 	err = llog_info_init();
> -	if (err)
> -		return err;
> -
> -	err = lustre_register_fs();
> 
> 	return err;
> }
> 
> static void obdclass_exit(void)
> {
> -	lustre_unregister_fs();
> -
> 	misc_deregister(&obd_psdev);
> 	llog_info_fini();
> 	cl_global_fini();
> diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> index ac841f4..b84bca4 100644
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -49,11 +49,6 @@
> #include <lustre_disk.h>
> #include <uapi/linux/lustre/lustre_param.h>
> 
> -static DEFINE_SPINLOCK(client_lock);
> -static int (*client_fill_super)(struct super_block *sb, void *data,
> -                                int silent);
> -static void (*kill_super_cb)(struct super_block *sb);
> -
> /**************** config llog ********************/
> 
> /** Get a config log from the MGS and process it.
> @@ -1225,62 +1220,3 @@ int lmd_parse(char *options, struct lustre_mount_data *lmd)
> 	return -EINVAL;
> }
> EXPORT_SYMBOL(lmd_parse);
> -
> -/* We can't call ll_fill_super by name because it lives in a module that
> - * must be loaded after this one.
> - */
> -void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
> -					  int silent),
> -			       void (*ksc)(struct super_block *sb))
> -{
> -	spin_lock(&client_lock);
> -	client_fill_super = cfs;
> -	kill_super_cb = ksc;
> -	spin_unlock(&client_lock);
> -}
> -EXPORT_SYMBOL(lustre_register_super_ops);
> -
> -/***************** FS registration ******************/
> -static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
> -				   const char *devname, void *data)
> -{
> -	struct dentry *root;
> -
> -	spin_lock(&client_lock);
> -	if (!client_fill_super)
> -		request_module("lustre");
> -	root = mount_nodev(fs_type, flags, data, client_fill_super);
> -	spin_unlock(&client_lock);
> -	return root;
> -}
> -
> -static void lustre_kill_super(struct super_block *sb)
> -{
> -	struct lustre_sb_info *lsi = s2lsi(sb);
> -
> -	if (kill_super_cb && lsi)
> -		(*kill_super_cb)(sb);
> -
> -	kill_anon_super(sb);
> -}
> -
> -/** Register the "lustre" fs type
> - */
> -static struct file_system_type lustre_fs_type = {
> -	.owner		= THIS_MODULE,
> -	.name		= "lustre",
> -	.mount		= lustre_mount,
> -	.kill_sb	= lustre_kill_super,
> -	.fs_flags	= FS_RENAME_DOES_D_MOVE,
> -};
> -MODULE_ALIAS_FS("lustre");
> -
> -int lustre_register_fs(void)
> -{
> -	return register_filesystem(&lustre_fs_type);
> -}
> -
> -int lustre_unregister_fs(void)
> -{
> -	return unregister_filesystem(&lustre_fs_type);
> -}
> --
> 1.8.3.1
> 

Cheers, Andreas
---
Andreas Dilger
CTO Whamcloud




-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 235 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180801/aa8aa40d/attachment.sig>

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

* [lustre-devel] [PATCH 2/3] lustre: llite: handle registering file system
  2018-08-01  0:13   ` Andreas Dilger
@ 2018-08-01  1:14     ` NeilBrown
  0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2018-08-01  1:14 UTC (permalink / raw)
  To: lustre-devel

On Wed, Aug 01 2018, Andreas Dilger wrote:

> On Jul 30, 2018, at 17:16, James Simmons <jsimmons@infradead.org> wrote:
>> 
>> Move the code that registers struct file_system_type for lustre
>> with the VFS layer to the llite layer where it belongs. This
>> removes the ugly function pointer passing between obdclass and
>> llite.
>
> One potential issue here is that "mount -t lustre" may get confused
> because the mount(8) binary (or kernel, I forget where it is these days)
> will modprobe "lustre" when trying to mount the filesystem.  This
> can fail if some odd combinations of modules are loaded, since obdclass
> used to register the "lustre" filesystem type, but the client needs
> "llite" loaded to actually mount, while the server does not.  This
> might end up causing "llite" to always be loaded on the server if this
> patch were included into master.
>
> One option that I've through for a while is to change the servers to
> mount with "-t mdt" and "-t ost", or "-t lustre_mdt" or maybe just
> "-t lustre_server" or something similar.  I don't have a great love
> for any of these options, so I'm open for suggestions.  Doing this sooner
> rather than later will ease the transition at some point in the future.
>
> We would add this as an additional filesystem type for the modules, and
> still allow "-t lustre" for some time for compatibility reasons.  I like
> the "lustre_mdt" and "lustre_ost" filesystem types, but using just "mdt"
> and "ost" might be the best options, since this would result in the
> "mdt.ko" and "ost.ko" modules being auto-loaded at mount time.

I think that having the same filesystem type "lustre" be used both to
mount on a client and to activate a server is a particularly unfortunate
aspect of the lustre design.  I really think it needs to change, and I'm
glad you seem to think so too.

I'm not keen on using "mdt" or "ost" as they are meaningless to people
outside of lustre (and I still have to think twice to decode
... metadata target and object storage target..).
I think "lustre_mdt" and "lustre_ost" are good suggestions.  Creating
module aliases is trivial.

I think it would be really good if upstream lustre could start accepting
these for server-side mounts soon.   When the server code eventually
lands in Linux, I expect it to only support (something like) lustre_mdt
and lustre_ost for server-side mounts.

Thanks,
NeilBrown

>
> Cheers, Andreas
>
>
>> Signed-off-by: James Simmons <jsimmons@infradead.org>
>> ---
>> .../staging/lustre/lustre/include/lustre_disk.h    |  3 -
>> drivers/staging/lustre/lustre/llite/super25.c      | 34 +++++++++++-
>> drivers/staging/lustre/lustre/obdclass/class_obd.c |  6 --
>> drivers/staging/lustre/lustre/obdclass/obd_mount.c | 64 ----------------------
>> 4 files changed, 31 insertions(+), 76 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h
>> index 772ecc9..d5fadde 100644
>> --- a/drivers/staging/lustre/lustre/include/lustre_disk.h
>> +++ b/drivers/staging/lustre/lustre/include/lustre_disk.h
>> @@ -145,9 +145,6 @@ struct lustre_sb_info {
>> int lustre_put_lsi(struct super_block *sb);
>> int lmd_parse(char *options, struct lustre_mount_data *lmd);
>> int lustre_start_mgc(struct super_block *sb);
>> -void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
>> -					  int silent),
>> -                               void (*ksc)(struct super_block *sb));
>> int lustre_common_put_super(struct super_block *sb);
>> 
>> int mgc_fsname2resid(char *fsname, struct ldlm_res_id *res_id, int type);
>> diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
>> index de43d58..ac8f6f1 100644
>> --- a/drivers/staging/lustre/lustre/llite/super25.c
>> +++ b/drivers/staging/lustre/lustre/llite/super25.c
>> @@ -81,7 +81,6 @@ struct super_operations lustre_super_operations = {
>> 	.remount_fs    = ll_remount_fs,
>> 	.show_options  = ll_show_options,
>> };
>> -MODULE_ALIAS_FS("lustre");
>> 
>> /** This is the entry point for the mount call into Lustre.
>>  * This is called when a server or client is mounted,
>> @@ -162,6 +161,30 @@ int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
>> 	return rc;
>> }
>> 
>> +/***************** FS registration ******************/
>> +static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
>> +				   const char *devname, void *data)
>> +{
>> +	return mount_nodev(fs_type, flags, data, lustre_fill_super);
>> +}
>> +
>> +static void lustre_kill_super(struct super_block *sb)
>> +{
>> +	ll_kill_super(sb);
>> +	kill_anon_super(sb);
>> +}
>> +
>> +/** Register the "lustre" fs type
>> + */
>> +static struct file_system_type lustre_fs_type = {
>> +	.owner		= THIS_MODULE,
>> +	.name		= "lustre",
>> +	.mount		= lustre_mount,
>> +	.kill_sb	= lustre_kill_super,
>> +	.fs_flags	= FS_RENAME_DOES_D_MOVE,
>> +};
>> +MODULE_ALIAS_FS("lustre");
>> +
>> static int __init lustre_init(void)
>> {
>> 	int rc;
>> @@ -224,11 +247,16 @@ static int __init lustre_init(void)
>> 	if (rc != 0)
>> 		goto out_inode_fini_env;
>> 
>> -	lustre_register_super_ops(lustre_fill_super, ll_kill_super);
>> +	rc = register_filesystem(&lustre_fs_type);
>> +	if (rc)
>> +		goto out_xattr_cache;
>> +
>> 	lustre_register_client_process_config(ll_process_config);
>> 
>> 	return 0;
>> 
>> +out_xattr_cache:
>> +	ll_xattr_fini();
>> out_inode_fini_env:
>> 	cl_env_put(cl_inode_fini_env, &cl_inode_fini_refcheck);
>> out_vvp:
>> @@ -245,8 +273,8 @@ static int __init lustre_init(void)
>> 
>> static void __exit lustre_exit(void)
>> {
>> -	lustre_register_super_ops(NULL, NULL);
>> 	lustre_register_client_process_config(NULL);
>> +	unregister_filesystem(&lustre_fs_type);
>> 
>> 	debugfs_remove(llite_root);
>> 	kset_unregister(llite_kset);
>> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> index 81a4c66..cdaf729 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> @@ -510,18 +510,12 @@ static int __init obdclass_init(void)
>> 		return err;
>> 
>> 	err = llog_info_init();
>> -	if (err)
>> -		return err;
>> -
>> -	err = lustre_register_fs();
>> 
>> 	return err;
>> }
>> 
>> static void obdclass_exit(void)
>> {
>> -	lustre_unregister_fs();
>> -
>> 	misc_deregister(&obd_psdev);
>> 	llog_info_fini();
>> 	cl_global_fini();
>> diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
>> index ac841f4..b84bca4 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
>> @@ -49,11 +49,6 @@
>> #include <lustre_disk.h>
>> #include <uapi/linux/lustre/lustre_param.h>
>> 
>> -static DEFINE_SPINLOCK(client_lock);
>> -static int (*client_fill_super)(struct super_block *sb, void *data,
>> -                                int silent);
>> -static void (*kill_super_cb)(struct super_block *sb);
>> -
>> /**************** config llog ********************/
>> 
>> /** Get a config log from the MGS and process it.
>> @@ -1225,62 +1220,3 @@ int lmd_parse(char *options, struct lustre_mount_data *lmd)
>> 	return -EINVAL;
>> }
>> EXPORT_SYMBOL(lmd_parse);
>> -
>> -/* We can't call ll_fill_super by name because it lives in a module that
>> - * must be loaded after this one.
>> - */
>> -void lustre_register_super_ops(int (*cfs)(struct super_block *sb, void *data,
>> -					  int silent),
>> -			       void (*ksc)(struct super_block *sb))
>> -{
>> -	spin_lock(&client_lock);
>> -	client_fill_super = cfs;
>> -	kill_super_cb = ksc;
>> -	spin_unlock(&client_lock);
>> -}
>> -EXPORT_SYMBOL(lustre_register_super_ops);
>> -
>> -/***************** FS registration ******************/
>> -static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
>> -				   const char *devname, void *data)
>> -{
>> -	struct dentry *root;
>> -
>> -	spin_lock(&client_lock);
>> -	if (!client_fill_super)
>> -		request_module("lustre");
>> -	root = mount_nodev(fs_type, flags, data, client_fill_super);
>> -	spin_unlock(&client_lock);
>> -	return root;
>> -}
>> -
>> -static void lustre_kill_super(struct super_block *sb)
>> -{
>> -	struct lustre_sb_info *lsi = s2lsi(sb);
>> -
>> -	if (kill_super_cb && lsi)
>> -		(*kill_super_cb)(sb);
>> -
>> -	kill_anon_super(sb);
>> -}
>> -
>> -/** Register the "lustre" fs type
>> - */
>> -static struct file_system_type lustre_fs_type = {
>> -	.owner		= THIS_MODULE,
>> -	.name		= "lustre",
>> -	.mount		= lustre_mount,
>> -	.kill_sb	= lustre_kill_super,
>> -	.fs_flags	= FS_RENAME_DOES_D_MOVE,
>> -};
>> -MODULE_ALIAS_FS("lustre");
>> -
>> -int lustre_register_fs(void)
>> -{
>> -	return register_filesystem(&lustre_fs_type);
>> -}
>> -
>> -int lustre_unregister_fs(void)
>> -{
>> -	return unregister_filesystem(&lustre_fs_type);
>> -}
>> --
>> 1.8.3.1
>> 
>
> Cheers, Andreas
> ---
> Andreas Dilger
> CTO Whamcloud
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180801/884d0f16/attachment.sig>

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

* [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite
  2018-07-31  3:13   ` NeilBrown
@ 2018-08-01  3:17     ` NeilBrown
  0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2018-08-01  3:17 UTC (permalink / raw)
  To: lustre-devel

On Tue, Jul 31 2018, NeilBrown wrote:

> On Mon, Jul 30 2018, James Simmons wrote:
>
>> Currently both obdclass and ptlrpc are built as separate modules
>> with ptlrpc being a child of obdclass. A recent change placed
>> ptlrpc_[inc|dec]_ref() into the obdclass code i.e obd_mount.c
>> which now causes a curricular module dependency. Examining the
>
> Oops, that was careless - sorry about that.
> Maybe I should just revert that patch .... but that probably
> doesn't actually make things easier.  Let's go with your fix.

Actually, I changed my mind - sorry.

I've reverted the offending patch, and written a patch that just
moves the minimum from obdclass to llite.
There is room for further cleaning up...

Thanks,
NeilBrown

From cb6578971fd9e1d1dedb6e88629741dcf8e31318 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.com>
Date: Wed, 1 Aug 2018 10:09:39 +1000
Subject: [PATCH] lustre: move client mounting from obdclass to llite.

Mounting a lustre client is currently handled
in obdclass, using services from llite.
This requires obdclass to load the llite module
and set up inter-module linkage.

The purpose of this was for common code to support both
client and server mounts.  This isn't really a good idea
and need to go.  When we add the server, it will use a
separate filesystem type.

So move the mounting code from obdclass/obd_mount to llite/super25
and remove the inter-module linkages.
Add some EXPORT_SYMBOL() so that llite can access some helpers
that remain in obdclass.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/include/lustre_disk.h    |   7 +-
 drivers/staging/lustre/lustre/llite/super25.c      | 116 +++++++++++++++-
 drivers/staging/lustre/lustre/obdclass/class_obd.c |   4 -
 drivers/staging/lustre/lustre/obdclass/obd_mount.c | 152 +--------------------
 4 files changed, 125 insertions(+), 154 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h
index 1a6d421ea64b..1d18b2fd210d 100644
--- a/drivers/staging/lustre/lustre/include/lustre_disk.h
+++ b/drivers/staging/lustre/lustre/include/lustre_disk.h
@@ -142,13 +142,14 @@ struct lustre_sb_info {
 /* obd_mount.c */
 
 int lustre_start_mgc(struct super_block *sb);
-void lustre_register_super_ops(struct module *mod,
-			       int (*cfs)(struct super_block *sb),
-			       void (*ksc)(struct super_block *sb));
 int lustre_common_put_super(struct super_block *sb);
 
 int mgc_fsname2resid(char *fsname, struct ldlm_res_id *res_id, int type);
 
+struct lustre_sb_info *lustre_init_lsi(struct super_block *sb);
+int lustre_put_lsi(struct super_block *sb);
+int lmd_parse(char *options, struct lustre_mount_data *lmd);
+
 /** @} disk */
 
 #endif /* _LUSTRE_DISK_H */
diff --git a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
index d335f29556c2..966b0564c952 100644
--- a/drivers/staging/lustre/lustre/llite/super25.c
+++ b/drivers/staging/lustre/lustre/llite/super25.c
@@ -32,6 +32,7 @@
  */
 
 #define DEBUG_SUBSYSTEM S_LLITE
+#define D_MOUNT (D_SUPER | D_CONFIG/*|D_WARNING */)
 
 #include <linux/module.h>
 #include <linux/types.h>
@@ -81,8 +82,115 @@ struct super_operations lustre_super_operations = {
 	.remount_fs    = ll_remount_fs,
 	.show_options  = ll_show_options,
 };
+
+/** This is the entry point for the mount call into Lustre.
+ * This is called when a server or client is mounted,
+ * and this is where we start setting things up.
+ * @param data Mount options (e.g. -o flock,abort_recov)
+ */
+static int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
+{
+	struct lustre_mount_data *lmd;
+	struct lustre_sb_info *lsi;
+	int rc;
+
+	CDEBUG(D_MOUNT | D_VFSTRACE, "VFS Op: sb %p\n", sb);
+
+	lsi = lustre_init_lsi(sb);
+	if (!lsi)
+		return -ENOMEM;
+	lmd = lsi->lsi_lmd;
+
+	/*
+	 * Disable lockdep during mount, because mount locking patterns are
+	 * `special'.
+	 */
+	lockdep_off();
+
+	/*
+	 * LU-639: the obd cleanup of last mount may not finish yet, wait here.
+	 */
+	obd_zombie_barrier();
+
+	/* Figure out the lmd from the mount options */
+	if (lmd_parse(lmd2_data, lmd)) {
+		lustre_put_lsi(sb);
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (lmd_is_client(lmd)) {
+		CDEBUG(D_MOUNT, "Mounting client %s\n", lmd->lmd_profile);
+
+		rc = lustre_start_mgc(sb);
+		if (rc) {
+			lustre_common_put_super(sb);
+			goto out;
+		}
+		/* Connect and start */
+		rc = ll_fill_super(sb);
+		/*
+		 * c_f_s will call lustre_common_put_super on failure, otherwise
+		 * c_f_s will have taken another reference to the module
+		 */
+	} else {
+		CERROR("This is client-side-only module, cannot handle server mount.\n");
+		rc = -EINVAL;
+	}
+
+	/* If error happens in fill_super() call, @lsi will be killed there.
+	 * This is why we do not put it here.
+	 */
+	goto out;
+out:
+	if (rc) {
+		CERROR("Unable to mount %s (%d)\n",
+		       s2lsi(sb) ? lmd->lmd_dev : "", rc);
+	} else {
+		CDEBUG(D_SUPER, "Mount %s complete\n",
+		       lmd->lmd_dev);
+	}
+	lockdep_on();
+	return rc;
+}
+
+/***************** FS registration ******************/
+static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
+				   const char *devname, void *data)
+{
+	return mount_nodev(fs_type, flags, data, lustre_fill_super);
+}
+
+static void lustre_kill_super(struct super_block *sb)
+{
+	struct lustre_sb_info *lsi = s2lsi(sb);
+
+	if (lsi)
+		ll_kill_super(sb);
+
+	kill_anon_super(sb);
+}
+
+/** Register the "lustre" fs type
+ */
+static struct file_system_type lustre_fs_type = {
+	.owner		= THIS_MODULE,
+	.name		= "lustre",
+	.mount		= lustre_mount,
+	.kill_sb	= lustre_kill_super,
+	.fs_flags	= FS_RENAME_DOES_D_MOVE,
+};
 MODULE_ALIAS_FS("lustre");
 
+int lustre_register_fs(void)
+{
+	return register_filesystem(&lustre_fs_type);
+}
+
+int lustre_unregister_fs(void)
+{
+	return unregister_filesystem(&lustre_fs_type);
+}
 static int __init lustre_init(void)
 {
 	int rc;
@@ -145,7 +253,10 @@ static int __init lustre_init(void)
 	if (rc != 0)
 		goto out_inode_fini_env;
 
-	lustre_register_super_ops(THIS_MODULE, ll_fill_super, ll_kill_super);
+	rc = lustre_register_fs();
+	if (rc)
+		goto out_inode_fini_env;
+
 	lustre_register_client_process_config(ll_process_config);
 
 	return 0;
@@ -166,7 +277,8 @@ static int __init lustre_init(void)
 
 static void __exit lustre_exit(void)
 {
-	lustre_register_super_ops(NULL, NULL, NULL);
+	lustre_unregister_fs();
+
 	lustre_register_client_process_config(NULL);
 
 	debugfs_remove(llite_root);
diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
index 81a4c666bb69..9d389288bea4 100644
--- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
+++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
@@ -513,15 +513,11 @@ static int __init obdclass_init(void)
 	if (err)
 		return err;
 
-	err = lustre_register_fs();
-
 	return err;
 }
 
 static void obdclass_exit(void)
 {
-	lustre_unregister_fs();
-
 	misc_deregister(&obd_psdev);
 	llog_info_fini();
 	cl_global_fini();
diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
index 39911dcb0fd9..f0f00b779cf1 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
@@ -49,11 +49,6 @@
 #include <lustre_disk.h>
 #include <uapi/linux/lustre/lustre_param.h>
 
-static DEFINE_SPINLOCK(client_lock);
-static struct module *client_mod;
-static int (*client_fill_super)(struct super_block *sb);
-static void (*kill_super_cb)(struct super_block *sb);
-
 /**************** config llog ********************/
 
 /** Get a config log from the MGS and process it.
@@ -430,6 +425,7 @@ int lustre_start_mgc(struct super_block *sb)
 	kfree(niduuid);
 	return rc;
 }
+EXPORT_SYMBOL(lustre_start_mgc);
 
 static int lustre_stop_mgc(struct super_block *sb)
 {
@@ -507,7 +503,7 @@ static int lustre_stop_mgc(struct super_block *sb)
 
 /***************** lustre superblock **************/
 
-static struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
+struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
 {
 	struct lustre_sb_info *lsi;
 
@@ -532,6 +528,7 @@ static struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
 
 	return lsi;
 }
+EXPORT_SYMBOL(lustre_init_lsi);
 
 static int lustre_free_lsi(struct super_block *sb)
 {
@@ -567,7 +564,7 @@ static int lustre_free_lsi(struct super_block *sb)
 /* The lsi has one reference for every server that is using the disk -
  * e.g. MDT, MGS, and potentially MGC
  */
-static int lustre_put_lsi(struct super_block *sb)
+int lustre_put_lsi(struct super_block *sb)
 {
 	struct lustre_sb_info *lsi = s2lsi(sb);
 
@@ -578,6 +575,7 @@ static int lustre_put_lsi(struct super_block *sb)
 	}
 	return 0;
 }
+EXPORT_SYMBOL(lustre_put_lsi);
 
 /*** SERVER NAME ***
  * <FSNAME><SEPARATOR><TYPE><INDEX>
@@ -991,7 +989,7 @@ static int lmd_parse_nidlist(char *buf, char **endh)
  * e.g. mount -v -t lustre -o abort_recov uml1:uml2:/lustre-client /mnt/lustre
  * dev is passed as device=uml1:/lustre by mount.lustre
  */
-static int lmd_parse(char *options, struct lustre_mount_data *lmd)
+int lmd_parse(char *options, struct lustre_mount_data *lmd)
 {
 	char *s1, *s2, *devname = NULL;
 	struct lustre_mount_data *raw = (struct lustre_mount_data *)options;
@@ -1215,141 +1213,5 @@ static int lmd_parse(char *options, struct lustre_mount_data *lmd)
 	CERROR("Bad mount options %s\n", options);
 	return -EINVAL;
 }
+EXPORT_SYMBOL(lmd_parse);
 
-/** This is the entry point for the mount call into Lustre.
- * This is called when a server or client is mounted,
- * and this is where we start setting things up.
- * @param data Mount options (e.g. -o flock,abort_recov)
- */
-static int lustre_fill_super(struct super_block *sb, void *lmd2_data, int silent)
-{
-	struct lustre_mount_data *lmd;
-	struct lustre_sb_info *lsi;
-	int rc;
-
-	CDEBUG(D_MOUNT | D_VFSTRACE, "VFS Op: sb %p\n", sb);
-
-	lsi = lustre_init_lsi(sb);
-	if (!lsi)
-		return -ENOMEM;
-	lmd = lsi->lsi_lmd;
-
-	/*
-	 * Disable lockdep during mount, because mount locking patterns are
-	 * `special'.
-	 */
-	lockdep_off();
-
-	/*
-	 * LU-639: the obd cleanup of last mount may not finish yet, wait here.
-	 */
-	obd_zombie_barrier();
-
-	/* Figure out the lmd from the mount options */
-	if (lmd_parse(lmd2_data, lmd)) {
-		lustre_put_lsi(sb);
-		rc = -EINVAL;
-		goto out;
-	}
-
-	if (lmd_is_client(lmd)) {
-		bool have_client = false;
-		CDEBUG(D_MOUNT, "Mounting client %s\n", lmd->lmd_profile);
-		if (!client_fill_super)
-			request_module("lustre");
-		spin_lock(&client_lock);
-		if (client_fill_super && try_module_get(client_mod))
-			have_client = true;
-		spin_unlock(&client_lock);
-		if (!have_client) {
-			LCONSOLE_ERROR_MSG(0x165, "Nothing registered for client mount! Is the 'lustre' module loaded?\n");
-			lustre_put_lsi(sb);
-			rc = -ENODEV;
-		} else {
-			rc = lustre_start_mgc(sb);
-			if (rc) {
-				lustre_common_put_super(sb);
-				goto out;
-			}
-			/* Connect and start */
-			/* (should always be ll_fill_super) */
-			rc = (*client_fill_super)(sb);
-			/*
-			 * c_f_s will call lustre_common_put_super on failure, otherwise
-			 * c_f_s will have taken another reference to the module
-			 */
-			module_put(client_mod);
-		}
-	} else {
-		CERROR("This is client-side-only module, cannot handle server mount.\n");
-		rc = -EINVAL;
-	}
-
-	/* If error happens in fill_super() call, @lsi will be killed there.
-	 * This is why we do not put it here.
-	 */
-	goto out;
-out:
-	if (rc) {
-		CERROR("Unable to mount %s (%d)\n",
-		       s2lsi(sb) ? lmd->lmd_dev : "", rc);
-	} else {
-		CDEBUG(D_SUPER, "Mount %s complete\n",
-		       lmd->lmd_dev);
-	}
-	lockdep_on();
-	return rc;
-}
-
-/* We can't call ll_fill_super by name because it lives in a module that
- * must be loaded after this one.
- */
-void lustre_register_super_ops(struct module *mod,
-			       int (*cfs)(struct super_block *sb),
-			       void (*ksc)(struct super_block *sb))
-{
-	spin_lock(&client_lock);
-	client_mod = mod;
-	client_fill_super = cfs;
-	kill_super_cb = ksc;
-	spin_unlock(&client_lock);
-}
-EXPORT_SYMBOL(lustre_register_super_ops);
-
-/***************** FS registration ******************/
-static struct dentry *lustre_mount(struct file_system_type *fs_type, int flags,
-				   const char *devname, void *data)
-{
-	return mount_nodev(fs_type, flags, data, lustre_fill_super);
-}
-
-static void lustre_kill_super(struct super_block *sb)
-{
-	struct lustre_sb_info *lsi = s2lsi(sb);
-
-	if (kill_super_cb && lsi)
-		(*kill_super_cb)(sb);
-
-	kill_anon_super(sb);
-}
-
-/** Register the "lustre" fs type
- */
-static struct file_system_type lustre_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "lustre",
-	.mount		= lustre_mount,
-	.kill_sb	= lustre_kill_super,
-	.fs_flags	= FS_RENAME_DOES_D_MOVE,
-};
-MODULE_ALIAS_FS("lustre");
-
-int lustre_register_fs(void)
-{
-	return register_filesystem(&lustre_fs_type);
-}
-
-int lustre_unregister_fs(void)
-{
-	return unregister_filesystem(&lustre_fs_type);
-}
-- 
2.14.0.rc0.dirty

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180801/37c46d70/attachment-0001.sig>

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

end of thread, other threads:[~2018-08-01  3:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30 23:16 [lustre-devel] [PATCH 0/3] lustre: llite: rework client mount code James Simmons
2018-07-30 23:16 ` [lustre-devel] [PATCH 1/3] lustre: obd: migrate lustre_fill_super() to llite James Simmons
2018-07-31  3:13   ` NeilBrown
2018-08-01  3:17     ` NeilBrown
2018-07-30 23:16 ` [lustre-devel] [PATCH 2/3] lustre: llite: handle registering file system James Simmons
2018-08-01  0:13   ` Andreas Dilger
2018-08-01  1:14     ` NeilBrown
2018-07-30 23:16 ` [lustre-devel] [PATCH 3/3] lustre: llite: simplify lustre_fill_super() James Simmons

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