[lustre-devel] [PATCH 00/28] More lustre patches...

[lustre-devel] [PATCH 00/28] More lustre patches...

[NeilBrown]

Some of these I posted before but didn't get a formal Reviewed-by.
Some needed some revision.
Some did get a reviewed-by, but depend on patches in the above
categories.

Others are brand new.

BTW I've updated my lustre branch to 5.0-rc8 with all the patches that
have been around for a while and received review.  All these are in
lustre-testing.

Thanks,
NeilBrown

---

NeilBrown (28):
      lustre: embed typ_kobj in obd_type
      lustre: collect all resource releasing for obj_type.
      lustre: obd_type: use typ_kobj.name as typ_name
      lustre: obd_type: discard obd_types linked list.
      lustre: obd_type: discard obd_type_lock
      lustre: obdclass: don't copy ops structures in to new type.
      lustre: obdclass: fix module load locking.
      lustre: convert rsi_sem to a spinlock.
      lustre: ldlm: discard varname in ldlm_pool.
      lustre: lprocfs: use log2.h macros instead of shift loop.
      lustre: handles: discard h_owner in favour of h_ops
      lustre: handle: move refcount into the lustre_handle.
      lustre: discard OBD_FREE_RCU
      lustre: portals_handle: rename ops to owner
      lustre: portals_handle: remove locking from class_handle2object()
      lustre: portals_handle: use hlist for hash lists.
      lustre: portals_handle: discard h_lock.
      lustre: remove unused fields from struct obd_device
      lustre: obd_sysfs: error-check value stored in jobid_var
      lustre: lov: use GFP_NOFS to allocate lo_entries.
      lustre: vvp_dev; increment *pos in .next
      lustre: obdclass: discard process_quota_config
      lustre: obdclass: remove unnecessary code from lustre_init_lsi()
      lustre: ldlm: discard l_lock from struct ldlm_lock.
      lustre: ldlm: don't access l_resource when not locked.
      lustre: ldlm: drop SLAB_TYPESAFE_BY_RCU from ldlm_lock slab.
      lustre: ldlm: simplify lock_mode_to_index()
      lustre: use list_move where appropriate.


 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |   10 -
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |    6 -
 .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    3 
 .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |    3 
 .../lustre/lnet/klnds/socklnd/socklnd_proto.c      |    3 
 drivers/staging/lustre/lnet/lnet/config.c          |    3 
 drivers/staging/lustre/lnet/lnet/lib-move.c        |   16 +
 drivers/staging/lustre/lnet/selftest/console.c     |    7 -
 drivers/staging/lustre/lustre/include/lustre_dlm.h |   11 -
 .../staging/lustre/lustre/include/lustre_export.h  |    1 
 .../staging/lustre/lustre/include/lustre_handles.h |   22 --
 .../staging/lustre/lustre/include/lustre_import.h  |    2 
 drivers/staging/lustre/lustre/include/lustre_net.h |    4 
 drivers/staging/lustre/lustre/include/obd.h        |   45 +---
 drivers/staging/lustre/lustre/include/obd_class.h  |    8 -
 .../staging/lustre/lustre/include/obd_support.h    |   10 -
 drivers/staging/lustre/lustre/ldlm/l_lock.c        |   21 +-
 drivers/staging/lustre/lustre/ldlm/ldlm_extent.c   |    3 
 drivers/staging/lustre/lustre/ldlm/ldlm_internal.h |    2 
 drivers/staging/lustre/lustre/ldlm/ldlm_lib.c      |    7 -
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |  101 ++++----
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c    |   23 ++
 drivers/staging/lustre/lustre/ldlm/ldlm_pool.c     |   18 --
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |    3 
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |    9 -
 drivers/staging/lustre/lustre/llite/llite_lib.c    |    6 -
 drivers/staging/lustre/lustre/llite/lproc_llite.c  |    4 
 drivers/staging/lustre/lustre/llite/vvp_dev.c      |    2 
 drivers/staging/lustre/lustre/lmv/lmv_obd.c        |    4 
 drivers/staging/lustre/lustre/lov/lov_obd.c        |    2 
 drivers/staging/lustre/lustre/lov/lov_object.c     |    2 
 drivers/staging/lustre/lustre/mdc/mdc_request.c    |    6 -
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |    8 -
 drivers/staging/lustre/lustre/obdclass/class_obd.c |    1 
 drivers/staging/lustre/lustre/obdclass/genops.c    |  238 ++++++++++----------
 .../lustre/lustre/obdclass/lprocfs_status.c        |   20 +-
 drivers/staging/lustre/lustre/obdclass/lu_object.c |    2 
 .../lustre/lustre/obdclass/lustre_handles.c        |   62 ++---
 .../staging/lustre/lustre/obdclass/obd_config.c    |   11 -
 drivers/staging/lustre/lustre/obdclass/obd_mount.c |    6 -
 drivers/staging/lustre/lustre/obdclass/obd_sysfs.c |   21 +-
 .../staging/lustre/lustre/obdecho/echo_client.c    |    4 
 drivers/staging/lustre/lustre/osc/osc_request.c    |    2 
 drivers/staging/lustre/lustre/ptlrpc/client.c      |   12 -
 drivers/staging/lustre/lustre/ptlrpc/import.c      |    3 
 drivers/staging/lustre/lustre/ptlrpc/service.c     |   17 +
 46 files changed, 330 insertions(+), 444 deletions(-)

--
Signature

[lustre-devel] [PATCH 01/28] lustre: embed typ_kobj in obd_type

[NeilBrown]

As there is a 1-1 mapping between obd_types and their ->typ_kobj, it
is simple and more normal to embed the kobj in the obd_type, rather
than allocate it separately.

This requires calling "kobject_init()" earlier, so we open-code
relevant part of class_setup_tunables() in class_register_type().

A new simple_class_ktype is needed for the remaining user of
class_setup_tunables().

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/obd.h        |    2 +
 drivers/staging/lustre/lustre/obdclass/genops.c    |   38 +++++++++++++-------
 .../lustre/lustre/obdclass/lprocfs_status.c        |    2 +
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index f62669573717..f20c2468712a 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -107,7 +107,7 @@ struct obd_type {
 	int			 typ_refcnt;
 	struct lu_device_type	*typ_lu;
 	spinlock_t		 obd_type_lock;
-	struct kobject		*typ_kobj;
+	struct kobject		 typ_kobj;
 };
 
 struct brw_page {
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index bc1f9792cf0b..7b3432f43a40 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -134,11 +134,23 @@ void class_put_type(struct obd_type *type)
 	spin_unlock(&type->obd_type_lock);
 }
 
-static void class_sysfs_release(struct kobject *kobj)
+static void simple_class_release(struct kobject *kobj)
 {
 	kfree(kobj);
 }
 
+static struct kobj_type simple_class_ktype = {
+	.sysfs_ops	= &lustre_sysfs_ops,
+	.release	= simple_class_release,
+};
+
+static void class_sysfs_release(struct kobject *kobj)
+{
+	struct obd_type *type = container_of(kobj, struct obd_type, typ_kobj);
+
+	kfree(type);
+}
+
 static struct kobj_type class_ktype = {
 	.sysfs_ops	= &lustre_sysfs_ops,
 	.release	= class_sysfs_release,
@@ -154,7 +166,7 @@ struct kobject *class_setup_tunables(const char *name)
 		return ERR_PTR(-ENOMEM);
 
 	kobj->kset = lustre_kset;
-	kobject_init(kobj, &class_ktype);
+	kobject_init(kobj, &simple_class_ktype);
 	rc = kobject_add(kobj, &lustre_kset->kobj, "%s", name);
 	if (rc) {
 		kobject_put(kobj);
@@ -186,6 +198,9 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 	if (!type)
 		return rc;
 
+	type->typ_kobj.kset = lustre_kset;
+	kobject_init(&type->typ_kobj, &class_ktype);
+
 	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
 	type->typ_md_ops = kzalloc(sizeof(*type->typ_md_ops), GFP_NOFS);
 	type->typ_name = kzalloc(strlen(name) + 1, GFP_NOFS);
@@ -205,19 +220,16 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 	type->typ_debugfs_entry = debugfs_create_dir(type->typ_name,
 						     debugfs_lustre_root);
 
-	type->typ_kobj = class_setup_tunables(type->typ_name);
-	if (IS_ERR(type->typ_kobj)) {
-		rc = PTR_ERR(type->typ_kobj);
+	rc = kobject_add(&type->typ_kobj, &lustre_kset->kobj, "%s", name);
+
+	if (rc)
 		goto failed;
-	}
 
 	if (ldt) {
 		type->typ_lu = ldt;
 		rc = lu_device_type_init(ldt);
-		if (rc != 0) {
-			kobject_put(type->typ_kobj);
+		if (rc != 0)
 			goto failed;
-		}
 	}
 
 	spin_lock(&obd_types_lock);
@@ -230,7 +242,8 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 	kfree(type->typ_name);
 	kfree(type->typ_md_ops);
 	kfree(type->typ_dt_ops);
-	kfree(type);
+	kobject_put(&type->typ_kobj);
+
 	return rc;
 }
 EXPORT_SYMBOL(class_register_type);
@@ -253,8 +266,6 @@ int class_unregister_type(const char *name)
 		return -EBUSY;
 	}
 
-	kobject_put(type->typ_kobj);
-
 	debugfs_remove_recursive(type->typ_debugfs_entry);
 
 	if (type->typ_lu)
@@ -266,7 +277,8 @@ int class_unregister_type(const char *name)
 	kfree(type->typ_name);
 	kfree(type->typ_dt_ops);
 	kfree(type->typ_md_ops);
-	kfree(type);
+	kobject_put(&type->typ_kobj);
+
 	return 0;
 } /* class_unregister_type */
 EXPORT_SYMBOL(class_unregister_type);
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 11fddc88d532..71bf409240f5 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -1036,7 +1036,7 @@ int lprocfs_obd_setup(struct obd_device *obd, bool uuid_only)
 	obd->obd_ktype.sysfs_ops = &lustre_sysfs_ops;
 	obd->obd_ktype.release = obd_sysfs_release;
 
-	obd->obd_kset.kobj.parent = obd->obd_type->typ_kobj;
+	obd->obd_kset.kobj.parent = &obd->obd_type->typ_kobj;
 	obd->obd_kset.kobj.ktype = &obd->obd_ktype;
 	init_completion(&obd->obd_kobj_unregister);
 	rc = kset_register(&obd->obd_kset);

[lustre-devel] [PATCH 02/28] lustre: collect all resource releasing for obj_type.

[NeilBrown]

Now that obj_type is managed as a kobject, move all
the freeing and deregistering into class_sysfs_release().

Reviewed-by: James Simmons <jsimmons@infradead.org>
Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/obdclass/genops.c |   27 +++++++++++------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index 7b3432f43a40..8dd217c888a5 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -148,6 +148,18 @@ static void class_sysfs_release(struct kobject *kobj)
 {
 	struct obd_type *type = container_of(kobj, struct obd_type, typ_kobj);
 
+	debugfs_remove_recursive(type->typ_debugfs_entry);
+
+	if (type->typ_lu)
+		lu_device_type_fini(type->typ_lu);
+
+	spin_lock(&obd_types_lock);
+	list_del(&type->typ_chain);
+	spin_unlock(&obd_types_lock);
+
+	kfree(type->typ_name);
+	kfree(type->typ_md_ops);
+	kfree(type->typ_dt_ops);
 	kfree(type);
 }
 
@@ -200,6 +212,7 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 
 	type->typ_kobj.kset = lustre_kset;
 	kobject_init(&type->typ_kobj, &class_ktype);
+	INIT_LIST_HEAD(&type->typ_chain);
 
 	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
 	type->typ_md_ops = kzalloc(sizeof(*type->typ_md_ops), GFP_NOFS);
@@ -239,9 +252,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 	return 0;
 
 failed:
-	kfree(type->typ_name);
-	kfree(type->typ_md_ops);
-	kfree(type->typ_dt_ops);
 	kobject_put(&type->typ_kobj);
 
 	return rc;
@@ -266,17 +276,6 @@ int class_unregister_type(const char *name)
 		return -EBUSY;
 	}
 
-	debugfs_remove_recursive(type->typ_debugfs_entry);
-
-	if (type->typ_lu)
-		lu_device_type_fini(type->typ_lu);
-
-	spin_lock(&obd_types_lock);
-	list_del(&type->typ_chain);
-	spin_unlock(&obd_types_lock);
-	kfree(type->typ_name);
-	kfree(type->typ_dt_ops);
-	kfree(type->typ_md_ops);
 	kobject_put(&type->typ_kobj);
 
 	return 0;

[lustre-devel] [PATCH 03/28] lustre: obd_type: use typ_kobj.name as typ_name

[NeilBrown]

As the kobject has a name (after kobject_add has been called),
we don't need to also store it in typ_name.
So use typ_kobj.name instead of typ_name.

This requires changing some "char *" to "const char *" as
typ.kobj.name is const.

Reviewed-by: James Simmons <jsimmons@infradead.org>
Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/lustre_net.h |    4 ++-
 drivers/staging/lustre/lustre/include/obd.h        |    2 +-
 drivers/staging/lustre/lustre/include/obd_class.h  |    2 +-
 drivers/staging/lustre/lustre/ldlm/ldlm_lib.c      |    2 +-
 drivers/staging/lustre/lustre/obdclass/genops.c    |   25 ++++++++------------
 drivers/staging/lustre/lustre/ptlrpc/client.c      |    2 +-
 6 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_net.h b/drivers/staging/lustre/lustre/include/lustre_net.h
index f6d1be1a8350..1cf0f127203a 100644
--- a/drivers/staging/lustre/lustre/include/lustre_net.h
+++ b/drivers/staging/lustre/lustre/include/lustre_net.h
@@ -308,7 +308,7 @@ struct ptlrpc_client {
 	/** What portal do we expect replies on */
 	u32			cli_reply_portal;
 	/** Name of the client */
-	char			*cli_name;
+	const char		*cli_name;
 };
 
 /** state flags of requests */
@@ -1817,7 +1817,7 @@ void ptlrpc_request_committed(struct ptlrpc_request *req, int force);
 int ptlrpc_inc_ref(void);
 void ptlrpc_dec_ref(void);
 
-void ptlrpc_init_client(int req_portal, int rep_portal, char *name,
+void ptlrpc_init_client(int req_portal, int rep_portal, const char *name,
 			struct ptlrpc_client *);
 struct ptlrpc_connection *ptlrpc_uuid_to_connection(struct obd_uuid *uuid,
 						    lnet_nid_t nid4refnet);
diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index f20c2468712a..6bf052a5ac5a 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -103,12 +103,12 @@ struct obd_type {
 	struct obd_ops		*typ_dt_ops;
 	struct md_ops		*typ_md_ops;
 	struct dentry		*typ_debugfs_entry;
-	char			*typ_name;
 	int			 typ_refcnt;
 	struct lu_device_type	*typ_lu;
 	spinlock_t		 obd_type_lock;
 	struct kobject		 typ_kobj;
 };
+#define typ_name typ_kobj.name
 
 struct brw_page {
 	u64			off;
diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
index e4cde1971e33..bf6ce3284bef 100644
--- a/drivers/staging/lustre/lustre/include/obd_class.h
+++ b/drivers/staging/lustre/lustre/include/obd_class.h
@@ -75,7 +75,7 @@ int class_name2dev(const char *name);
 struct obd_device *class_name2obd(const char *name);
 int class_uuid2dev(struct obd_uuid *uuid);
 struct obd_device *class_find_client_obd(struct obd_uuid *tgt_uuid,
-					 const char *typ_name,
+					 const char *type_name,
 					 struct obd_uuid *grp_uuid);
 struct obd_device *class_devices_in_group(struct obd_uuid *grp_uuid,
 					  int *next);
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c
index e0d2851accef..9c61b332a51c 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c
@@ -247,7 +247,7 @@ int client_obd_setup(struct obd_device *obddev, struct lustre_cfg *lcfg)
 	struct obd_import *imp;
 	struct obd_uuid server_uuid;
 	int rq_portal, rp_portal, connect_op;
-	char *name = obddev->obd_type->typ_name;
+	const char *name = obddev->obd_type->typ_name;
 	enum ldlm_ns_type ns_type = LDLM_NS_TYPE_UNKNOWN;
 	struct ptlrpc_connection fake_conn = {
 		.c_self = 0,
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index 8dd217c888a5..8f683313acf2 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -157,7 +157,6 @@ static void class_sysfs_release(struct kobject *kobj)
 	list_del(&type->typ_chain);
 	spin_unlock(&obd_types_lock);
 
-	kfree(type->typ_name);
 	kfree(type->typ_md_ops);
 	kfree(type->typ_dt_ops);
 	kfree(type);
@@ -216,28 +215,24 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 
 	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
 	type->typ_md_ops = kzalloc(sizeof(*type->typ_md_ops), GFP_NOFS);
-	type->typ_name = kzalloc(strlen(name) + 1, GFP_NOFS);
 
 	if (!type->typ_dt_ops ||
-	    !type->typ_md_ops ||
-	    !type->typ_name)
+	    !type->typ_md_ops)
 		goto failed;
 
 	*type->typ_dt_ops = *dt_ops;
 	/* md_ops is optional */
 	if (md_ops)
 		*type->typ_md_ops = *md_ops;
-	strcpy(type->typ_name, name);
 	spin_lock_init(&type->obd_type_lock);
 
-	type->typ_debugfs_entry = debugfs_create_dir(type->typ_name,
-						     debugfs_lustre_root);
-
 	rc = kobject_add(&type->typ_kobj, &lustre_kset->kobj, "%s", name);
-
 	if (rc)
 		goto failed;
 
+	type->typ_debugfs_entry = debugfs_create_dir(type->typ_name,
+						     debugfs_lustre_root);
+
 	if (ldt) {
 		type->typ_lu = ldt;
 		rc = lu_device_type_init(ldt);
@@ -564,7 +559,7 @@ struct obd_device *class_num2obd(int num)
  * otherwise any client connected to the tgt is returned.
  */
 struct obd_device *class_find_client_obd(struct obd_uuid *tgt_uuid,
-					 const char *typ_name,
+					 const char *type_name,
 					 struct obd_uuid *grp_uuid)
 {
 	int i;
@@ -575,8 +570,8 @@ struct obd_device *class_find_client_obd(struct obd_uuid *tgt_uuid,
 
 		if (!obd)
 			continue;
-		if ((strncmp(obd->obd_type->typ_name, typ_name,
-			     strlen(typ_name)) == 0)) {
+		if ((strncmp(obd->obd_type->typ_name, type_name,
+			     strlen(type_name)) == 0)) {
 			if (obd_uuid_equals(tgt_uuid,
 					    &obd->u.cli.cl_target_uuid) &&
 			    ((grp_uuid) ? obd_uuid_equals(grp_uuid,
@@ -1388,7 +1383,7 @@ EXPORT_SYMBOL(obd_get_max_rpcs_in_flight);
 int obd_set_max_rpcs_in_flight(struct client_obd *cli, u32 max)
 {
 	struct obd_request_slot_waiter *orsw;
-	const char *typ_name;
+	const char *type_name;
 	u32 old;
 	int diff;
 	int rc;
@@ -1397,8 +1392,8 @@ int obd_set_max_rpcs_in_flight(struct client_obd *cli, u32 max)
 	if (max > OBD_MAX_RIF_MAX || max < 1)
 		return -ERANGE;
 
-	typ_name = cli->cl_import->imp_obd->obd_type->typ_name;
-	if (!strcmp(typ_name, LUSTRE_MDC_NAME)) {
+	type_name = cli->cl_import->imp_obd->obd_type->typ_name;
+	if (!strcmp(type_name, LUSTRE_MDC_NAME)) {
 		/*
 		 * adjust max_mod_rpcs_in_flight to ensure it is always
 		 * strictly lower that max_rpcs_in_flight
diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
index eb5d22a538a0..da1ccd8f9dcc 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -67,7 +67,7 @@ static int ptlrpc_unregister_reply(struct ptlrpc_request *request, int async);
 /**
  * Initialize passed in client structure @cl.
  */
-void ptlrpc_init_client(int req_portal, int rep_portal, char *name,
+void ptlrpc_init_client(int req_portal, int rep_portal, const char *name,
 			struct ptlrpc_client *cl)
 {
 	cl->cli_request_portal = req_portal;

[lustre-devel] [PATCH 04/28] lustre: obd_type: discard obd_types linked list.

[NeilBrown]

As all obd_types are kobjects in the lustre_kset kset,
they are linked together in that kset and don't
need any extra linkage.

There are non-obd_type objects in lustre_kset, added by
class_setup_tunables().  These have a different ->ktype, so we are
careful to only return objects with the correct ->ktype.

As kset_find_obj() returns a counted reference, we need
to put that reference when done.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/obd.h     |    1 -
 drivers/staging/lustre/lustre/obdclass/genops.c |   42 +++++++++++------------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index 6bf052a5ac5a..4c58b916e0a3 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -99,7 +99,6 @@ struct obd_info {
 };
 
 struct obd_type {
-	struct list_head	 typ_chain;
 	struct obd_ops		*typ_dt_ops;
 	struct md_ops		*typ_md_ops;
 	struct dentry		*typ_debugfs_entry;
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index 8f683313acf2..74195de639e4 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -42,8 +42,6 @@
 #include <lprocfs_status.h>
 #include <lustre_kernelcomm.h>
 
-static DEFINE_SPINLOCK(obd_types_lock);
-static LIST_HEAD(obd_types);
 DEFINE_RWLOCK(obd_dev_lock);
 static struct obd_device *obd_devs[MAX_OBD_DEVICES];
 
@@ -52,6 +50,7 @@ struct kmem_cache *obdo_cachep;
 EXPORT_SYMBOL(obdo_cachep);
 static struct kmem_cache *import_cachep;
 
+static struct kobj_type class_ktype;
 static struct workqueue_struct *zombie_wq;
 static void obd_zombie_export_add(struct obd_export *exp);
 static void obd_zombie_import_add(struct obd_import *imp);
@@ -88,22 +87,19 @@ static void obd_device_free(struct obd_device *obd)
 
 static struct obd_type *class_search_type(const char *name)
 {
-	struct obd_type *type;
+	struct kobject *kobj = kset_find_obj(lustre_kset, name);
+
+	if (kobj && kobj->ktype == &class_ktype)
+		return container_of(kobj, struct obd_type, typ_kobj);
 
-	spin_lock(&obd_types_lock);
-	list_for_each_entry(type, &obd_types, typ_chain) {
-		if (strcmp(type->typ_name, name) == 0) {
-			spin_unlock(&obd_types_lock);
-			return type;
-		}
-	}
-	spin_unlock(&obd_types_lock);
 	return NULL;
 }
 
 static struct obd_type *class_get_type(const char *name)
 {
-	struct obd_type *type = class_search_type(name);
+	struct obd_type *type;
+
+	type = class_search_type(name);
 
 	if (!type) {
 		const char *modname = name;
@@ -121,6 +117,11 @@ static struct obd_type *class_get_type(const char *name)
 		type->typ_refcnt++;
 		try_module_get(type->typ_dt_ops->owner);
 		spin_unlock(&type->obd_type_lock);
+		/* class_search_type() returned a counted reference,
+		 * but we don't need that count any more as
+		 * we have one through typ_refcnt.
+		 */
+		kobject_put(&type->typ_kobj);
 	}
 	return type;
 }
@@ -153,10 +154,6 @@ static void class_sysfs_release(struct kobject *kobj)
 	if (type->typ_lu)
 		lu_device_type_fini(type->typ_lu);
 
-	spin_lock(&obd_types_lock);
-	list_del(&type->typ_chain);
-	spin_unlock(&obd_types_lock);
-
 	kfree(type->typ_md_ops);
 	kfree(type->typ_dt_ops);
 	kfree(type);
@@ -199,8 +196,10 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 	/* sanity check */
 	LASSERT(strnlen(name, CLASS_MAX_NAME) < CLASS_MAX_NAME);
 
-	if (class_search_type(name)) {
+	type = class_search_type(name);
+	if (type) {
 		CDEBUG(D_IOCTL, "Type %s already registered\n", name);
+		kobject_put(&type->typ_kobj);
 		return -EEXIST;
 	}
 
@@ -211,7 +210,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 
 	type->typ_kobj.kset = lustre_kset;
 	kobject_init(&type->typ_kobj, &class_ktype);
-	INIT_LIST_HEAD(&type->typ_chain);
 
 	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
 	type->typ_md_ops = kzalloc(sizeof(*type->typ_md_ops), GFP_NOFS);
@@ -240,10 +238,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 			goto failed;
 	}
 
-	spin_lock(&obd_types_lock);
-	list_add(&type->typ_chain, &obd_types);
-	spin_unlock(&obd_types_lock);
-
 	return 0;
 
 failed:
@@ -268,9 +262,13 @@ int class_unregister_type(const char *name)
 		/* Remove ops, but leave the name for debugging */
 		kfree(type->typ_dt_ops);
 		kfree(type->typ_md_ops);
+		kobject_put(&type->typ_kobj);
 		return -EBUSY;
 	}
 
+	/* Put the ref returned by class_search_type() */
+	kobject_put(&type->typ_kobj);
+	/* Put the final ref */
 	kobject_put(&type->typ_kobj);
 
 	return 0;

[lustre-devel] [PATCH 05/28] lustre: obd_type: discard obd_type_lock

[NeilBrown]

This lock is only used to protect typ_refcnt, so change
that to an atomic_t and discard the lock.

The lock also covers calls to try_module_get and module_put,
but this serves no purpose as it does not prevent the module
from being unloaded.

Finally, the return value for the call to try_module_get is
ignored, which is not safe.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/obd.h        |    3 +-
 drivers/staging/lustre/lustre/mdc/mdc_request.c    |    2 +
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |    2 +
 drivers/staging/lustre/lustre/obdclass/genops.c    |   30 +++++++++-----------
 drivers/staging/lustre/lustre/obdclass/lu_object.c |    2 +
 5 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index 4c58b916e0a3..61fb8159af20 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -102,9 +102,8 @@ struct obd_type {
 	struct obd_ops		*typ_dt_ops;
 	struct md_ops		*typ_md_ops;
 	struct dentry		*typ_debugfs_entry;
-	int			 typ_refcnt;
+	atomic_t		 typ_refcnt;
 	struct lu_device_type	*typ_lu;
-	spinlock_t		 obd_type_lock;
 	struct kobject		 typ_kobj;
 };
 #define typ_name typ_kobj.name
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index bc764f9dd102..705a4e3b518a 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -2542,7 +2542,7 @@ static int mdc_init_ea_size(struct obd_export *exp, u32 easize, u32 def_easize)
 static int mdc_precleanup(struct obd_device *obd)
 {
 	/* Failsafe, ok if racy */
-	if (obd->obd_type->typ_refcnt <= 1)
+	if (atomic_read(&obd->obd_type->typ_refcnt) <= 1)
 		libcfs_kkuc_group_rem(0, KUC_GRP_HSM);
 
 	mdc_changelog_cdev_finish(obd);
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 84ba6d0e3493..0580afa2755d 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -715,7 +715,7 @@ static int mgc_cleanup(struct obd_device *obd)
 	/* COMPAT_146 - old config logs may have added profiles we don't
 	 * know about
 	 */
-	if (obd->obd_type->typ_refcnt <= 1)
+	if (atomic_read(&obd->obd_type->typ_refcnt) <= 1)
 		/* Only for the last mgc */
 		class_del_profiles();
 
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index 74195de639e4..02d829617519 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -113,15 +113,17 @@ static struct obd_type *class_get_type(const char *name)
 		}
 	}
 	if (type) {
-		spin_lock(&type->obd_type_lock);
-		type->typ_refcnt++;
-		try_module_get(type->typ_dt_ops->owner);
-		spin_unlock(&type->obd_type_lock);
-		/* class_search_type() returned a counted reference,
-		 * but we don't need that count any more as
-		 * we have one through typ_refcnt.
-		 */
-		kobject_put(&type->typ_kobj);
+		if (try_module_get(type->typ_dt_ops->owner)) {
+			atomic_inc(&type->typ_refcnt);
+			/* class_search_type() returned a counted reference,
+			 * but we don't need that count any more as
+			 * we have one through typ_refcnt.
+			 */
+			kobject_put(&type->typ_kobj);
+		} else {
+			kobject_put(&type->typ_kobj);
+			type = NULL;
+		}
 	}
 	return type;
 }
@@ -129,10 +131,7 @@ static struct obd_type *class_get_type(const char *name)
 void class_put_type(struct obd_type *type)
 {
 	LASSERT(type);
-	spin_lock(&type->obd_type_lock);
-	type->typ_refcnt--;
-	module_put(type->typ_dt_ops->owner);
-	spin_unlock(&type->obd_type_lock);
+	atomic_dec(&type->typ_refcnt);
 }
 
 static void simple_class_release(struct kobject *kobj)
@@ -222,7 +221,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 	/* md_ops is optional */
 	if (md_ops)
 		*type->typ_md_ops = *md_ops;
-	spin_lock_init(&type->obd_type_lock);
 
 	rc = kobject_add(&type->typ_kobj, &lustre_kset->kobj, "%s", name);
 	if (rc)
@@ -256,8 +254,8 @@ int class_unregister_type(const char *name)
 		return -EINVAL;
 	}
 
-	if (type->typ_refcnt) {
-		CERROR("type %s has refcount (%d)\n", name, type->typ_refcnt);
+	if (atomic_read(&type->typ_refcnt)) {
+		CERROR("type %s has refcount (%d)\n", name, atomic_read(&type->typ_refcnt));
 		/* This is a bad situation, let's make the best of it */
 		/* Remove ops, but leave the name for debugging */
 		kfree(type->typ_dt_ops);
diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
index 9c872db21040..770cc1b9e40b 100644
--- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
@@ -1267,7 +1267,7 @@ void lu_stack_fini(const struct lu_env *env, struct lu_device *top)
 		next = ldt->ldt_ops->ldto_device_free(env, scan);
 		type = ldt->ldt_obd_type;
 		if (type) {
-			type->typ_refcnt--;
+			atomic_dec(&type->typ_refcnt);
 			class_put_type(type);
 		}
 	}

[lustre-devel] [PATCH 06/28] lustre: obdclass: don't copy ops structures in to new type.

[NeilBrown]

The obd_ops and md_ops structures passed to class_register_type() are
read-only, and have a lifetime that is exceeds the lifetime of the
obd_type - they are static in a module which unregisters the type before
being unloaded.

So there is no need to copy the ops, just store a pointer.

Also mark all the structures as read-only to confirm they don't get
written.  This is best-practice for structures of function pointers.

Reviewed-by: James Simmons <jsimmons@infradead.org>
Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/obd.h        |    4 ++--
 drivers/staging/lustre/lustre/include/obd_class.h  |    3 ++-
 drivers/staging/lustre/lustre/lmv/lmv_obd.c        |    4 ++--
 drivers/staging/lustre/lustre/lov/lov_obd.c        |    2 +-
 drivers/staging/lustre/lustre/mdc/mdc_request.c    |    4 ++--
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |    2 +-
 drivers/staging/lustre/lustre/obdclass/genops.c    |   22 +++++---------------
 .../staging/lustre/lustre/obdecho/echo_client.c    |    2 +-
 drivers/staging/lustre/lustre/osc/osc_request.c    |    2 +-
 9 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index 61fb8159af20..3bdde312e4a6 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -99,8 +99,8 @@ struct obd_info {
 };
 
 struct obd_type {
-	struct obd_ops		*typ_dt_ops;
-	struct md_ops		*typ_md_ops;
+	const struct obd_ops	*typ_dt_ops;
+	const struct md_ops	*typ_md_ops;
 	struct dentry		*typ_debugfs_entry;
 	atomic_t		 typ_refcnt;
 	struct lu_device_type	*typ_lu;
diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
index bf6ce3284bef..4567ef3b21dc 100644
--- a/drivers/staging/lustre/lustre/include/obd_class.h
+++ b/drivers/staging/lustre/lustre/include/obd_class.h
@@ -61,7 +61,8 @@ struct lu_device_type;
 /* genops.c */
 struct obd_export *class_conn2export(struct lustre_handle *conn);
 struct kobject *class_setup_tunables(const char *name);
-int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
+int class_register_type(const struct obd_ops *dt_ops,
+			const struct md_ops *md_ops,
 			const char *name, struct lu_device_type *ldt);
 int class_unregister_type(const char *name);
 
diff --git a/drivers/staging/lustre/lustre/lmv/lmv_obd.c b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
index aae80c7f725b..64ebaf399530 100644
--- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
@@ -3043,7 +3043,7 @@ static int lmv_merge_attr(struct obd_export *exp,
 	return 0;
 }
 
-static struct obd_ops lmv_obd_ops = {
+static const struct obd_ops lmv_obd_ops = {
 	.owner			= THIS_MODULE,
 	.setup			= lmv_setup,
 	.cleanup		= lmv_cleanup,
@@ -3060,7 +3060,7 @@ static struct obd_ops lmv_obd_ops = {
 	.quotactl		= lmv_quotactl
 };
 
-static struct md_ops lmv_md_ops = {
+static const struct md_ops lmv_md_ops = {
 	.get_root		= lmv_get_root,
 	.null_inode		= lmv_null_inode,
 	.close			= lmv_close,
diff --git a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
index 203923f9bd96..652ed6a20af3 100644
--- a/drivers/staging/lustre/lustre/lov/lov_obd.c
+++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
@@ -1332,7 +1332,7 @@ static int lov_quotactl(struct obd_device *obd, struct obd_export *exp,
 	return rc;
 }
 
-static struct obd_ops lov_obd_ops = {
+static const struct obd_ops lov_obd_ops = {
 	.owner		= THIS_MODULE,
 	.setup		= lov_setup,
 	.cleanup	= lov_cleanup,
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 705a4e3b518a..6f933b511366 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -2571,7 +2571,7 @@ static int mdc_process_config(struct obd_device *obd, u32 len, void *buf)
 	return count > 0 ? 0 : count;
 }
 
-static struct obd_ops mdc_obd_ops = {
+static const struct obd_ops mdc_obd_ops = {
 	.owner			= THIS_MODULE,
 	.setup			= mdc_setup,
 	.precleanup		= mdc_precleanup,
@@ -2593,7 +2593,7 @@ static struct obd_ops mdc_obd_ops = {
 	.quotactl		= mdc_quotactl,
 };
 
-static struct md_ops mdc_md_ops = {
+static const struct md_ops mdc_md_ops = {
 	.get_root		= mdc_get_root,
 	.null_inode		= mdc_null_inode,
 	.close			= mdc_close,
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 0580afa2755d..5ce49e708287 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1817,7 +1817,7 @@ static int mgc_process_config(struct obd_device *obd, u32 len, void *buf)
 	return rc;
 }
 
-static struct obd_ops mgc_obd_ops = {
+static const struct obd_ops mgc_obd_ops = {
 	.owner		= THIS_MODULE,
 	.setup		= mgc_setup,
 	.precleanup	= mgc_precleanup,
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index 02d829617519..dc09c302a246 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -153,8 +153,6 @@ static void class_sysfs_release(struct kobject *kobj)
 	if (type->typ_lu)
 		lu_device_type_fini(type->typ_lu);
 
-	kfree(type->typ_md_ops);
-	kfree(type->typ_dt_ops);
 	kfree(type);
 }
 
@@ -185,7 +183,8 @@ EXPORT_SYMBOL(class_setup_tunables);
 
 #define CLASS_MAX_NAME 1024
 
-int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
+int class_register_type(const struct obd_ops *dt_ops,
+			const struct md_ops *md_ops,
 			const char *name,
 			struct lu_device_type *ldt)
 {
@@ -210,17 +209,8 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
 	type->typ_kobj.kset = lustre_kset;
 	kobject_init(&type->typ_kobj, &class_ktype);
 
-	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
-	type->typ_md_ops = kzalloc(sizeof(*type->typ_md_ops), GFP_NOFS);
-
-	if (!type->typ_dt_ops ||
-	    !type->typ_md_ops)
-		goto failed;
-
-	*type->typ_dt_ops = *dt_ops;
-	/* md_ops is optional */
-	if (md_ops)
-		*type->typ_md_ops = *md_ops;
+	type->typ_dt_ops = dt_ops;
+	type->typ_md_ops = md_ops;
 
 	rc = kobject_add(&type->typ_kobj, &lustre_kset->kobj, "%s", name);
 	if (rc)
@@ -258,8 +248,8 @@ int class_unregister_type(const char *name)
 		CERROR("type %s has refcount (%d)\n", name, atomic_read(&type->typ_refcnt));
 		/* This is a bad situation, let's make the best of it */
 		/* Remove ops, but leave the name for debugging */
-		kfree(type->typ_dt_ops);
-		kfree(type->typ_md_ops);
+		type->typ_dt_ops = NULL;
+		type->typ_md_ops = NULL;
 		kobject_put(&type->typ_kobj);
 		return -EBUSY;
 	}
diff --git a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
index 317123fd27cb..baf34c85c4b5 100644
--- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
+++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
@@ -1676,7 +1676,7 @@ static int echo_client_disconnect(struct obd_export *exp)
 	return rc;
 }
 
-static struct obd_ops echo_client_obd_ops = {
+static const struct obd_ops echo_client_obd_ops = {
 	.owner			= THIS_MODULE,
 	.iocontrol		= echo_client_iocontrol,
 	.connect		= echo_client_connect,
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index 104bf4391fef..9484b9a90483 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2988,7 +2988,7 @@ static int osc_process_config(struct obd_device *obd, u32 len, void *buf)
 	return osc_process_config_base(obd, buf);
 }
 
-static struct obd_ops osc_obd_ops = {
+static const struct obd_ops osc_obd_ops = {
 	.owner		= THIS_MODULE,
 	.setup		= osc_setup,
 	.precleanup	= osc_precleanup,

[lustre-devel] [PATCH 07/28] lustre: obdclass: fix module load locking.

[NeilBrown]

Safe module loading requires that we try_module_get() in a context
where the module cannot be unloaded, typically protected by
a spinlock that module-unload has to take.
This doesn't currently happen in class_get_type().

As free_module() calls synchronize_rcu() between calling the
exit function and freeing the module, we can use rcu_read_lock()
to check if the exit function has been called, and try_module_get()
if it hasn't.

We must also check the return status of try_module_get().

Reviewed-by: James Simmons <jsimmons@infradead.org>
Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/obdclass/genops.c |   24 ++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index dc09c302a246..e206bb401fe3 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -99,21 +99,31 @@ static struct obd_type *class_get_type(const char *name)
 {
 	struct obd_type *type;
 
+	rcu_read_lock();
 	type = class_search_type(name);
 
 	if (!type) {
 		const char *modname = name;
 
+		rcu_read_unlock();
 		if (!request_module("%s", modname)) {
 			CDEBUG(D_INFO, "Loaded module '%s'\n", modname);
-			type = class_search_type(name);
 		} else {
 			LCONSOLE_ERROR_MSG(0x158, "Can't load module '%s'\n",
 					   modname);
 		}
+		rcu_read_lock();
+		type = class_search_type(name);
 	}
 	if (type) {
-		if (try_module_get(type->typ_dt_ops->owner)) {
+		/*
+		 * Holding rcu_read_lock() matches the synchronize_rcu() call
+		 * in free_module() and ensures that if type->typ_dt_ops is
+		 * not yet NULL, then the module won't be freed until after
+		 * we rcu_read_unlock().
+		 */
+		const struct obd_ops *dt_ops = READ_ONCE(type->typ_dt_ops);
+		if (dt_ops && try_module_get(dt_ops->owner)) {
 			atomic_inc(&type->typ_refcnt);
 			/* class_search_type() returned a counted reference,
 			 * but we don't need that count any more as
@@ -125,6 +135,7 @@ static struct obd_type *class_get_type(const char *name)
 			type = NULL;
 		}
 	}
+	rcu_read_unlock();
 	return type;
 }
 
@@ -244,11 +255,18 @@ int class_unregister_type(const char *name)
 		return -EINVAL;
 	}
 
+	/*
+	 * Ensure that class_get_type doesn't try to get the module
+	 * as it could be freed before the obd_type is released.
+	 * synchronize_rcu() will be called before the module
+	 * is freed.
+	 */
+	type->typ_dt_ops = NULL;
+
 	if (atomic_read(&type->typ_refcnt)) {
 		CERROR("type %s has refcount (%d)\n", name, atomic_read(&type->typ_refcnt));
 		/* This is a bad situation, let's make the best of it */
 		/* Remove ops, but leave the name for debugging */
-		type->typ_dt_ops = NULL;
 		type->typ_md_ops = NULL;
 		kobject_put(&type->typ_kobj);
 		return -EBUSY;

[lustre-devel] [PATCH 08/28] lustre: convert rsi_sem to a spinlock.

[NeilBrown]

This lock is never held over code that sleeps, and is
only ever held for short periods of time.
So a simple spinlock is best.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/obd_class.h  |    3 +--
 drivers/staging/lustre/lustre/llite/llite_lib.c    |    6 +++---
 drivers/staging/lustre/lustre/llite/lproc_llite.c  |    4 ++--
 .../lustre/lustre/obdclass/lprocfs_status.c        |    8 ++++----
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
index 4567ef3b21dc..cdcc56dc4d1d 100644
--- a/drivers/staging/lustre/lustre/include/obd_class.h
+++ b/drivers/staging/lustre/lustre/include/obd_class.h
@@ -1690,12 +1690,11 @@ void statfs_pack(struct obd_statfs *osfs, struct kstatfs *sfs);
 void statfs_unpack(struct kstatfs *sfs, struct obd_statfs *osfs);
 
 /* root squash info */
-struct rw_semaphore;
 struct root_squash_info {
 	uid_t			rsi_uid;
 	gid_t			rsi_gid;
 	struct list_head	rsi_nosquash_nids;
-	struct rw_semaphore	rsi_sem;
+	spinlock_t		rsi_lock;	/* protects rsi_nosquash_nids */
 };
 
 /* linux-module.c */
diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 4e98eb4af9c0..cfe8105fe138 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -130,7 +130,7 @@ static struct ll_sb_info *ll_init_sbi(void)
 	sbi->ll_squash.rsi_uid = 0;
 	sbi->ll_squash.rsi_gid = 0;
 	INIT_LIST_HEAD(&sbi->ll_squash.rsi_nosquash_nids);
-	init_rwsem(&sbi->ll_squash.rsi_sem);
+	spin_lock_init(&sbi->ll_squash.rsi_lock);
 
 	return sbi;
 }
@@ -2576,7 +2576,7 @@ void ll_compute_rootsquash_state(struct ll_sb_info *sbi)
 	int i;
 
 	/* Update norootsquash flag */
-	down_write(&squash->rsi_sem);
+	spin_lock(&squash->rsi_lock);
 	if (list_empty(&squash->rsi_nosquash_nids)) {
 		spin_lock(&sbi->ll_lock);
 		sbi->ll_flags &= ~LL_SBI_NOROOTSQUASH;
@@ -2604,7 +2604,7 @@ void ll_compute_rootsquash_state(struct ll_sb_info *sbi)
 			sbi->ll_flags &= ~LL_SBI_NOROOTSQUASH;
 		spin_unlock(&sbi->ll_lock);
 	}
-	up_write(&squash->rsi_sem);
+	spin_unlock(&squash->rsi_lock);
 }
 
 /**
diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
index 5de846230cd2..8434e603a122 100644
--- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
+++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
@@ -1133,7 +1133,7 @@ static int ll_nosquash_nids_seq_show(struct seq_file *m, void *v)
 	struct root_squash_info *squash = &sbi->ll_squash;
 	int len;
 
-	down_read(&squash->rsi_sem);
+	spin_lock(&squash->rsi_lock);
 	if (!list_empty(&squash->rsi_nosquash_nids)) {
 		len = cfs_print_nidlist(m->buf + m->count, m->size - m->count,
 					&squash->rsi_nosquash_nids);
@@ -1142,7 +1142,7 @@ static int ll_nosquash_nids_seq_show(struct seq_file *m, void *v)
 	} else {
 		seq_puts(m, "NONE\n");
 	}
-	up_read(&squash->rsi_sem);
+	spin_unlock(&squash->rsi_lock);
 
 	return 0;
 }
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 71bf409240f5..29c6b5d7abec 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -1751,10 +1751,10 @@ int lprocfs_wr_nosquash_nids(const char __user *buffer, unsigned long count,
 	if ((len == 4 && !strncmp(kernbuf, "NONE", len)) ||
 	    (len == 5 && !strncmp(kernbuf, "clear", len))) {
 		/* empty string is special case */
-		down_write(&squash->rsi_sem);
+		spin_lock(&squash->rsi_lock);
 		if (!list_empty(&squash->rsi_nosquash_nids))
 			cfs_free_nidlist(&squash->rsi_nosquash_nids);
-		up_write(&squash->rsi_sem);
+		spin_unlock(&squash->rsi_lock);
 		LCONSOLE_INFO("%s: nosquash_nids is cleared\n", name);
 		kfree(kernbuf);
 		return count;
@@ -1771,11 +1771,11 @@ int lprocfs_wr_nosquash_nids(const char __user *buffer, unsigned long count,
 	kfree(kernbuf);
 	kernbuf = NULL;
 
-	down_write(&squash->rsi_sem);
+	spin_lock(&squash->rsi_lock);
 	if (!list_empty(&squash->rsi_nosquash_nids))
 		cfs_free_nidlist(&squash->rsi_nosquash_nids);
 	list_splice(&tmp, &squash->rsi_nosquash_nids);
-	up_write(&squash->rsi_sem);
+	spin_unlock(&squash->rsi_lock);
 
 	return count;
 

[lustre-devel] [PATCH 09/28] lustre: ldlm: discard varname in ldlm_pool.

[NeilBrown]

This allocated buffer serves no purpose.
A constant string is copied into it, it is passed to some
function which copies it out again, then the buffer is freed.
Instead, we can pass the constant string to that function.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_internal.h |    2 --
 drivers/staging/lustre/lustre/ldlm/ldlm_pool.c     |   18 +++++-------------
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h b/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h
index d8dcf8a73e4b..c907d880274d 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h
@@ -31,8 +31,6 @@
  * Lustre is a trademark of Sun Microsystems, Inc.
  */
 
-#define MAX_STRING_SIZE 128
-
 extern int ldlm_srv_namespace_nr;
 extern int ldlm_cli_namespace_nr;
 extern struct mutex ldlm_srv_namespace_lock;
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
index 1f81795cc6f6..a984b5df7bc1 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c
@@ -503,9 +503,9 @@ LDLM_POOL_SYSFS_READER_NOLOCK_SHOW(lock_volume_factor, atomic);
 LDLM_POOL_SYSFS_WRITER_NOLOCK_STORE(lock_volume_factor, atomic);
 LUSTRE_RW_ATTR(lock_volume_factor);
 
-#define LDLM_POOL_ADD_VAR(name, var, ops)			\
+#define LDLM_POOL_ADD_VAR(_name, var, ops)			\
 	do {							\
-		snprintf(var_name, MAX_STRING_SIZE, #name);	\
+		pool_vars[0].name = #_name;			\
 		pool_vars[0].data = var;			\
 		pool_vars[0].fops = ops;			\
 		ldebugfs_add_vars(pl->pl_debugfs_entry, pool_vars, NULL);\
@@ -557,25 +557,18 @@ static int ldlm_pool_debugfs_init(struct ldlm_pool *pl)
 						 ns_pool);
 	struct dentry *debugfs_ns_parent;
 	struct lprocfs_vars pool_vars[2];
-	char *var_name = NULL;
 	int rc = 0;
 
-	var_name = kzalloc(MAX_STRING_SIZE + 1, GFP_NOFS);
-	if (!var_name)
-		return -ENOMEM;
-
 	debugfs_ns_parent = ns->ns_debugfs_entry;
 	if (IS_ERR_OR_NULL(debugfs_ns_parent)) {
 		CERROR("%s: debugfs entry is not initialized\n",
 		       ldlm_ns_name(ns));
 		rc = -EINVAL;
-		goto out_free_name;
+		goto out;
 	}
 	pl->pl_debugfs_entry = debugfs_create_dir("pool", debugfs_ns_parent);
 
-	var_name[MAX_STRING_SIZE] = '\0';
 	memset(pool_vars, 0, sizeof(pool_vars));
-	pool_vars[0].name = var_name;
 
 	LDLM_POOL_ADD_VAR(state, pl, &lprocfs_pool_state_fops);
 
@@ -583,7 +576,7 @@ static int ldlm_pool_debugfs_init(struct ldlm_pool *pl)
 					   LDLM_POOL_FIRST_STAT, 0);
 	if (!pl->pl_stats) {
 		rc = -ENOMEM;
-		goto out_free_name;
+		goto out;
 	}
 
 	lprocfs_counter_init(pl->pl_stats, LDLM_POOL_GRANTED_STAT,
@@ -622,8 +615,7 @@ static int ldlm_pool_debugfs_init(struct ldlm_pool *pl)
 	debugfs_create_file("stats", 0644, pl->pl_debugfs_entry, pl->pl_stats,
 			    &lprocfs_stats_seq_fops);
 
-out_free_name:
-	kfree(var_name);
+out:
 	return rc;
 }
 

[lustre-devel] [PATCH 10/28] lustre: lprocfs: use log2.h macros instead of shift loop.

[NeilBrown]

These shift loops seem to be trying to avoid doing a
multiplication.
We same effect can be achieved more transparently using
rounddown_pow_of_two().  Even though there is a multiplication
in the C code, the resulting machine code just does a single shift.

As rounddown_pow_of_two() is not defined for 0, and as we cannot be
positively use the blk_size is non-zero, use blk_size ?: 1.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/lustre/obdclass/lprocfs_status.c        |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 29c6b5d7abec..2af9dada195e 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -374,9 +374,7 @@ static ssize_t kbytestotal_show(struct kobject *kobj, struct attribute *attr,
 		u32 blk_size = osfs.os_bsize >> 10;
 		u64 result = osfs.os_blocks;
 
-		while (blk_size >>= 1)
-			result <<= 1;
-
+		result *= rounddown_pow_of_two(blk_size ?: 1);
 		return sprintf(buf, "%llu\n", result);
 	}
 
@@ -397,8 +395,7 @@ static ssize_t kbytesfree_show(struct kobject *kobj, struct attribute *attr,
 		u32 blk_size = osfs.os_bsize >> 10;
 		u64 result = osfs.os_bfree;
 
-		while (blk_size >>= 1)
-			result <<= 1;
+		result *= rounddown_pow_of_two(blk_size ?: 1);
 
 		return sprintf(buf, "%llu\n", result);
 	}
@@ -420,8 +417,7 @@ static ssize_t kbytesavail_show(struct kobject *kobj, struct attribute *attr,
 		u32 blk_size = osfs.os_bsize >> 10;
 		u64 result = osfs.os_bavail;
 
-		while (blk_size >>= 1)
-			result <<= 1;
+		result *= rounddown_pow_of_two(blk_size ?: 1);
 
 		return sprintf(buf, "%llu\n", result);
 	}

[lustre-devel] [PATCH 11/28] lustre: handles: discard h_owner in favour of h_ops

[NeilBrown]

lustre_handles  assigned a  64bit  unique identifier  (a 'cookie')  to
objects of  various types and  stored them  in a hash  table, allowing
them to be accessed by the cookie.

The is a facility for type checking by recording an 'owner' for each
object, and checking the owner on lookup.  Unfortunately this is not
used - owner is always zero.

Eahc object also contains an h_ops pointer which can be used to
reliably identify an owner.

So discard h_owner, pass and 'ops' pointer to class_handle2object(),
and only return objects for which the h_ops matches.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/include/lustre_handles.h |    7 +++----
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    2 +-
 drivers/staging/lustre/lustre/obdclass/genops.c    |    3 ++-
 .../lustre/lustre/obdclass/lustre_handles.c        |    6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
index 683680891e4c..9a4b1a821e7b 100644
--- a/drivers/staging/lustre/lustre/include/lustre_handles.h
+++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
@@ -65,8 +65,7 @@ struct portals_handle_ops {
 struct portals_handle {
 	struct list_head		h_link;
 	u64				h_cookie;
-	const void			*h_owner;
-	struct portals_handle_ops	*h_ops;
+	const struct portals_handle_ops	*h_ops;
 
 	/* newly added fields to handle the RCU issue. -jxiong */
 	struct rcu_head			h_rcu;
@@ -79,9 +78,9 @@ struct portals_handle {
 
 /* Add a handle to the hash table */
 void class_handle_hash(struct portals_handle *,
-		       struct portals_handle_ops *ops);
+		       const struct portals_handle_ops *ops);
 void class_handle_unhash(struct portals_handle *);
-void *class_handle2object(u64 cookie, const void *owner);
+void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops);
 void class_handle_free_cb(struct rcu_head *rcu);
 int class_handle_init(void);
 void class_handle_cleanup(void);
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 7ec5fc900da8..768cccc1fa82 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -515,7 +515,7 @@ struct ldlm_lock *__ldlm_handle2lock(const struct lustre_handle *handle,
 
 	LASSERT(handle);
 
-	lock = class_handle2object(handle->cookie, NULL);
+	lock = class_handle2object(handle->cookie, &lock_handle_ops);
 	if (!lock)
 		return NULL;
 
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index e206bb401fe3..42859fbca330 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -708,6 +708,7 @@ int obd_init_caches(void)
 	return -ENOMEM;
 }
 
+static struct portals_handle_ops export_handle_ops;
 /* map connection to client */
 struct obd_export *class_conn2export(struct lustre_handle *conn)
 {
@@ -724,7 +725,7 @@ struct obd_export *class_conn2export(struct lustre_handle *conn)
 	}
 
 	CDEBUG(D_INFO, "looking for export cookie %#llx\n", conn->cookie);
-	export = class_handle2object(conn->cookie, NULL);
+	export = class_handle2object(conn->cookie, &export_handle_ops);
 	return export;
 }
 EXPORT_SYMBOL(class_conn2export);
diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index 0674afb0059f..32b70d613f71 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -59,7 +59,7 @@ static struct handle_bucket {
  * global (per-node) hash-table.
  */
 void class_handle_hash(struct portals_handle *h,
-		       struct portals_handle_ops *ops)
+		       const struct portals_handle_ops *ops)
 {
 	struct handle_bucket *bucket;
 
@@ -132,7 +132,7 @@ void class_handle_unhash(struct portals_handle *h)
 }
 EXPORT_SYMBOL(class_handle_unhash);
 
-void *class_handle2object(u64 cookie, const void *owner)
+void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
 {
 	struct handle_bucket *bucket;
 	struct portals_handle *h;
@@ -147,7 +147,7 @@ void *class_handle2object(u64 cookie, const void *owner)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(h, &bucket->head, h_link) {
-		if (h->h_cookie != cookie || h->h_owner != owner)
+		if (h->h_cookie != cookie || h->h_ops != ops)
 			continue;
 
 		spin_lock(&h->h_lock);

[lustre-devel] [PATCH 12/28] lustre: handle: move refcount into the lustre_handle.

[NeilBrown]

Every object with a lustre_handle has (and must have) a refcount.
The lustre_handles code needs a call-out to increment this.
To simplify things, move the refcount into the lustre_handle
and discard the call-out.

In order to preserve the same debug messages, we store and object type
name in the portals_handle_ops, and use that in a CDEBUG() when
incrementing the ref count.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/lustre_dlm.h |    6 --
 .../staging/lustre/lustre/include/lustre_export.h  |    1 
 .../staging/lustre/lustre/include/lustre_handles.h |    4 +-
 .../staging/lustre/lustre/include/lustre_import.h  |    2 -
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |   36 ++++++--------
 drivers/staging/lustre/lustre/obdclass/genops.c    |   50 ++++++++------------
 .../lustre/lustre/obdclass/lustre_handles.c        |    5 ++
 .../staging/lustre/lustre/obdecho/echo_client.c    |    2 -
 drivers/staging/lustre/lustre/ptlrpc/service.c     |    4 +-
 9 files changed, 45 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
index 1bd511906f69..fd9b0f870c93 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
@@ -590,12 +590,6 @@ struct ldlm_lock {
 	 * Must be first in the structure.
 	 */
 	struct portals_handle		l_handle;
-	/**
-	 * Lock reference count.
-	 * This is how many users have pointers to actual structure, so that
-	 * we do not accidentally free lock structure that is in use.
-	 */
-	atomic_t			l_refc;
 	/**
 	 * Internal spinlock protects l_resource.  We should hold this lock
 	 * first before taking res_lock.
diff --git a/drivers/staging/lustre/lustre/include/lustre_export.h b/drivers/staging/lustre/lustre/include/lustre_export.h
index fb34e0b7de35..d05323313f55 100644
--- a/drivers/staging/lustre/lustre/include/lustre_export.h
+++ b/drivers/staging/lustre/lustre/include/lustre_export.h
@@ -67,7 +67,6 @@ struct obd_export {
 	 * what export they are talking to.
 	 */
 	struct portals_handle		exp_handle;
-	refcount_t			exp_refcount;
 	/**
 	 * Set of counters below is to track where export references are
 	 * kept. The exp_rpc_count is used for reconnect handling also,
diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
index 9a4b1a821e7b..be5d41b1a398 100644
--- a/drivers/staging/lustre/lustre/include/lustre_handles.h
+++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
@@ -46,8 +46,9 @@
 #include <linux/types.h>
 
 struct portals_handle_ops {
-	void (*hop_addref)(void *object);
 	void (*hop_free)(void *object, int size);
+	/* hop_type is used for some debugging messages */
+	char *hop_type;
 };
 
 /* These handles are most easily used by having them appear at the very top of
@@ -66,6 +67,7 @@ struct portals_handle {
 	struct list_head		h_link;
 	u64				h_cookie;
 	const struct portals_handle_ops	*h_ops;
+	refcount_t			h_ref;
 
 	/* newly added fields to handle the RCU issue. -jxiong */
 	struct rcu_head			h_rcu;
diff --git a/drivers/staging/lustre/lustre/include/lustre_import.h b/drivers/staging/lustre/lustre/include/lustre_import.h
index fc1f87cc1172..6c830da797c1 100644
--- a/drivers/staging/lustre/lustre/include/lustre_import.h
+++ b/drivers/staging/lustre/lustre/include/lustre_import.h
@@ -154,8 +154,6 @@ struct import_state_hist {
 struct obd_import {
 	/** Local handle (== id) for this import. */
 	struct portals_handle		imp_handle;
-	/** Reference counter */
-	atomic_t			imp_refcount;
 	struct lustre_handle		imp_dlm_handle; /* client's ldlm export */
 	/** Currently active connection */
 	struct ptlrpc_connection       *imp_connection;
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 768cccc1fa82..ecd3f4a93e8d 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -148,7 +148,7 @@ EXPORT_SYMBOL(ldlm_it2str);
  */
 struct ldlm_lock *ldlm_lock_get(struct ldlm_lock *lock)
 {
-	atomic_inc(&lock->l_refc);
+	refcount_inc(&lock->l_handle.h_ref);
 	return lock;
 }
 EXPORT_SYMBOL(ldlm_lock_get);
@@ -161,8 +161,8 @@ EXPORT_SYMBOL(ldlm_lock_get);
 void ldlm_lock_put(struct ldlm_lock *lock)
 {
 	LASSERT(lock->l_resource != LP_POISON);
-	LASSERT(atomic_read(&lock->l_refc) > 0);
-	if (atomic_dec_and_test(&lock->l_refc)) {
+	LASSERT(refcount_read(&lock->l_handle.h_ref) > 0);
+	if (refcount_dec_and_test(&lock->l_handle.h_ref)) {
 		struct ldlm_resource *res;
 
 		LDLM_DEBUG(lock,
@@ -356,12 +356,6 @@ void ldlm_lock_destroy_nolock(struct ldlm_lock *lock)
 	}
 }
 
-/* this is called by portals_handle2object with the handle lock taken */
-static void lock_handle_addref(void *lock)
-{
-	LDLM_LOCK_GET((struct ldlm_lock *)lock);
-}
-
 static void lock_handle_free(void *lock, int size)
 {
 	LASSERT(size == sizeof(struct ldlm_lock));
@@ -369,8 +363,8 @@ static void lock_handle_free(void *lock, int size)
 }
 
 static struct portals_handle_ops lock_handle_ops = {
-	.hop_addref = lock_handle_addref,
 	.hop_free   = lock_handle_free,
+	.hop_type   = "ldlm",
 };
 
 /**
@@ -395,7 +389,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
 	lock->l_resource = resource;
 	lu_ref_add(&resource->lr_reference, "lock", lock);
 
-	atomic_set(&lock->l_refc, 2);
+	refcount_set(&lock->l_handle.h_ref, 2);
 	INIT_LIST_HEAD(&lock->l_res_link);
 	INIT_LIST_HEAD(&lock->l_lru);
 	INIT_LIST_HEAD(&lock->l_pending_chain);
@@ -1996,13 +1990,13 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 &vaf,
 				 lock,
 				 lock->l_handle.h_cookie,
-				 atomic_read(&lock->l_refc),
+				 refcount_read(&lock->l_handle.h_ref),
 				 lock->l_readers, lock->l_writers,
 				 ldlm_lockname[lock->l_granted_mode],
 				 ldlm_lockname[lock->l_req_mode],
 				 lock->l_flags, nid,
 				 lock->l_remote_handle.cookie,
-				 exp ? refcount_read(&exp->exp_refcount) : -99,
+				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
 				 lock->l_pid, lock->l_callback_timeout,
 				 lock->l_lvb_type);
 		va_end(args);
@@ -2016,7 +2010,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 &vaf,
 				 ldlm_lock_to_ns_name(lock), lock,
 				 lock->l_handle.h_cookie,
-				 atomic_read(&lock->l_refc),
+				 refcount_read(&lock->l_handle.h_ref),
 				 lock->l_readers, lock->l_writers,
 				 ldlm_lockname[lock->l_granted_mode],
 				 ldlm_lockname[lock->l_req_mode],
@@ -2029,7 +2023,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 lock->l_req_extent.end,
 				 lock->l_flags, nid,
 				 lock->l_remote_handle.cookie,
-				 exp ? refcount_read(&exp->exp_refcount) : -99,
+				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
 				 lock->l_pid, lock->l_callback_timeout,
 				 lock->l_lvb_type);
 		break;
@@ -2040,7 +2034,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 &vaf,
 				 ldlm_lock_to_ns_name(lock), lock,
 				 lock->l_handle.h_cookie,
-				 atomic_read(&lock->l_refc),
+				 refcount_read(&lock->l_handle.h_ref),
 				 lock->l_readers, lock->l_writers,
 				 ldlm_lockname[lock->l_granted_mode],
 				 ldlm_lockname[lock->l_req_mode],
@@ -2052,7 +2046,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 lock->l_policy_data.l_flock.end,
 				 lock->l_flags, nid,
 				 lock->l_remote_handle.cookie,
-				 exp ? refcount_read(&exp->exp_refcount) : -99,
+				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
 				 lock->l_pid, lock->l_callback_timeout);
 		break;
 
@@ -2062,7 +2056,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 &vaf,
 				 ldlm_lock_to_ns_name(lock),
 				 lock, lock->l_handle.h_cookie,
-				 atomic_read(&lock->l_refc),
+				 refcount_read(&lock->l_handle.h_ref),
 				 lock->l_readers, lock->l_writers,
 				 ldlm_lockname[lock->l_granted_mode],
 				 ldlm_lockname[lock->l_req_mode],
@@ -2072,7 +2066,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 ldlm_typename[resource->lr_type],
 				 lock->l_flags, nid,
 				 lock->l_remote_handle.cookie,
-				 exp ? refcount_read(&exp->exp_refcount) : -99,
+				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
 				 lock->l_pid, lock->l_callback_timeout,
 				 lock->l_lvb_type);
 		break;
@@ -2083,7 +2077,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 &vaf,
 				 ldlm_lock_to_ns_name(lock),
 				 lock, lock->l_handle.h_cookie,
-				 atomic_read(&lock->l_refc),
+				 refcount_read(&lock->l_handle.h_ref),
 				 lock->l_readers, lock->l_writers,
 				 ldlm_lockname[lock->l_granted_mode],
 				 ldlm_lockname[lock->l_req_mode],
@@ -2092,7 +2086,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 				 ldlm_typename[resource->lr_type],
 				 lock->l_flags, nid,
 				 lock->l_remote_handle.cookie,
-				 exp ? refcount_read(&exp->exp_refcount) : -99,
+				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
 				 lock->l_pid, lock->l_callback_timeout,
 				 lock->l_lvb_type);
 		break;
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index 42859fbca330..c6a5e6569c88 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -753,7 +753,7 @@ static void class_export_destroy(struct obd_export *exp)
 {
 	struct obd_device *obd = exp->exp_obd;
 
-	LASSERT(refcount_read(&exp->exp_refcount) == 0);
+	LASSERT(refcount_read(&exp->exp_handle.h_ref) == 0);
 	LASSERT(obd);
 
 	CDEBUG(D_IOCTL, "destroying export %p/%s for %s\n", exp,
@@ -777,33 +777,28 @@ static void class_export_destroy(struct obd_export *exp)
 	OBD_FREE_RCU(exp, sizeof(*exp), &exp->exp_handle);
 }
 
-static void export_handle_addref(void *export)
-{
-	class_export_get(export);
-}
-
 static struct portals_handle_ops export_handle_ops = {
-	.hop_addref	= export_handle_addref,
 	.hop_free	= NULL,
+	.hop_type	= "export",
 };
 
 struct obd_export *class_export_get(struct obd_export *exp)
 {
-	refcount_inc(&exp->exp_refcount);
-	CDEBUG(D_INFO, "GETting export %p : new refcount %d\n", exp,
-	       refcount_read(&exp->exp_refcount));
+	refcount_inc(&exp->exp_handle.h_ref);
+	CDEBUG(D_INFO, "GET export %p refcount=%d\n", exp,
+	       refcount_read(&exp->exp_handle.h_ref));
 	return exp;
 }
 EXPORT_SYMBOL(class_export_get);
 
 void class_export_put(struct obd_export *exp)
 {
-	LASSERT(refcount_read(&exp->exp_refcount) >  0);
-	LASSERT(refcount_read(&exp->exp_refcount) < LI_POISON);
+	LASSERT(refcount_read(&exp->exp_handle.h_ref) >  0);
+	LASSERT(refcount_read(&exp->exp_handle.h_ref) < LI_POISON);
 	CDEBUG(D_INFO, "PUTting export %p : new refcount %d\n", exp,
-	       refcount_read(&exp->exp_refcount) - 1);
+	       refcount_read(&exp->exp_handle.h_ref) - 1);
 
-	if (refcount_dec_and_test(&exp->exp_refcount)) {
+	if (refcount_dec_and_test(&exp->exp_handle.h_ref)) {
 		struct obd_device *obd = exp->exp_obd;
 
 		CDEBUG(D_IOCTL, "final put %p/%s\n",
@@ -853,7 +848,7 @@ static struct obd_export *__class_new_export(struct obd_device *obd,
 
 	export->exp_conn_cnt = 0;
 	/* 2 = class_handle_hash + last */
-	refcount_set(&export->exp_refcount, 2);
+	refcount_set(&export->exp_handle.h_ref, 2);
 	atomic_set(&export->exp_rpc_count, 0);
 	atomic_set(&export->exp_cb_count, 0);
 	atomic_set(&export->exp_locks_count, 0);
@@ -956,7 +951,7 @@ static void class_import_destroy(struct obd_import *imp)
 	CDEBUG(D_IOCTL, "destroying import %p for %s\n", imp,
 	       imp->imp_obd->obd_name);
 
-	LASSERT_ATOMIC_ZERO(&imp->imp_refcount);
+	LASSERT(refcount_read(&imp->imp_handle.h_ref) == 0);
 
 	ptlrpc_put_connection_superhack(imp->imp_connection);
 
@@ -973,21 +968,16 @@ static void class_import_destroy(struct obd_import *imp)
 	OBD_FREE_RCU(imp, sizeof(*imp), &imp->imp_handle);
 }
 
-static void import_handle_addref(void *import)
-{
-	class_import_get(import);
-}
-
 static struct portals_handle_ops import_handle_ops = {
-	.hop_addref	= import_handle_addref,
 	.hop_free	= NULL,
+	.hop_type	= "import",
 };
 
 struct obd_import *class_import_get(struct obd_import *import)
 {
-	atomic_inc(&import->imp_refcount);
-	CDEBUG(D_INFO, "import %p refcount=%d obd=%s\n", import,
-	       atomic_read(&import->imp_refcount),
+	refcount_inc(&import->imp_handle.h_ref);
+	CDEBUG(D_INFO, "GET import %p refcount=%d obd=%s\n", import,
+	       refcount_read(&import->imp_handle.h_ref),
 	       import->imp_obd->obd_name);
 	return import;
 }
@@ -995,19 +985,19 @@ EXPORT_SYMBOL(class_import_get);
 
 void class_import_put(struct obd_import *imp)
 {
-	LASSERT_ATOMIC_GT_LT(&imp->imp_refcount, 0, LI_POISON);
+	LASSERT(refcount_read(&imp->imp_handle.h_ref) > 0);
 
 	CDEBUG(D_INFO, "import %p refcount=%d obd=%s\n", imp,
-	       atomic_read(&imp->imp_refcount) - 1,
+	       refcount_read(&imp->imp_handle.h_ref) - 1,
 	       imp->imp_obd->obd_name);
 
-	if (atomic_dec_and_test(&imp->imp_refcount)) {
+	if (refcount_dec_and_test(&imp->imp_handle.h_ref)) {
 		CDEBUG(D_INFO, "final put import %p\n", imp);
 		obd_zombie_import_add(imp);
 	}
 
 	/* catch possible import put race */
-	LASSERT_ATOMIC_GE_LT(&imp->imp_refcount, 0, LI_POISON);
+	LASSERT(refcount_read(&imp->imp_handle.h_ref) >= 0);
 }
 EXPORT_SYMBOL(class_import_put);
 
@@ -1057,7 +1047,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
 	init_waitqueue_head(&imp->imp_recovery_waitq);
 	INIT_WORK(&imp->imp_zombie_work, obd_zombie_imp_cull);
 
-	atomic_set(&imp->imp_refcount, 2);
+	refcount_set(&imp->imp_handle.h_ref, 2);
 	atomic_set(&imp->imp_unregistering, 0);
 	atomic_set(&imp->imp_inflight, 0);
 	atomic_set(&imp->imp_replay_inflight, 0);
diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index 32b70d613f71..32cd6aedd5f6 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -152,7 +152,10 @@ void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
 
 		spin_lock(&h->h_lock);
 		if (likely(h->h_in != 0)) {
-			h->h_ops->hop_addref(h);
+			refcount_inc(&h->h_ref);
+			CDEBUG(D_INFO, "GET %s %p refcount=%d\n",
+			       h->h_ops->hop_type, h,
+			       refcount_read(&h->h_ref));
 			retval = h;
 		}
 		spin_unlock(&h->h_lock);
diff --git a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
index baf34c85c4b5..6f00eee6170a 100644
--- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
+++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
@@ -1638,7 +1638,7 @@ static int echo_client_cleanup(struct obd_device *obddev)
 		return -EBUSY;
 	}
 
-	LASSERT(refcount_read(&ec->ec_exp->exp_refcount) > 0);
+	LASSERT(refcount_read(&ec->ec_exp->exp_handle.h_ref) > 0);
 	rc = obd_disconnect(ec->ec_exp);
 	if (rc != 0)
 		CERROR("fail to disconnect device: %d\n", rc);
diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
index 5a7e9fa6e5c8..5e541ca0dc00 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -1697,7 +1697,7 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt,
 	       (request->rq_export ?
 		(char *)request->rq_export->exp_client_uuid.uuid : "0"),
 	       (request->rq_export ?
-		refcount_read(&request->rq_export->exp_refcount) : -99),
+		refcount_read(&request->rq_export->exp_handle.h_ref) : -99),
 	       lustre_msg_get_status(request->rq_reqmsg), request->rq_xid,
 	       libcfs_id2str(request->rq_peer),
 	       lustre_msg_get_opc(request->rq_reqmsg));
@@ -1741,7 +1741,7 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt,
 	       (request->rq_export ?
 		(char *)request->rq_export->exp_client_uuid.uuid : "0"),
 	       (request->rq_export ?
-		refcount_read(&request->rq_export->exp_refcount) : -99),
+		refcount_read(&request->rq_export->exp_handle.h_ref) : -99),
 	       lustre_msg_get_status(request->rq_reqmsg),
 	       request->rq_xid,
 	       libcfs_id2str(request->rq_peer),

[lustre-devel] [PATCH 13/28] lustre: discard OBD_FREE_RCU

[NeilBrown]

OBD_FREE_RCU and the hop_free call-back together form an overly
complex mechanism equivalent to kfree_rcu() or call_rcu(...).
Discard them and use the simpler approach.

This removes the only use for the field h_size, so discard
that too.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/include/lustre_handles.h |    3 ---
 .../staging/lustre/lustre/include/obd_support.h    |   10 ----------
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |   16 ++++++++--------
 drivers/staging/lustre/lustre/obdclass/genops.c    |    6 ++----
 .../lustre/lustre/obdclass/lustre_handles.c        |   15 ---------------
 5 files changed, 10 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
index be5d41b1a398..8fb42851f6d1 100644
--- a/drivers/staging/lustre/lustre/include/lustre_handles.h
+++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
@@ -46,7 +46,6 @@
 #include <linux/types.h>
 
 struct portals_handle_ops {
-	void (*hop_free)(void *object, int size);
 	/* hop_type is used for some debugging messages */
 	char *hop_type;
 };
@@ -72,7 +71,6 @@ struct portals_handle {
 	/* newly added fields to handle the RCU issue. -jxiong */
 	struct rcu_head			h_rcu;
 	spinlock_t			h_lock;
-	unsigned int			h_size:31;
 	unsigned int			h_in:1;
 };
 
@@ -83,7 +81,6 @@ void class_handle_hash(struct portals_handle *,
 		       const struct portals_handle_ops *ops);
 void class_handle_unhash(struct portals_handle *);
 void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops);
-void class_handle_free_cb(struct rcu_head *rcu);
 int class_handle_init(void);
 void class_handle_cleanup(void);
 
diff --git a/drivers/staging/lustre/lustre/include/obd_support.h b/drivers/staging/lustre/lustre/include/obd_support.h
index 3e15cac04ac5..91d6a862fa92 100644
--- a/drivers/staging/lustre/lustre/include/obd_support.h
+++ b/drivers/staging/lustre/lustre/include/obd_support.h
@@ -503,16 +503,6 @@ extern char obd_jobid_var[];
 #define POISON_PAGE(page, val) do { } while (0)
 #endif
 
-#define OBD_FREE_RCU(ptr, size, handle)			\
-do {							\
-	struct portals_handle *__h = (handle);		\
-							\
-	__h->h_cookie = (unsigned long)(ptr);		\
-	__h->h_size = (size);				\
-	call_rcu(&__h->h_rcu, class_handle_free_cb);	\
-	POISON_PTR(ptr);				\
-} while (0)
-
 #define KEY_IS(str)					\
 	(keylen >= (sizeof(str) - 1) &&			\
 	memcmp(key, str, (sizeof(str) - 1)) == 0)
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index ecd3f4a93e8d..18f018d27936 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -153,6 +153,13 @@ struct ldlm_lock *ldlm_lock_get(struct ldlm_lock *lock)
 }
 EXPORT_SYMBOL(ldlm_lock_get);
 
+static void lock_handle_free(struct rcu_head *rcu)
+{
+	struct ldlm_lock *lock = container_of(rcu, struct ldlm_lock,
+					      l_handle.h_rcu);
+	kmem_cache_free(ldlm_lock_slab, lock);
+}
+
 /**
  * Release lock reference.
  *
@@ -186,7 +193,7 @@ void ldlm_lock_put(struct ldlm_lock *lock)
 		kfree(lock->l_lvb_data);
 
 		lu_ref_fini(&lock->l_reference);
-		OBD_FREE_RCU(lock, sizeof(*lock), &lock->l_handle);
+		call_rcu(&lock->l_handle.h_rcu, lock_handle_free);
 	}
 }
 EXPORT_SYMBOL(ldlm_lock_put);
@@ -356,14 +363,7 @@ void ldlm_lock_destroy_nolock(struct ldlm_lock *lock)
 	}
 }
 
-static void lock_handle_free(void *lock, int size)
-{
-	LASSERT(size == sizeof(struct ldlm_lock));
-	kmem_cache_free(ldlm_lock_slab, lock);
-}
-
 static struct portals_handle_ops lock_handle_ops = {
-	.hop_free   = lock_handle_free,
 	.hop_type   = "ldlm",
 };
 
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index c6a5e6569c88..ed306bbfbfb8 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -774,11 +774,10 @@ static void class_export_destroy(struct obd_export *exp)
 	if (exp != obd->obd_self_export)
 		class_decref(obd, "export", exp);
 
-	OBD_FREE_RCU(exp, sizeof(*exp), &exp->exp_handle);
+	kfree_rcu(exp, exp_handle.h_rcu);
 }
 
 static struct portals_handle_ops export_handle_ops = {
-	.hop_free	= NULL,
 	.hop_type	= "export",
 };
 
@@ -965,11 +964,10 @@ static void class_import_destroy(struct obd_import *imp)
 
 	LASSERT(!imp->imp_sec);
 	class_decref(imp->imp_obd, "import", imp);
-	OBD_FREE_RCU(imp, sizeof(*imp), &imp->imp_handle);
+	kfree_rcu(imp, imp_handle.h_rcu);
 }
 
 static struct portals_handle_ops import_handle_ops = {
-	.hop_free	= NULL,
 	.hop_type	= "import",
 };
 
diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index 32cd6aedd5f6..8aece57ec8c9 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -167,21 +167,6 @@ void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
 }
 EXPORT_SYMBOL(class_handle2object);
 
-void class_handle_free_cb(struct rcu_head *rcu)
-{
-	struct portals_handle *h;
-	void *ptr;
-
-	h = container_of(rcu, struct portals_handle, h_rcu);
-	ptr = (void *)(unsigned long)h->h_cookie;
-
-	if (h->h_ops->hop_free)
-		h->h_ops->hop_free(ptr, h->h_size);
-	else
-		kfree(ptr);
-}
-EXPORT_SYMBOL(class_handle_free_cb);
-
 int class_handle_init(void)
 {
 	struct handle_bucket *bucket;

[lustre-devel] [PATCH 14/28] lustre: portals_handle: rename ops to owner

[NeilBrown]

Now the portals_handle_ops contains only a char*,
it is functioning primarily to identify the owner of each handle.
So change the name to h_owner, and the type to char*.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/include/lustre_handles.h |   12 +++---------
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    8 +++-----
 drivers/staging/lustre/lustre/obdclass/genops.c    |   17 ++++++-----------
 .../lustre/lustre/obdclass/lustre_handles.c        |   15 +++++++--------
 4 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
index 8fb42851f6d1..ebbbb01710e7 100644
--- a/drivers/staging/lustre/lustre/include/lustre_handles.h
+++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
@@ -45,11 +45,6 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
-struct portals_handle_ops {
-	/* hop_type is used for some debugging messages */
-	char *hop_type;
-};
-
 /* These handles are most easily used by having them appear at the very top of
  * whatever object that you want to make handles for.  ie:
  *
@@ -65,7 +60,7 @@ struct portals_handle_ops {
 struct portals_handle {
 	struct list_head		h_link;
 	u64				h_cookie;
-	const struct portals_handle_ops	*h_ops;
+	char				*h_owner;
 	refcount_t			h_ref;
 
 	/* newly added fields to handle the RCU issue. -jxiong */
@@ -77,10 +72,9 @@ struct portals_handle {
 /* handles.c */
 
 /* Add a handle to the hash table */
-void class_handle_hash(struct portals_handle *,
-		       const struct portals_handle_ops *ops);
+void class_handle_hash(struct portals_handle *, char *owner);
 void class_handle_unhash(struct portals_handle *);
-void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops);
+void *class_handle2object(u64 cookie, char *owner);
 int class_handle_init(void);
 void class_handle_cleanup(void);
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 18f018d27936..56a2d1dcd663 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -363,9 +363,7 @@ void ldlm_lock_destroy_nolock(struct ldlm_lock *lock)
 	}
 }
 
-static struct portals_handle_ops lock_handle_ops = {
-	.hop_type   = "ldlm",
-};
+static const char lock_handle_owner[] = "ldlm";
 
 /**
  *
@@ -405,7 +403,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
 	lprocfs_counter_incr(ldlm_res_to_ns(resource)->ns_stats,
 			     LDLM_NSS_LOCKS);
 	INIT_LIST_HEAD(&lock->l_handle.h_link);
-	class_handle_hash(&lock->l_handle, &lock_handle_ops);
+	class_handle_hash(&lock->l_handle, lock_handle_owner);
 
 	lu_ref_init(&lock->l_reference);
 	lu_ref_add(&lock->l_reference, "hash", lock);
@@ -509,7 +507,7 @@ struct ldlm_lock *__ldlm_handle2lock(const struct lustre_handle *handle,
 
 	LASSERT(handle);
 
-	lock = class_handle2object(handle->cookie, &lock_handle_ops);
+	lock = class_handle2object(handle->cookie, lock_handle_owner);
 	if (!lock)
 		return NULL;
 
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index ed306bbfbfb8..e0da46e7d355 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -708,7 +708,8 @@ int obd_init_caches(void)
 	return -ENOMEM;
 }
 
-static struct portals_handle_ops export_handle_ops;
+static const char export_handle_owner[] = "export";
+
 /* map connection to client */
 struct obd_export *class_conn2export(struct lustre_handle *conn)
 {
@@ -725,7 +726,7 @@ struct obd_export *class_conn2export(struct lustre_handle *conn)
 	}
 
 	CDEBUG(D_INFO, "looking for export cookie %#llx\n", conn->cookie);
-	export = class_handle2object(conn->cookie, &export_handle_ops);
+	export = class_handle2object(conn->cookie, export_handle_owner);
 	return export;
 }
 EXPORT_SYMBOL(class_conn2export);
@@ -777,10 +778,6 @@ static void class_export_destroy(struct obd_export *exp)
 	kfree_rcu(exp, exp_handle.h_rcu);
 }
 
-static struct portals_handle_ops export_handle_ops = {
-	.hop_type	= "export",
-};
-
 struct obd_export *class_export_get(struct obd_export *exp)
 {
 	refcount_inc(&exp->exp_handle.h_ref);
@@ -863,7 +860,7 @@ static struct obd_export *__class_new_export(struct obd_device *obd,
 	INIT_LIST_HEAD(&export->exp_req_replay_queue);
 	INIT_LIST_HEAD(&export->exp_handle.h_link);
 	INIT_LIST_HEAD(&export->exp_hp_rpcs);
-	class_handle_hash(&export->exp_handle, &export_handle_ops);
+	class_handle_hash(&export->exp_handle, export_handle_owner);
 	spin_lock_init(&export->exp_lock);
 	spin_lock_init(&export->exp_rpc_lock);
 	spin_lock_init(&export->exp_bl_list_lock);
@@ -967,9 +964,7 @@ static void class_import_destroy(struct obd_import *imp)
 	kfree_rcu(imp, imp_handle.h_rcu);
 }
 
-static struct portals_handle_ops import_handle_ops = {
-	.hop_type	= "import",
-};
+static const char import_handle_owner[] = "import";
 
 struct obd_import *class_import_get(struct obd_import *import)
 {
@@ -1052,7 +1047,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
 	atomic_set(&imp->imp_inval_count, 0);
 	INIT_LIST_HEAD(&imp->imp_conn_list);
 	INIT_LIST_HEAD(&imp->imp_handle.h_link);
-	class_handle_hash(&imp->imp_handle, &import_handle_ops);
+	class_handle_hash(&imp->imp_handle, import_handle_owner);
 	init_imp_at(&imp->imp_at);
 
 	/* the default magic is V2, will be used in connect RPC, and
diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index 8aece57ec8c9..f41558ccdfcf 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -58,8 +58,7 @@ static struct handle_bucket {
  * Generate a unique 64bit cookie (hash) for a handle and insert it into
  * global (per-node) hash-table.
  */
-void class_handle_hash(struct portals_handle *h,
-		       const struct portals_handle_ops *ops)
+void class_handle_hash(struct portals_handle *h, char *owner)
 {
 	struct handle_bucket *bucket;
 
@@ -85,7 +84,7 @@ void class_handle_hash(struct portals_handle *h,
 	h->h_cookie = handle_base;
 	spin_unlock(&handle_base_lock);
 
-	h->h_ops = ops;
+	h->h_owner = owner;
 	spin_lock_init(&h->h_lock);
 
 	bucket = &handle_hash[h->h_cookie & HANDLE_HASH_MASK];
@@ -132,7 +131,7 @@ void class_handle_unhash(struct portals_handle *h)
 }
 EXPORT_SYMBOL(class_handle_unhash);
 
-void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
+void *class_handle2object(u64 cookie, char *owner)
 {
 	struct handle_bucket *bucket;
 	struct portals_handle *h;
@@ -147,14 +146,14 @@ void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(h, &bucket->head, h_link) {
-		if (h->h_cookie != cookie || h->h_ops != ops)
+		if (h->h_cookie != cookie || h->h_owner != owner)
 			continue;
 
 		spin_lock(&h->h_lock);
 		if (likely(h->h_in != 0)) {
 			refcount_inc(&h->h_ref);
 			CDEBUG(D_INFO, "GET %s %p refcount=%d\n",
-			       h->h_ops->hop_type, h,
+			       h->h_owner, h,
 			       refcount_read(&h->h_ref));
 			retval = h;
 		}
@@ -201,8 +200,8 @@ static int cleanup_all_handles(void)
 
 		spin_lock(&handle_hash[i].lock);
 		list_for_each_entry_rcu(h, &handle_hash[i].head, h_link) {
-			CERROR("force clean handle %#llx addr %p ops %p\n",
-			       h->h_cookie, h, h->h_ops);
+			CERROR("force clean handle %#llx addr %p owner %p\n",
+			       h->h_cookie, h, h->h_owner);
 
 			class_handle_unhash_nolock(h);
 			rc++;

[lustre-devel] [PATCH 15/28] lustre: portals_handle: remove locking from class_handle2object()

[NeilBrown]

There can be no value in this locking and test on h_in.

If the lookup could have run in parallel with
class_handle_unhash_nolock() and seen "h_in == 0", then it could
equally well have run moments earlier and not seen it - no locking
would prevent that, so the caller much be prepared to have
an object returned which has already been unhashed by the time it
sees the object.

In other words, an interlock between unhash and lookup much be
provided at a higher level than where this code is trying
to handle it.

So remove this pointless code.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/lustre/obdclass/lustre_handles.c        |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index f41558ccdfcf..45e5eac47292 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -149,15 +149,11 @@ void *class_handle2object(u64 cookie, char *owner)
 		if (h->h_cookie != cookie || h->h_owner != owner)
 			continue;
 
-		spin_lock(&h->h_lock);
-		if (likely(h->h_in != 0)) {
-			refcount_inc(&h->h_ref);
-			CDEBUG(D_INFO, "GET %s %p refcount=%d\n",
-			       h->h_owner, h,
-			       refcount_read(&h->h_ref));
-			retval = h;
-		}
-		spin_unlock(&h->h_lock);
+		refcount_inc(&h->h_ref);
+		CDEBUG(D_INFO, "GET %s %p refcount=%d\n",
+		       h->h_owner, h,
+		       refcount_read(&h->h_ref));
+		retval = h;
 		break;
 	}
 	rcu_read_unlock();

[lustre-devel] [PATCH 16/28] lustre: portals_handle: use hlist for hash lists.

[NeilBrown]

hlist_head/hlist_node is the preferred data structure
for hash tables. Not only does it make the 'head' smaller,
but is also provides hlist_unhashed() which can be used to
check if an object is in the list.  This means that
we don't need h_in any more.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/include/lustre_handles.h |    3 +--
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    2 +-
 drivers/staging/lustre/lustre/obdclass/genops.c    |    4 ++--
 .../lustre/lustre/obdclass/lustre_handles.c        |   20 +++++++++-----------
 4 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
index ebbbb01710e7..cc433f48d367 100644
--- a/drivers/staging/lustre/lustre/include/lustre_handles.h
+++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
@@ -58,7 +58,7 @@
  * to compute the start of the structure based on the handle field.
  */
 struct portals_handle {
-	struct list_head		h_link;
+	struct hlist_node		h_link;
 	u64				h_cookie;
 	char				*h_owner;
 	refcount_t			h_ref;
@@ -66,7 +66,6 @@ struct portals_handle {
 	/* newly added fields to handle the RCU issue. -jxiong */
 	struct rcu_head			h_rcu;
 	spinlock_t			h_lock;
-	unsigned int			h_in:1;
 };
 
 /* handles.c */
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 56a2d1dcd663..5ac77238e5f2 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -402,7 +402,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
 
 	lprocfs_counter_incr(ldlm_res_to_ns(resource)->ns_stats,
 			     LDLM_NSS_LOCKS);
-	INIT_LIST_HEAD(&lock->l_handle.h_link);
+	INIT_HLIST_NODE(&lock->l_handle.h_link);
 	class_handle_hash(&lock->l_handle, lock_handle_owner);
 
 	lu_ref_init(&lock->l_reference);
diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
index e0da46e7d355..562e8a9f35c9 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -858,7 +858,7 @@ static struct obd_export *__class_new_export(struct obd_device *obd,
 	spin_lock_init(&export->exp_uncommitted_replies_lock);
 	INIT_LIST_HEAD(&export->exp_uncommitted_replies);
 	INIT_LIST_HEAD(&export->exp_req_replay_queue);
-	INIT_LIST_HEAD(&export->exp_handle.h_link);
+	INIT_HLIST_NODE(&export->exp_handle.h_link);
 	INIT_LIST_HEAD(&export->exp_hp_rpcs);
 	class_handle_hash(&export->exp_handle, export_handle_owner);
 	spin_lock_init(&export->exp_lock);
@@ -1046,7 +1046,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
 	atomic_set(&imp->imp_replay_inflight, 0);
 	atomic_set(&imp->imp_inval_count, 0);
 	INIT_LIST_HEAD(&imp->imp_conn_list);
-	INIT_LIST_HEAD(&imp->imp_handle.h_link);
+	INIT_HLIST_NODE(&imp->imp_handle.h_link);
 	class_handle_hash(&imp->imp_handle, import_handle_owner);
 	init_imp_at(&imp->imp_at);
 
diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index 45e5eac47292..72de668c879b 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -48,7 +48,7 @@ static spinlock_t handle_base_lock;
 
 static struct handle_bucket {
 	spinlock_t		lock;
-	struct list_head	head;
+	struct hlist_head	head;
 } *handle_hash;
 
 #define HANDLE_HASH_SIZE (1 << 16)
@@ -63,7 +63,7 @@ void class_handle_hash(struct portals_handle *h, char *owner)
 	struct handle_bucket *bucket;
 
 	LASSERT(h);
-	LASSERT(list_empty(&h->h_link));
+	LASSERT(hlist_unhashed(&h->h_link));
 
 	/*
 	 * This is fast, but simplistic cookie generation algorithm, it will
@@ -89,8 +89,7 @@ void class_handle_hash(struct portals_handle *h, char *owner)
 
 	bucket = &handle_hash[h->h_cookie & HANDLE_HASH_MASK];
 	spin_lock(&bucket->lock);
-	list_add_rcu(&h->h_link, &bucket->head);
-	h->h_in = 1;
+	hlist_add_head_rcu(&h->h_link, &bucket->head);
 	spin_unlock(&bucket->lock);
 
 	CDEBUG(D_INFO, "added object %p with handle %#llx to hash\n",
@@ -100,7 +99,7 @@ EXPORT_SYMBOL(class_handle_hash);
 
 static void class_handle_unhash_nolock(struct portals_handle *h)
 {
-	if (list_empty(&h->h_link)) {
+	if (hlist_unhashed(&h->h_link)) {
 		CERROR("removing an already-removed handle (%#llx)\n",
 		       h->h_cookie);
 		return;
@@ -110,13 +109,12 @@ static void class_handle_unhash_nolock(struct portals_handle *h)
 	       h, h->h_cookie);
 
 	spin_lock(&h->h_lock);
-	if (h->h_in == 0) {
+	if (hlist_unhashed(&h->h_link)) {
 		spin_unlock(&h->h_lock);
 		return;
 	}
-	h->h_in = 0;
+	hlist_del_init_rcu(&h->h_link);
 	spin_unlock(&h->h_lock);
-	list_del_rcu(&h->h_link);
 }
 
 void class_handle_unhash(struct portals_handle *h)
@@ -145,7 +143,7 @@ void *class_handle2object(u64 cookie, char *owner)
 	bucket = handle_hash + (cookie & HANDLE_HASH_MASK);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(h, &bucket->head, h_link) {
+	hlist_for_each_entry_rcu(h, &bucket->head, h_link) {
 		if (h->h_cookie != cookie || h->h_owner != owner)
 			continue;
 
@@ -176,7 +174,7 @@ int class_handle_init(void)
 	spin_lock_init(&handle_base_lock);
 	for (bucket = handle_hash + HANDLE_HASH_SIZE - 1; bucket >= handle_hash;
 	     bucket--) {
-		INIT_LIST_HEAD(&bucket->head);
+		INIT_HLIST_HEAD(&bucket->head);
 		spin_lock_init(&bucket->lock);
 	}
 
@@ -195,7 +193,7 @@ static int cleanup_all_handles(void)
 		struct portals_handle *h;
 
 		spin_lock(&handle_hash[i].lock);
-		list_for_each_entry_rcu(h, &handle_hash[i].head, h_link) {
+		hlist_for_each_entry_rcu(h, &handle_hash[i].head, h_link) {
 			CERROR("force clean handle %#llx addr %p owner %p\n",
 			       h->h_cookie, h, h->h_owner);
 

[lustre-devel] [PATCH 17/28] lustre: portals_handle: discard h_lock.

[NeilBrown]

The h_lock spinlock is now only taken while bucket->lock
is held.  As a handle is associated with precisely one bucket,
this means that h_lock can never be contended, so it isn't needed.

So discard h_lock.

Also discard and increasingly irrelevant comment in the declaration
of struct portals_handle.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/include/lustre_handles.h |    3 ---
 .../lustre/lustre/obdclass/lustre_handles.c        |    7 -------
 2 files changed, 10 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
index cc433f48d367..ef3a6ec55403 100644
--- a/drivers/staging/lustre/lustre/include/lustre_handles.h
+++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
@@ -62,10 +62,7 @@ struct portals_handle {
 	u64				h_cookie;
 	char				*h_owner;
 	refcount_t			h_ref;
-
-	/* newly added fields to handle the RCU issue. -jxiong */
 	struct rcu_head			h_rcu;
-	spinlock_t			h_lock;
 };
 
 /* handles.c */
diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index 72de668c879b..c6e0ca3ddf3a 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -85,7 +85,6 @@ void class_handle_hash(struct portals_handle *h, char *owner)
 	spin_unlock(&handle_base_lock);
 
 	h->h_owner = owner;
-	spin_lock_init(&h->h_lock);
 
 	bucket = &handle_hash[h->h_cookie & HANDLE_HASH_MASK];
 	spin_lock(&bucket->lock);
@@ -108,13 +107,7 @@ static void class_handle_unhash_nolock(struct portals_handle *h)
 	CDEBUG(D_INFO, "removing object %p with handle %#llx from hash\n",
 	       h, h->h_cookie);
 
-	spin_lock(&h->h_lock);
-	if (hlist_unhashed(&h->h_link)) {
-		spin_unlock(&h->h_lock);
-		return;
-	}
 	hlist_del_init_rcu(&h->h_link);
-	spin_unlock(&h->h_lock);
 }
 
 void class_handle_unhash(struct portals_handle *h)

[lustre-devel] [PATCH 18/28] lustre: remove unused fields from struct obd_device

[NeilBrown]

One field is set but never access.
Another is accessed but never set.
Other are never mentioned at all.

Also remove the comment
	/* use separate field as it is set in interrupt to don't mess with
	 * protection of other bits using _bh lock
	 */

This is not correct - were it used, obd_recovery_expired would be part
of the same 'unsigned long' as the other fields.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/obd.h        |   33 +++++++-------------
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |    4 +-
 drivers/staging/lustre/lustre/obdclass/class_obd.c |    1 -
 .../staging/lustre/lustre/obdclass/obd_config.c    |    1 -
 4 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index 3bdde312e4a6..93a47cf0ef68 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -539,27 +539,18 @@ struct obd_device {
 	char			 obd_name[MAX_OBD_NAME];
 
 	/* bitfield modification is protected by obd_dev_lock */
-	unsigned long obd_attached:1,      /* finished attach */
-		      obd_set_up:1,	/* finished setup */
-		      obd_version_recov:1, /* obd uses version checking */
-		      obd_replayable:1,/* recovery is enabled; inform clients */
-		      obd_no_transno:1,  /* no committed-transno notification */
-		      obd_no_recov:1,      /* fail instead of retry messages */
-		      obd_stopping:1,      /* started cleanup */
-		      obd_starting:1,      /* started setup */
-		      obd_force:1,	 /* cleanup with > 0 obd refcount */
-		      obd_fail:1,	 /* cleanup with failover */
-		      obd_no_conn:1,       /* deny new connections */
-		      obd_inactive:1,      /* device active/inactive
-					    * (for sysfs status only!!)
-					    */
-		      obd_no_ir:1,	 /* no imperative recovery. */
-		      obd_process_conf:1,  /* device is processing mgs config */
-		      obd_checksum_dump:1; /* dump pages upon cksum error */
-	/* use separate field as it is set in interrupt to don't mess with
-	 * protection of other bits using _bh lock
-	 */
-	unsigned long obd_recovery_expired:1;
+	unsigned long obd_attached:1,	  /* finished attach */
+		      obd_set_up:1,	  /* finished setup */
+		      obd_no_recov:1,	  /* fail instead of retry messages */
+		      obd_stopping:1,	  /* started cleanup */
+		      obd_starting:1,	  /* started setup */
+		      obd_force:1,	  /* cleanup with > 0 obd refcount */
+		      obd_fail:1,	  /* cleanup with failover */
+		      obd_inactive:1,	  /* device active/inactive
+					   * (for sysfs status only!!)
+					   */
+		      obd_process_conf:1;/* device is processing mgs config */
+
 	/* uuid-export hash body */
 	struct rhashtable	 obd_uuid_hash;
 	wait_queue_head_t	 obd_refcount_waitq;
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 5ce49e708287..6daadf24b894 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -991,10 +991,10 @@ static int mgc_set_info_async(const struct lu_env *env, struct obd_export *exp,
 		if (vallen != sizeof(int))
 			return -EINVAL;
 		value = *(int *)val;
-		CDEBUG(D_MGC, "InitRecov %s %d/d%d:i%d:r%d:or%d:%s\n",
+		CDEBUG(D_MGC, "InitRecov %s %d/d%d:i%d:r%d:%s\n",
 		       imp->imp_obd->obd_name, value,
 		       imp->imp_deactive, imp->imp_invalid,
-		       imp->imp_replayable, imp->imp_obd->obd_replayable,
+		       imp->imp_replayable,
 		       ptlrpc_import_state_name(imp->imp_state));
 		/* Resurrect if we previously died */
 		if ((imp->imp_state != LUSTRE_IMP_FULL &&
diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
index b8fc74044fe3..1fcbda128a58 100644
--- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
+++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
@@ -525,7 +525,6 @@ int class_handle_ioctl(unsigned int cmd, unsigned long arg)
 		}
 		CDEBUG(D_HA, "%s: disabling committed-transno notification\n",
 		       obd->obd_name);
-		obd->obd_no_transno = 1;
 		err = 0;
 		break;
 
diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
index 0cdadea4e63c..7b10206d6e52 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
@@ -487,7 +487,6 @@ static int class_cleanup(struct obd_device *obd, struct lustre_cfg *lcfg)
 				LCONSOLE_WARN("Failing over %s\n",
 					      obd->obd_name);
 				obd->obd_fail = 1;
-				obd->obd_no_transno = 1;
 				obd->obd_no_recov = 1;
 				if (OBP(obd, iocontrol)) {
 					obd_iocontrol(OBD_IOC_SYNC,

[lustre-devel] [PATCH 19/28] lustre: obd_sysfs: error-check value stored in jobid_var

[NeilBrown]

The jobid_var sysfs attribute only has 3 meaningful values.
Other values cause lustre_get_jobid() to return an error
which is uniformly ignored.

To improve usability and resilience, check that the value
written is acceptable before storing it.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/obdclass/obd_sysfs.c |   21 ++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/obd_sysfs.c b/drivers/staging/lustre/lustre/obdclass/obd_sysfs.c
index e9e0bcf39d4e..1431ea02f144 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_sysfs.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_sysfs.c
@@ -216,16 +216,25 @@ static ssize_t jobid_var_store(struct kobject *kobj, struct attribute *attr,
 			       const char *buffer,
 			       size_t count)
 {
+	static char *valid[] = {
+		JOBSTATS_DISABLE,
+		JOBSTATS_PROCNAME_UID,
+		JOBSTATS_NODELOCAL,
+		NULL
+	};
+	int i;
+
 	if (!count || count > JOBSTATS_JOBID_VAR_MAX_LEN)
 		return -EINVAL;
 
-	memset(obd_jobid_var, 0, JOBSTATS_JOBID_VAR_MAX_LEN + 1);
-
-	memcpy(obd_jobid_var, buffer, count);
+	for (i = 0; valid[i]; i++)
+		if (sysfs_streq(buffer, valid[i]))
+			break;
+	if (!valid[i])
+		return -EINVAL;
 
-	/* Trim the trailing '\n' if any */
-	if (obd_jobid_var[count - 1] == '\n')
-		obd_jobid_var[count - 1] = 0;
+	memset(obd_jobid_var, 0, JOBSTATS_JOBID_VAR_MAX_LEN + 1);
+	strcpy(obd_jobid_var, valid[i]);
 
 	return count;
 }

[lustre-devel] [PATCH 20/28] lustre: lov: use GFP_NOFS to allocate lo_entries.

[NeilBrown]

lo_type_guard is taken during memory reclaim:

               -> #0 (&lov->lo_type_guard){++++}:
[  576.552501]        down_read+0x27/0x7c
[  576.553072]        lov_object_delete+0xd5/0x1bb
[  576.553836]        lu_object_free+0x5f/0x111
[  576.554524]        lu_object_put+0x313/0x337
[  576.555267]        cl_inode_fini+0x158/0x1ac
[  576.555898]        ll_delete_inode+0xcc/0xd5
[  576.556572]        evict+0xb4/0x166
[  576.557162]        dispose_list+0x30/0x34
[  576.557834]        prune_icache_sb+0x55/0x73
[  576.558465]        super_cache_scan+0x122/0x16d
[  576.559138]        shrink_slab.part.22.constprop.37+0x27b/0x414
[  576.560059]        shrink_node+0x8d/0x1b3

and lov_init_composite is called while lo_type_guard is held:

[  576.540707]        lov_init_composite+0xff/0xc5f
[  576.541496]        lov_conf_set+0x4cf/0x658
[  576.542179]        cl_conf_set+0x49/0x58
[  576.542790]        cl_file_inode_init+0x21a/0x2b8
[  576.543564]        ll_update_inode+0x4e/0xea5
[  576.544277]        ll_iget+0x112/0x1bf
[  576.544871]        ll_prep_inode+0x242/0x43d
[  576.545541]        ll_lookup_it_finish+0xcf/0x51d
[  576.546308]        ll_lookup_it+0x52e/0x5fc
[  576.546976]        ll_atomic_open+0x1ce/0x74f
[  576.547666]        lookup_open+0x298/0x526
[  576.548325]        path_openat+0x29a/0x7d3
[  576.548917]        do_filp_open+0x57/0xc1
[  576.549539]        do_sys_open+0x137/0x1e7

so it is not safe to use GFP_KERNEL in lov_init_composite().

Use GFP_NOFS instead.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/lov/lov_object.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lov/lov_object.c b/drivers/staging/lustre/lustre/lov/lov_object.c
index 2058275d1cdc..eee89ce97703 100644
--- a/drivers/staging/lustre/lustre/lov/lov_object.c
+++ b/drivers/staging/lustre/lustre/lov/lov_object.c
@@ -302,7 +302,7 @@ static int lov_init_composite(const struct lu_env *env, struct lov_device *dev,
 	comp->lo_entry_count = entry_count;
 
 	comp->lo_entries = kcalloc(entry_count, sizeof(*comp->lo_entries),
-				   GFP_KERNEL);
+				   GFP_NOFS);
 	if (!comp->lo_entries)
 		return -ENOMEM;
 

[lustre-devel] [PATCH 21/28] lustre: vvp_dev; increment *pos in .next

[NeilBrown]

As described in

Commit 78bb03e221c4 ("lustre: llite: change how "dump_page_cache" walks a hash table")

The .next function should increment *pos.  For some reason it didn't,
and this can trigger the warning in that function.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/llite/vvp_dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/vvp_dev.c b/drivers/staging/lustre/lustre/llite/vvp_dev.c
index c10ca6e7c606..568943dcb3b4 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_dev.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_dev.c
@@ -564,7 +564,7 @@ static void *vvp_pgcache_next(struct seq_file *f, void *v, loff_t *pos)
 	struct vvp_seq_private *priv = f->private;
 
 	WARN_ON(*pos != priv->vsp_prev_pos);
-
+	*pos += 1;
 	priv->vsp_prev_pos = *pos;
 	return vvp_pgcache_next_page(priv);
 }

[lustre-devel] [PATCH 22/28] lustre: obdclass: discard process_quota_config

[NeilBrown]

This variable is never set, so it can be discarded.

If there is, later, a need for config messages to be handled by
different modules, it would be best to use a notifier chain.
This ensures race-free registration and deregistration.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lustre/obdclass/obd_config.c    |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
index 7b10206d6e52..ccc0748f63a6 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
@@ -761,11 +761,6 @@ void class_del_profiles(void)
 }
 EXPORT_SYMBOL(class_del_profiles);
 
-/* We can't call lquota_process_config directly because
- * it lives in a module that must be loaded after this one.
- */
-static int (*quota_process_config)(struct lustre_cfg *lcfg);
-
 static int process_param2_config(struct lustre_cfg *lcfg)
 {
 	char *param = lustre_cfg_string(lcfg, 1);
@@ -941,11 +936,6 @@ int class_process_config(struct lustre_cfg *lcfg)
 
 			err = 0;
 			goto out;
-		} else if ((class_match_param(lustre_cfg_string(lcfg, 1),
-					      PARAM_QUOTA, &tmp) == 0) &&
-			   quota_process_config) {
-			err = (*quota_process_config)(lcfg);
-			goto out;
 		}
 
 		break;

[lustre-devel] [PATCH 23/28] lustre: obdclass: remove unnecessary code from lustre_init_lsi()

[NeilBrown]

After allocating a struct with kzalloc, there is no value
in setting a few of the fields to zero.

And as all fields were zero, it must be safe to kfree lmd_exclude,
whether lmd_exclude_count is zero or not.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/obdclass/obd_mount.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
index 104e64b5889d..4c86ca5a5f23 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
@@ -517,9 +517,6 @@ struct lustre_sb_info *lustre_init_lsi(struct super_block *sb)
 		return NULL;
 	}
 
-	lsi->lsi_lmd->lmd_exclude_count = 0;
-	lsi->lsi_lmd->lmd_recovery_time_soft = 0;
-	lsi->lsi_lmd->lmd_recovery_time_hard = 0;
 	s2lsi_nocast(sb) = lsi;
 	/* we take 1 extra ref for our setup */
 	atomic_set(&lsi->lsi_mounts, 1);
@@ -546,8 +543,7 @@ static int lustre_free_lsi(struct super_block *sb)
 		kfree(lsi->lsi_lmd->lmd_fileset);
 		kfree(lsi->lsi_lmd->lmd_mgssec);
 		kfree(lsi->lsi_lmd->lmd_opts);
-		if (lsi->lsi_lmd->lmd_exclude_count)
-			kfree(lsi->lsi_lmd->lmd_exclude);
+		kfree(lsi->lsi_lmd->lmd_exclude);
 		kfree(lsi->lsi_lmd->lmd_mgs);
 		kfree(lsi->lsi_lmd->lmd_osd_type);
 		kfree(lsi->lsi_lmd->lmd_params);

[lustre-devel] [PATCH 24/28] lustre: ldlm: discard l_lock from struct ldlm_lock.

[NeilBrown]

This spinlock (l_lock) is only used to stablise the l_resource
pointer while taking a spinlock on the resource.

This is not necessary - it is sufficient to take the resource
spinlock, and then check if l_resource has changed or not.  If it
hasn't then it cannot change until the resource spinlock is dropped.

We must ensure this is safe even if the resource is freed before
lock_res_and_lock() managed to get the lock.  To do this we mark the
slab as SLAB_TYPESAFE_BY_RCU and initialise the lock in an
init_once() function, but not on every allocate (and specifically
don't zero the whole srtuct on each allocation).
This means that if we find a resource after taking the RCU read lock,
then it is always safe to take and then drop the spinlock.
After taking the spinlock, we can check if it is more generally safe
to use.

Discarding l_lock shrinks 'struct ldlm_lock' which helps save memory.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/include/lustre_dlm.h |    5 ----
 drivers/staging/lustre/lustre/ldlm/l_lock.c        |   21 +++++++++-------
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |   26 ++++++++++----------
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c    |   20 +++++++++++++++
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |    9 ++++---
 5 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
index fd9b0f870c93..341766123e67 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
@@ -590,11 +590,6 @@ struct ldlm_lock {
 	 * Must be first in the structure.
 	 */
 	struct portals_handle		l_handle;
-	/**
-	 * Internal spinlock protects l_resource.  We should hold this lock
-	 * first before taking res_lock.
-	 */
-	spinlock_t			l_lock;
 	/**
 	 * Pointer to actual resource this lock is in.
 	 * ldlm_lock_change_resource() can change this.
diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c b/drivers/staging/lustre/lustre/ldlm/l_lock.c
index 296259aa51e6..315eb5099bdb 100644
--- a/drivers/staging/lustre/lustre/ldlm/l_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c
@@ -45,15 +45,20 @@
  * being an atomic operation.
  */
 struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock)
-				__acquires(&lock->l_lock)
 				__acquires(&lock->l_resource->lr_lock)
 {
-	spin_lock(&lock->l_lock);
-
-	lock_res(lock->l_resource);
-
-	ldlm_set_res_locked(lock);
-	return lock->l_resource;
+	struct ldlm_resource *res;
+	rcu_read_lock();
+	while (1) {
+		res = rcu_dereference(lock->l_resource);
+		lock_res(res);
+		if (res == lock->l_resource) {
+			ldlm_set_res_locked(lock);
+			rcu_read_unlock();
+			return res;
+		}
+		unlock_res(res);
+	}
 }
 EXPORT_SYMBOL(lock_res_and_lock);
 
@@ -62,12 +67,10 @@ EXPORT_SYMBOL(lock_res_and_lock);
  */
 void unlock_res_and_lock(struct ldlm_lock *lock)
 		__releases(&lock->l_resource->lr_lock)
-		__releases(&lock->l_lock)
 {
 	/* on server-side resource of lock doesn't change */
 	ldlm_clear_res_locked(lock);
 
 	unlock_res(lock->l_resource);
-	spin_unlock(&lock->l_lock);
 }
 EXPORT_SYMBOL(unlock_res_and_lock);
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 5ac77238e5f2..e62dad14d822 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -383,7 +383,6 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
 	if (!lock)
 		return NULL;
 
-	spin_lock_init(&lock->l_lock);
 	lock->l_resource = resource;
 	lu_ref_add(&resource->lr_reference, "lock", lock);
 
@@ -452,12 +451,13 @@ int ldlm_lock_change_resource(struct ldlm_namespace *ns, struct ldlm_lock *lock,
 
 	lu_ref_add(&newres->lr_reference, "lock", lock);
 	/*
-	 * To flip the lock from the old to the new resource, lock, oldres and
-	 * newres have to be locked. Resource spin-locks are nested within
-	 * lock->l_lock, and are taken in the memory address order to avoid
-	 * dead-locks.
+	 * To flip the lock from the old to the new resource, oldres
+	 * and newres have to be locked. Resource spin-locks are taken
+	 * in the memory address order to avoid dead-locks.
+	 * As this is the only circumstance where ->l_resource
+	 * can change, and this cannot race with itself, it is safe
+	 * to access lock->l_resource without being careful about locking.
 	 */
-	spin_lock(&lock->l_lock);
 	oldres = lock->l_resource;
 	if (oldres < newres) {
 		lock_res(oldres);
@@ -468,9 +468,9 @@ int ldlm_lock_change_resource(struct ldlm_namespace *ns, struct ldlm_lock *lock,
 	}
 	LASSERT(memcmp(new_resid, &oldres->lr_name,
 		       sizeof(oldres->lr_name)) != 0);
-	lock->l_resource = newres;
+	rcu_assign_pointer(lock->l_resource, newres);
 	unlock_res(oldres);
-	unlock_res_and_lock(lock);
+	unlock_res(newres);
 
 	/* ...and the flowers are still standing! */
 	lu_ref_del(&oldres->lr_reference, "lock", lock);
@@ -1964,11 +1964,11 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 	va_list args;
 	struct va_format vaf;
 
-	if (spin_trylock(&lock->l_lock)) {
-		if (lock->l_resource)
-			resource = ldlm_resource_getref(lock->l_resource);
-		spin_unlock(&lock->l_lock);
-	}
+	rcu_read_lock();
+	resource = rcu_dereference(lock->l_resource);
+	if (resource && !atomic_inc_not_zero(&resource->lr_refcount))
+		resource = NULL;
+	rcu_read_unlock();
 
 	va_start(args, fmt);
 	vaf.fmt = fmt;
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
index 589b89d5885a..a9ca9a5e49be 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
@@ -1097,6 +1097,23 @@ static int ldlm_cleanup(void)
 	return 0;
 }
 
+void ldlm_resource_init_once(void *p)
+{
+	/*
+	 * It is import to initialise the spinlock only once,
+	 * as ldlm_lock_change_resource() could try to lock
+	 * the resource *after* it has been freed and possibly
+	 * reused. SLAB_TYPESAFE_BY_RCU ensures the memory won't
+	 * be freed while the lock is being taken, but we need to
+	 * ensure that it doesn't get reinitialized either.
+	 */
+	struct ldlm_resource *res = p;
+
+	memset(res, 0, sizeof(*res));
+	mutex_init(&res->lr_lvb_mutex);
+	spin_lock_init(&res->lr_lock);
+}
+
 int ldlm_init(void)
 {
 	mutex_init(&ldlm_ref_mutex);
@@ -1104,7 +1121,8 @@ int ldlm_init(void)
 	mutex_init(ldlm_namespace_lock(LDLM_NAMESPACE_CLIENT));
 	ldlm_resource_slab = kmem_cache_create("ldlm_resources",
 					       sizeof(struct ldlm_resource), 0,
-					       SLAB_HWCACHE_ALIGN, NULL);
+					       SLAB_TYPESAFE_BY_RCU |
+					       SLAB_HWCACHE_ALIGN, ldlm_resource_init_once);
 	if (!ldlm_resource_slab)
 		return -ENOMEM;
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
index 45b2e97f7e1c..d79f70d17220 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
@@ -994,12 +994,14 @@ static struct ldlm_resource *ldlm_resource_new(enum ldlm_type ldlm_type)
 {
 	struct ldlm_resource *res;
 
-	res = kmem_cache_zalloc(ldlm_resource_slab, GFP_NOFS);
+	res = kmem_cache_alloc(ldlm_resource_slab, GFP_NOFS);
 	if (!res)
 		return NULL;
 
 	INIT_LIST_HEAD(&res->lr_granted);
 	INIT_LIST_HEAD(&res->lr_waiting);
+	res->lr_lvb_inode = NULL;
+	res->lr_lvb_len = 0;
 
 	if (ldlm_type == LDLM_EXTENT) {
 		int idx;
@@ -1017,16 +1019,15 @@ static struct ldlm_resource *ldlm_resource_new(enum ldlm_type ldlm_type)
 			res->lr_itree[idx].lit_mode = 1 << idx;
 			res->lr_itree[idx].lit_root = RB_ROOT_CACHED;
 		}
-	}
+	} else
+		res->lr_itree = NULL;
 
 	atomic_set(&res->lr_refcount, 1);
-	spin_lock_init(&res->lr_lock);
 	lu_ref_init(&res->lr_reference);
 
 	/* The creator of the resource must unlock the mutex after LVB
 	 * initialization.
 	 */
-	mutex_init(&res->lr_lvb_mutex);
 	mutex_lock(&res->lr_lvb_mutex);
 
 	return res;

[lustre-devel] [PATCH 25/28] lustre: ldlm: don't access l_resource when not locked.

[NeilBrown]

lock->l_resource can (sometimes) change when the resource
isn't locked.
So dereferencing lock->l_resource and the locking the
resource looks wrong.
As lock_res_and_lock() returns the locked resource, this
code can easily be more obviously correct by using
that return value.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index e62dad14d822..830d089774a3 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -425,13 +425,13 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
 int ldlm_lock_change_resource(struct ldlm_namespace *ns, struct ldlm_lock *lock,
 			      const struct ldlm_res_id *new_resid)
 {
-	struct ldlm_resource *oldres = lock->l_resource;
+	struct ldlm_resource *oldres;
 	struct ldlm_resource *newres;
 	int type;
 
-	lock_res_and_lock(lock);
-	if (memcmp(new_resid, &lock->l_resource->lr_name,
-		   sizeof(lock->l_resource->lr_name)) == 0) {
+	oldres = lock_res_and_lock(lock);
+	if (memcmp(new_resid, &oldres->lr_name,
+		   sizeof(oldres->lr_name)) == 0) {
 		/* Nothing to do */
 		unlock_res_and_lock(lock);
 		return 0;
@@ -1573,9 +1573,9 @@ enum ldlm_error ldlm_lock_enqueue(struct ldlm_namespace *ns,
 				  void *cookie, u64 *flags)
 {
 	struct ldlm_lock *lock = *lockp;
-	struct ldlm_resource *res = lock->l_resource;
+	struct ldlm_resource *res;
 
-	lock_res_and_lock(lock);
+	res = lock_res_and_lock(lock);
 	if (lock->l_req_mode == lock->l_granted_mode) {
 		/* The server returned a blocked lock, but it was granted
 		 * before we got a chance to actually enqueue it.  We don't
@@ -1874,9 +1874,8 @@ void ldlm_lock_cancel(struct ldlm_lock *lock)
 	struct ldlm_resource *res;
 	struct ldlm_namespace *ns;
 
-	lock_res_and_lock(lock);
+	res = lock_res_and_lock(lock);
 
-	res = lock->l_resource;
 	ns  = ldlm_res_to_ns(res);
 
 	/* Please do not, no matter how tempting, remove this LBUG without

[lustre-devel] [PATCH 26/28] lustre: ldlm: drop SLAB_TYPESAFE_BY_RCU from ldlm_lock slab.

[NeilBrown]

ldlm_locks are always freed with kfree_rcu(), so there is
no need for the underlying pages to also be freed after
a grace period.  So remove this flag, it is not useful.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
index a9ca9a5e49be..bf2c3531b39b 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
@@ -1128,8 +1128,7 @@ int ldlm_init(void)
 
 	ldlm_lock_slab = kmem_cache_create("ldlm_locks",
 					   sizeof(struct ldlm_lock), 0,
-					   SLAB_HWCACHE_ALIGN |
-					   SLAB_TYPESAFE_BY_RCU, NULL);
+					   SLAB_HWCACHE_ALIGN, NULL);
 	if (!ldlm_lock_slab)
 		goto out_resource;
 

[lustre-devel] [PATCH 27/28] lustre: ldlm: simplify lock_mode_to_index()

[NeilBrown]

This function has the same effect as ilog2(), so just use ilog2
directly.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_extent.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
index 99aef0bee53c..b3d1a2565dbc 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
@@ -104,8 +104,7 @@ static inline int lock_mode_to_index(enum ldlm_mode mode)
 
 	LASSERT(mode != 0);
 	LASSERT(is_power_of_2(mode));
-	for (index = -1; mode; index++)
-		mode >>= 1;
+	index = ilog2(mode);
 	LASSERT(index < LCK_MODE_NUM);
 	return index;
 }

[lustre-devel] [PATCH 28/28] lustre: use list_move where appropriate.

[NeilBrown]

There are several places in lustre where "list_del" (or occasionally
"list_del_init") is followed by "list_add" or "list_add_tail" which
moves the object to a different list.
These can be combined into "list_move" or "list_move_tail".

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |   10 ++++------
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |    6 ++----
 .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    3 +--
 .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |    3 +--
 .../lustre/lnet/klnds/socklnd/socklnd_proto.c      |    3 +--
 drivers/staging/lustre/lnet/lnet/config.c          |    3 +--
 drivers/staging/lustre/lnet/lnet/lib-move.c        |   16 ++++++----------
 drivers/staging/lustre/lnet/selftest/console.c     |    7 ++-----
 drivers/staging/lustre/lustre/ldlm/ldlm_lib.c      |    5 ++---
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  |    3 +--
 drivers/staging/lustre/lustre/ptlrpc/client.c      |   10 ++++------
 drivers/staging/lustre/lustre/ptlrpc/import.c      |    3 +--
 drivers/staging/lustre/lustre/ptlrpc/service.c     |   13 ++++---------
 13 files changed, 30 insertions(+), 55 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index d67a197e718d..94388b406149 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1555,11 +1555,10 @@ static void kiblnd_fail_fmr_poolset(struct kib_fmr_poolset *fps,
 					       struct kib_fmr_pool,
 					       fpo_list)) != NULL) {
 		fpo->fpo_failed = 1;
-		list_del(&fpo->fpo_list);
 		if (!fpo->fpo_map_count)
-			list_add(&fpo->fpo_list, zombies);
+			list_move(&fpo->fpo_list, zombies);
 		else
-			list_add(&fpo->fpo_list, &fps->fps_failed_pool_list);
+			list_move(&fpo->fpo_list, &fps->fps_failed_pool_list);
 	}
 
 	spin_unlock(&fps->fps_lock);
@@ -1877,11 +1876,10 @@ static void kiblnd_fail_poolset(struct kib_poolset *ps, struct list_head *zombie
 					      struct kib_pool,
 					      po_list)) == NULL) {
 		po->po_failed = 1;
-		list_del(&po->po_list);
 		if (!po->po_allocated)
-			list_add(&po->po_list, zombies);
+			list_move(&po->po_list, zombies);
 		else
-			list_add(&po->po_list, &ps->ps_failed_pool_list);
+			list_move(&po->po_list, &ps->ps_failed_pool_list);
 	}
 	spin_unlock(&ps->ps_lock);
 }
diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index b9585f607463..3579d90df98d 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -961,8 +961,7 @@ kiblnd_check_sends_locked(struct kib_conn *conn)
 	       (tx = list_first_entry_or_null(
 		       &conn->ibc_tx_queue_rsrvd,
 		       struct kib_tx, tx_list)) != NULL) {
-		list_del(&tx->tx_list);
-		list_add_tail(&tx->tx_list, &conn->ibc_tx_queue);
+		list_move_tail(&tx->tx_list, &conn->ibc_tx_queue);
 		conn->ibc_reserved_credits--;
 	}
 
@@ -2051,8 +2050,7 @@ kiblnd_abort_txs(struct kib_conn *conn, struct list_head *txs)
 
 		if (!tx->tx_sending) {
 			tx->tx_queued = 0;
-			list_del(&tx->tx_list);
-			list_add(&tx->tx_list, &zombies);
+			list_move(&tx->tx_list, &zombies);
 		}
 	}
 
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
index 08feaf7ce33a..922ba76f5913 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
@@ -1560,8 +1560,7 @@ ksocknal_finalize_zcreq(struct ksock_conn *conn)
 
 		tx->tx_msg.ksm_zc_cookies[0] = 0;
 		tx->tx_zc_aborted = 1; /* mark it as not-acked */
-		list_del(&tx->tx_zc_list);
-		list_add(&tx->tx_zc_list, &zlist);
+		list_move(&tx->tx_zc_list, &zlist);
 	}
 
 	spin_unlock(&peer_ni->ksnp_lock);
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index 208b8d360d5c..cbba90c6b9da 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -2255,8 +2255,7 @@ ksocknal_flush_stale_txs(struct ksock_peer *peer_ni)
 		if (ktime_get_seconds() < tx->tx_deadline)
 			break;
 
-		list_del(&tx->tx_list);
-		list_add_tail(&tx->tx_list, &stale_txs);
+		list_move_tail(&tx->tx_list, &stale_txs);
 	}
 
 	write_unlock_bh(&ksocknal_data.ksnd_global_lock);
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
index e8b95affee96..fca63763c260 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_proto.c
@@ -437,8 +437,7 @@ ksocknal_handle_zcack(struct ksock_conn *conn, u64 cookie1, u64 cookie2)
 		if (c == cookie1 || c == cookie2 ||
 		    (cookie1 < c && c < cookie2)) {
 			tx->tx_msg.ksm_zc_cookies[0] = 0;
-			list_del(&tx->tx_zc_list);
-			list_add(&tx->tx_zc_list, &zlist);
+			list_move(&tx->tx_zc_list, &zlist);
 
 			if (!--count)
 				break;
diff --git a/drivers/staging/lustre/lnet/lnet/config.c b/drivers/staging/lustre/lnet/lnet/config.c
index f34844465d01..1ec4afd1800c 100644
--- a/drivers/staging/lustre/lnet/lnet/config.c
+++ b/drivers/staging/lustre/lnet/lnet/config.c
@@ -1530,8 +1530,7 @@ lnet_match_networks(char **networksp, char *ip2nets, u32 *ipaddrs, int nip)
 		list_for_each_safe(t, t2, &current_nets) {
 			tb = list_entry(t, struct lnet_text_buf, ltb_list);
 
-			list_del(&tb->ltb_list);
-			list_add_tail(&tb->ltb_list, &matched_nets);
+			list_move_tail(&tb->ltb_list, &matched_nets);
 
 			len += snprintf(networks + len, sizeof(networks) - len,
 					"%s%s", !len ? "" : ",",
diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c
index 875d2898154f..17f1c4a1029c 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-move.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-move.c
@@ -175,12 +175,10 @@ lnet_fail_nid(lnet_nid_t nid, unsigned int threshold)
 	list_for_each_safe(el, next, &the_lnet.ln_test_peers) {
 		tp = list_entry(el, struct lnet_test_peer, tp_list);
 
-		if (!tp->tp_threshold ||    /* needs culling anyway */
-		    nid == LNET_NID_ANY ||       /* removing all entries */
-		    tp->tp_nid == nid) {	  /* matched this one */
-			list_del(&tp->tp_list);
-			list_add(&tp->tp_list, &cull);
-		}
+		if (!tp->tp_threshold ||	/* needs culling anyway */
+		    nid == LNET_NID_ANY ||	/* removing all entries */
+		    tp->tp_nid == nid)		/* matched this one */
+			list_move(&tp->tp_list, &cull);
 	}
 
 	lnet_net_unlock(0);
@@ -219,8 +217,7 @@ fail_peer(lnet_nid_t nid, int outgoing)
 				 * since we may be at interrupt priority on
 				 * incoming messages.
 				 */
-				list_del(&tp->tp_list);
-				list_add(&tp->tp_list, &cull);
+				list_move(&tp->tp_list, &cull);
 			}
 			continue;
 		}
@@ -234,8 +231,7 @@ fail_peer(lnet_nid_t nid, int outgoing)
 				if (outgoing &&
 				    !tp->tp_threshold) {
 					/* see above */
-					list_del(&tp->tp_list);
-					list_add(&tp->tp_list, &cull);
+					list_move(&tp->tp_list, &cull);
 				}
 			}
 			break;
diff --git a/drivers/staging/lustre/lnet/selftest/console.c b/drivers/staging/lustre/lnet/selftest/console.c
index abc342c6a842..dfd2b94acf90 100644
--- a/drivers/staging/lustre/lnet/selftest/console.c
+++ b/drivers/staging/lustre/lnet/selftest/console.c
@@ -316,12 +316,9 @@ lstcon_group_ndlink_move(struct lstcon_group *old,
 	unsigned int idx = LNET_NIDADDR(ndl->ndl_node->nd_id.nid) %
 					LST_NODE_HASHSIZE;
 
-	list_del(&ndl->ndl_hlink);
-	list_del(&ndl->ndl_link);
 	old->grp_nnode--;
-
-	list_add_tail(&ndl->ndl_hlink, &new->grp_ndl_hash[idx]);
-	list_add_tail(&ndl->ndl_link, &new->grp_ndl_list);
+	list_move_tail(&ndl->ndl_hlink, &new->grp_ndl_hash[idx]);
+	list_move_tail(&ndl->ndl_link, &new->grp_ndl_list);
 	new->grp_nnode++;
 }
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c
index 9c61b332a51c..2274e0cc0cdf 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c
@@ -86,9 +86,8 @@ static int import_set_conn(struct obd_import *imp, struct obd_uuid *uuid,
 	list_for_each_entry(item, &imp->imp_conn_list, oic_item) {
 		if (obd_uuid_equals(uuid, &item->oic_uuid)) {
 			if (priority) {
-				list_del(&item->oic_item);
-				list_add(&item->oic_item,
-					 &imp->imp_conn_list);
+				list_move(&item->oic_item,
+					  &imp->imp_conn_list);
 				item->oic_last_attempt = 0;
 			}
 			CDEBUG(D_HA, "imp %p@%s: found existing conn %s%s\n",
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
index a614d7419b8f..fbb12f540dbd 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
@@ -1101,8 +1101,7 @@ int ldlm_cli_cancel_list_local(struct list_head *cancels, int count,
 		 */
 		if (!(flags & LCF_BL_AST) && (rc == LDLM_FL_BL_AST)) {
 			LDLM_DEBUG(lock, "Cancel lock separately");
-			list_del_init(&lock->l_bl_ast);
-			list_add(&lock->l_bl_ast, &head);
+			list_move(&lock->l_bl_ast, &head);
 			bl_ast++;
 			continue;
 		}
diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
index da1ccd8f9dcc..e24c9c5eb90d 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -1803,9 +1803,8 @@ int ptlrpc_check_set(const struct lu_env *env, struct ptlrpc_request_set *set)
 					 * put on delay list - only if we wait
 					 * recovery finished - before send
 					 */
-					list_del_init(&req->rq_list);
-					list_add_tail(&req->rq_list,
-						      &imp->imp_delayed_list);
+					list_move_tail(&req->rq_list,
+						       &imp->imp_delayed_list);
 					spin_unlock(&imp->imp_lock);
 					continue;
 				}
@@ -1826,9 +1825,8 @@ int ptlrpc_check_set(const struct lu_env *env, struct ptlrpc_request_set *set)
 					goto interpret;
 				}
 
-				list_del_init(&req->rq_list);
-				list_add_tail(&req->rq_list,
-					      &imp->imp_sending_list);
+				list_move_tail(&req->rq_list,
+					       &imp->imp_sending_list);
 
 				spin_unlock(&imp->imp_lock);
 
diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c
index 18823d52e06b..a68b870faad2 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/import.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/import.c
@@ -757,8 +757,7 @@ static int ptlrpc_connect_set_flags(struct obd_import *imp,
 	}
 
 	spin_lock(&imp->imp_lock);
-	list_del(&imp->imp_conn_current->oic_item);
-	list_add(&imp->imp_conn_current->oic_item, &imp->imp_conn_list);
+	list_move(&imp->imp_conn_current->oic_item, &imp->imp_conn_list);
 	imp->imp_last_success_conn = imp->imp_conn_current->oic_last_attempt;
 
 	spin_unlock(&imp->imp_lock);
diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
index 5e541ca0dc00..c6b95c721167 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -302,11 +302,10 @@ ptlrpc_server_post_idle_rqbds(struct ptlrpc_service_part *svcpt)
 		rqbd = list_first_entry(&svcpt->scp_rqbd_idle,
 					struct ptlrpc_request_buffer_desc,
 					rqbd_list);
-		list_del(&rqbd->rqbd_list);
 
 		/* assume we will post successfully */
 		svcpt->scp_nrqbds_posted++;
-		list_add(&rqbd->rqbd_list, &svcpt->scp_rqbd_posted);
+		list_move(&rqbd->rqbd_list, &svcpt->scp_rqbd_posted);
 
 		spin_unlock(&svcpt->scp_lock);
 
@@ -320,8 +319,7 @@ ptlrpc_server_post_idle_rqbds(struct ptlrpc_service_part *svcpt)
 	spin_lock(&svcpt->scp_lock);
 
 	svcpt->scp_nrqbds_posted--;
-	list_del(&rqbd->rqbd_list);
-	list_add_tail(&rqbd->rqbd_list, &svcpt->scp_rqbd_idle);
+	list_move_tail(&rqbd->rqbd_list, &svcpt->scp_rqbd_idle);
 
 	/* Don't complain if no request buffers are posted right now; LNET
 	 * won't drop requests because we set the portal lazy!
@@ -760,9 +758,7 @@ static void ptlrpc_server_drop_request(struct ptlrpc_request *req)
 	refcount = --(rqbd->rqbd_refcount);
 	if (refcount == 0) {
 		/* request buffer is now idle: add to history */
-		list_del(&rqbd->rqbd_list);
-
-		list_add_tail(&rqbd->rqbd_list, &svcpt->scp_hist_rqbds);
+		list_move_tail(&rqbd->rqbd_list, &svcpt->scp_hist_rqbds);
 		svcpt->scp_hist_nrqbds++;
 
 		/* cull some history?
@@ -2350,8 +2346,7 @@ static void ptlrpc_svcpt_stop_threads(struct ptlrpc_service_part *svcpt)
 						  struct ptlrpc_thread,
 						  t_link)) != NULL) {
 		if (thread_is_stopped(thread)) {
-			list_del(&thread->t_link);
-			list_add(&thread->t_link, &zombie);
+			list_move(&thread->t_link, &zombie);
 			continue;
 		}
 		spin_unlock(&svcpt->scp_lock);

[lustre-devel] [PATCH 02/28] lustre: collect all resource releasing for obj_type.

[James Simmons]

> Now that obj_type is managed as a kobject, move all
> the freeing and deregistering into class_sysfs_release().

I don't know why I didn't receive the first patch in my mailbox :-(
In any case I developed a patch to cleanup the class_setup_tunables()
use in llite. That change impacts the reset of the class kobject 
cleanup so I like to push a new set of patches. I have a few kinks
to work out yet with the special needs of the server code which can
impact how this code flows.
 
> Reviewed-by: James Simmons <jsimmons@infradead.org>
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/obdclass/genops.c |   27 +++++++++++------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> index 7b3432f43a40..8dd217c888a5 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -148,6 +148,18 @@ static void class_sysfs_release(struct kobject *kobj)
>  {
>  	struct obd_type *type = container_of(kobj, struct obd_type, typ_kobj);
>  
> +	debugfs_remove_recursive(type->typ_debugfs_entry);
> +
> +	if (type->typ_lu)
> +		lu_device_type_fini(type->typ_lu);
> +
> +	spin_lock(&obd_types_lock);
> +	list_del(&type->typ_chain);
> +	spin_unlock(&obd_types_lock);
> +
> +	kfree(type->typ_name);
> +	kfree(type->typ_md_ops);
> +	kfree(type->typ_dt_ops);
>  	kfree(type);
>  }
>  
> @@ -200,6 +212,7 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
>  
>  	type->typ_kobj.kset = lustre_kset;
>  	kobject_init(&type->typ_kobj, &class_ktype);
> +	INIT_LIST_HEAD(&type->typ_chain);
>  
>  	type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
>  	type->typ_md_ops = kzalloc(sizeof(*type->typ_md_ops), GFP_NOFS);
> @@ -239,9 +252,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
>  	return 0;
>  
>  failed:
> -	kfree(type->typ_name);
> -	kfree(type->typ_md_ops);
> -	kfree(type->typ_dt_ops);
>  	kobject_put(&type->typ_kobj);
>  
>  	return rc;
> @@ -266,17 +276,6 @@ int class_unregister_type(const char *name)
>  		return -EBUSY;
>  	}
>  
> -	debugfs_remove_recursive(type->typ_debugfs_entry);
> -
> -	if (type->typ_lu)
> -		lu_device_type_fini(type->typ_lu);
> -
> -	spin_lock(&obd_types_lock);
> -	list_del(&type->typ_chain);
> -	spin_unlock(&obd_types_lock);
> -	kfree(type->typ_name);
> -	kfree(type->typ_dt_ops);
> -	kfree(type->typ_md_ops);
>  	kobject_put(&type->typ_kobj);
>  
>  	return 0;
> 
> 
> 

[lustre-devel] [PATCH 05/28] lustre: obd_type: discard obd_type_lock

[James Simmons]

> This lock is only used to protect typ_refcnt, so change
> that to an atomic_t and discard the lock.
> 
> The lock also covers calls to try_module_get and module_put,
> but this serves no purpose as it does not prevent the module
> from being unloaded.
> 
> Finally, the return value for the call to try_module_get is
> ignored, which is not safe.

Nak. Looking at the code we can easily use the kref of the
kobject instead. The two special cases for the ref_count
can be removed. The one for mdc_request was removed in
patch https://review.whamcloud.com/30419. The other refcount
use in mgc looks like its for supporting lustre 1.4 version
logs. I bet that can be removed. Andreas can state if it 
is still need.

> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/include/obd.h        |    3 +-
>  drivers/staging/lustre/lustre/mdc/mdc_request.c    |    2 +
>  drivers/staging/lustre/lustre/mgc/mgc_request.c    |    2 +
>  drivers/staging/lustre/lustre/obdclass/genops.c    |   30 +++++++++-----------
>  drivers/staging/lustre/lustre/obdclass/lu_object.c |    2 +
>  5 files changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
> index 4c58b916e0a3..61fb8159af20 100644
> --- a/drivers/staging/lustre/lustre/include/obd.h
> +++ b/drivers/staging/lustre/lustre/include/obd.h
> @@ -102,9 +102,8 @@ struct obd_type {
>  	struct obd_ops		*typ_dt_ops;
>  	struct md_ops		*typ_md_ops;
>  	struct dentry		*typ_debugfs_entry;
> -	int			 typ_refcnt;
> +	atomic_t		 typ_refcnt;
>  	struct lu_device_type	*typ_lu;
> -	spinlock_t		 obd_type_lock;
>  	struct kobject		 typ_kobj;
>  };
>  #define typ_name typ_kobj.name
> diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
> index bc764f9dd102..705a4e3b518a 100644
> --- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
> +++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
> @@ -2542,7 +2542,7 @@ static int mdc_init_ea_size(struct obd_export *exp, u32 easize, u32 def_easize)
>  static int mdc_precleanup(struct obd_device *obd)
>  {
>  	/* Failsafe, ok if racy */
> -	if (obd->obd_type->typ_refcnt <= 1)
> +	if (atomic_read(&obd->obd_type->typ_refcnt) <= 1)
>  		libcfs_kkuc_group_rem(0, KUC_GRP_HSM);
>  
>  	mdc_changelog_cdev_finish(obd);
> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> index 84ba6d0e3493..0580afa2755d 100644
> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> @@ -715,7 +715,7 @@ static int mgc_cleanup(struct obd_device *obd)
>  	/* COMPAT_146 - old config logs may have added profiles we don't
>  	 * know about
>  	 */
> -	if (obd->obd_type->typ_refcnt <= 1)
> +	if (atomic_read(&obd->obd_type->typ_refcnt) <= 1)
>  		/* Only for the last mgc */
>  		class_del_profiles();
>  
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> index 74195de639e4..02d829617519 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -113,15 +113,17 @@ static struct obd_type *class_get_type(const char *name)
>  		}
>  	}
>  	if (type) {
> -		spin_lock(&type->obd_type_lock);
> -		type->typ_refcnt++;
> -		try_module_get(type->typ_dt_ops->owner);
> -		spin_unlock(&type->obd_type_lock);
> -		/* class_search_type() returned a counted reference,
> -		 * but we don't need that count any more as
> -		 * we have one through typ_refcnt.
> -		 */
> -		kobject_put(&type->typ_kobj);
> +		if (try_module_get(type->typ_dt_ops->owner)) {
> +			atomic_inc(&type->typ_refcnt);
> +			/* class_search_type() returned a counted reference,
> +			 * but we don't need that count any more as
> +			 * we have one through typ_refcnt.
> +			 */
> +			kobject_put(&type->typ_kobj);
> +		} else {
> +			kobject_put(&type->typ_kobj);
> +			type = NULL;
> +		}
>  	}
>  	return type;
>  }
> @@ -129,10 +131,7 @@ static struct obd_type *class_get_type(const char *name)
>  void class_put_type(struct obd_type *type)
>  {
>  	LASSERT(type);
> -	spin_lock(&type->obd_type_lock);
> -	type->typ_refcnt--;
> -	module_put(type->typ_dt_ops->owner);
> -	spin_unlock(&type->obd_type_lock);
> +	atomic_dec(&type->typ_refcnt);
>  }
>  
>  static void simple_class_release(struct kobject *kobj)
> @@ -222,7 +221,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
>  	/* md_ops is optional */
>  	if (md_ops)
>  		*type->typ_md_ops = *md_ops;
> -	spin_lock_init(&type->obd_type_lock);
>  
>  	rc = kobject_add(&type->typ_kobj, &lustre_kset->kobj, "%s", name);
>  	if (rc)
> @@ -256,8 +254,8 @@ int class_unregister_type(const char *name)
>  		return -EINVAL;
>  	}
>  
> -	if (type->typ_refcnt) {
> -		CERROR("type %s has refcount (%d)\n", name, type->typ_refcnt);
> +	if (atomic_read(&type->typ_refcnt)) {
> +		CERROR("type %s has refcount (%d)\n", name, atomic_read(&type->typ_refcnt));
>  		/* This is a bad situation, let's make the best of it */
>  		/* Remove ops, but leave the name for debugging */
>  		kfree(type->typ_dt_ops);
> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> index 9c872db21040..770cc1b9e40b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
> @@ -1267,7 +1267,7 @@ void lu_stack_fini(const struct lu_env *env, struct lu_device *top)
>  		next = ldt->ldt_ops->ldto_device_free(env, scan);
>  		type = ldt->ldt_obd_type;
>  		if (type) {
> -			type->typ_refcnt--;
> +			atomic_dec(&type->typ_refcnt);
>  			class_put_type(type);
>  		}
>  	}
> 
> 
> 

[lustre-devel] [PATCH 05/28] lustre: obd_type: discard obd_type_lock

[NeilBrown]

On Fri, Mar 22 2019, James Simmons wrote:

>> This lock is only used to protect typ_refcnt, so change
>> that to an atomic_t and discard the lock.
>> 
>> The lock also covers calls to try_module_get and module_put,
>> but this serves no purpose as it does not prevent the module
>> from being unloaded.
>> 
>> Finally, the return value for the call to try_module_get is
>> ignored, which is not safe.
>
> Nak. Looking at the code we can easily use the kref of the
> kobject instead. The two special cases for the ref_count
> can be removed. The one for mdc_request was removed in
> patch https://review.whamcloud.com/30419. The other refcount
> use in mgc looks like its for supporting lustre 1.4 version
> logs. I bet that can be removed. Andreas can state if it 
> is still need.

Hi James,
 I think what you mean by "NAK" here is that you like the patch and that
 it improves the code, but there are even more improvements that can be
 made.
 Would that be correct?  In that case I'd rather leave the further
 improvements to a separate patch - especially if it needs confirmation
 from Andreas.

 Though I just noticed there is a bug in the patch - I dropped the
 module_put() from class_put_type() without any justification.  Oops.

Thanks,
NeilBrown

 

>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>>  drivers/staging/lustre/lustre/include/obd.h        |    3 +-
>>  drivers/staging/lustre/lustre/mdc/mdc_request.c    |    2 +
>>  drivers/staging/lustre/lustre/mgc/mgc_request.c    |    2 +
>>  drivers/staging/lustre/lustre/obdclass/genops.c    |   30 +++++++++-----------
>>  drivers/staging/lustre/lustre/obdclass/lu_object.c |    2 +
>>  5 files changed, 18 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
>> index 4c58b916e0a3..61fb8159af20 100644
>> --- a/drivers/staging/lustre/lustre/include/obd.h
>> +++ b/drivers/staging/lustre/lustre/include/obd.h
>> @@ -102,9 +102,8 @@ struct obd_type {
>>  	struct obd_ops		*typ_dt_ops;
>>  	struct md_ops		*typ_md_ops;
>>  	struct dentry		*typ_debugfs_entry;
>> -	int			 typ_refcnt;
>> +	atomic_t		 typ_refcnt;
>>  	struct lu_device_type	*typ_lu;
>> -	spinlock_t		 obd_type_lock;
>>  	struct kobject		 typ_kobj;
>>  };
>>  #define typ_name typ_kobj.name
>> diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
>> index bc764f9dd102..705a4e3b518a 100644
>> --- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
>> +++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
>> @@ -2542,7 +2542,7 @@ static int mdc_init_ea_size(struct obd_export *exp, u32 easize, u32 def_easize)
>>  static int mdc_precleanup(struct obd_device *obd)
>>  {
>>  	/* Failsafe, ok if racy */
>> -	if (obd->obd_type->typ_refcnt <= 1)
>> +	if (atomic_read(&obd->obd_type->typ_refcnt) <= 1)
>>  		libcfs_kkuc_group_rem(0, KUC_GRP_HSM);
>>  
>>  	mdc_changelog_cdev_finish(obd);
>> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> index 84ba6d0e3493..0580afa2755d 100644
>> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> @@ -715,7 +715,7 @@ static int mgc_cleanup(struct obd_device *obd)
>>  	/* COMPAT_146 - old config logs may have added profiles we don't
>>  	 * know about
>>  	 */
>> -	if (obd->obd_type->typ_refcnt <= 1)
>> +	if (atomic_read(&obd->obd_type->typ_refcnt) <= 1)
>>  		/* Only for the last mgc */
>>  		class_del_profiles();
>>  
>> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
>> index 74195de639e4..02d829617519 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
>> @@ -113,15 +113,17 @@ static struct obd_type *class_get_type(const char *name)
>>  		}
>>  	}
>>  	if (type) {
>> -		spin_lock(&type->obd_type_lock);
>> -		type->typ_refcnt++;
>> -		try_module_get(type->typ_dt_ops->owner);
>> -		spin_unlock(&type->obd_type_lock);
>> -		/* class_search_type() returned a counted reference,
>> -		 * but we don't need that count any more as
>> -		 * we have one through typ_refcnt.
>> -		 */
>> -		kobject_put(&type->typ_kobj);
>> +		if (try_module_get(type->typ_dt_ops->owner)) {
>> +			atomic_inc(&type->typ_refcnt);
>> +			/* class_search_type() returned a counted reference,
>> +			 * but we don't need that count any more as
>> +			 * we have one through typ_refcnt.
>> +			 */
>> +			kobject_put(&type->typ_kobj);
>> +		} else {
>> +			kobject_put(&type->typ_kobj);
>> +			type = NULL;
>> +		}
>>  	}
>>  	return type;
>>  }
>> @@ -129,10 +131,7 @@ static struct obd_type *class_get_type(const char *name)
>>  void class_put_type(struct obd_type *type)
>>  {
>>  	LASSERT(type);
>> -	spin_lock(&type->obd_type_lock);
>> -	type->typ_refcnt--;
>> -	module_put(type->typ_dt_ops->owner);
>> -	spin_unlock(&type->obd_type_lock);
>> +	atomic_dec(&type->typ_refcnt);
>>  }
>>  
>>  static void simple_class_release(struct kobject *kobj)
>> @@ -222,7 +221,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops,
>>  	/* md_ops is optional */
>>  	if (md_ops)
>>  		*type->typ_md_ops = *md_ops;
>> -	spin_lock_init(&type->obd_type_lock);
>>  
>>  	rc = kobject_add(&type->typ_kobj, &lustre_kset->kobj, "%s", name);
>>  	if (rc)
>> @@ -256,8 +254,8 @@ int class_unregister_type(const char *name)
>>  		return -EINVAL;
>>  	}
>>  
>> -	if (type->typ_refcnt) {
>> -		CERROR("type %s has refcount (%d)\n", name, type->typ_refcnt);
>> +	if (atomic_read(&type->typ_refcnt)) {
>> +		CERROR("type %s has refcount (%d)\n", name, atomic_read(&type->typ_refcnt));
>>  		/* This is a bad situation, let's make the best of it */
>>  		/* Remove ops, but leave the name for debugging */
>>  		kfree(type->typ_dt_ops);
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> index 9c872db21040..770cc1b9e40b 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c
>> @@ -1267,7 +1267,7 @@ void lu_stack_fini(const struct lu_env *env, struct lu_device *top)
>>  		next = ldt->ldt_ops->ldto_device_free(env, scan);
>>  		type = ldt->ldt_obd_type;
>>  		if (type) {
>> -			type->typ_refcnt--;
>> +			atomic_dec(&type->typ_refcnt);
>>  			class_put_type(type);
>>  		}
>>  	}
>> 
>> 
>> 
-------------- 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/20190325/12b61df9/attachment.sig>

[lustre-devel] [PATCH 05/28] lustre: obd_type: discard obd_type_lock

[Andreas Dilger]

On Fri, Mar 22 2019, James Simmons wrote:
>> This lock is only used to protect typ_refcnt, so change
>> that to an atomic_t and discard the lock.
>> 
>> The lock also covers calls to try_module_get and module_put,
>> but this serves no purpose as it does not prevent the module
>> from being unloaded.
>> 
>> Finally, the return value for the call to try_module_get is
>> ignored, which is not safe.
> 
> Nak. Looking at the code we can easily use the kref of the
> kobject instead. The two special cases for the ref_count
> can be removed. The one for mdc_request was removed in
> patch https://review.whamcloud.com/30419. The other refcount
> use in mgc looks like its for supporting lustre 1.4 version
> logs. I bet that can be removed. Andreas can state if it is
> still need.

James, I looked at that patch, but don't see anything in it
that relates to this.  Could you please point out the specific
code you are referencing.  Definitely there may be old code
lurking in some corners that could be removed.

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud

[lustre-devel] [PATCH 11/28] lustre: handles: discard h_owner in favour of h_ops

[Andreas Dilger]

On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
> 
> lustre_handles  assigned a  64bit  unique identifier  (a 'cookie')  to
> objects of  various types and  stored them  in a hash  table, allowing
> them to be accessed by the cookie.
> 
> The is a facility for type checking by recording an 'owner' for each
> object, and checking the owner on lookup.  Unfortunately this is not
> used - owner is always zero.
> 
> Eahc object also contains an h_ops pointer which can be used to
> reliably identify an owner.
> 
> So discard h_owner, pass and 'ops' pointer to class_handle2object(),
> and only return objects for which the h_ops matches.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Probably a bit late to the party here, but it would be useful to add a
portability note here, that the pointer to "struct mdt_export_data"
that is currently stored in h_owner should move to "struct mdt_file_data"
in the server code.

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

> ---
> .../staging/lustre/lustre/include/lustre_handles.h |    7 +++----
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    2 +-
> drivers/staging/lustre/lustre/obdclass/genops.c    |    3 ++-
> .../lustre/lustre/obdclass/lustre_handles.c        |    6 +++---
> 4 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
> index 683680891e4c..9a4b1a821e7b 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_handles.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
> @@ -65,8 +65,7 @@ struct portals_handle_ops {
> struct portals_handle {
> 	struct list_head		h_link;
> 	u64				h_cookie;
> -	const void			*h_owner;
> -	struct portals_handle_ops	*h_ops;
> +	const struct portals_handle_ops	*h_ops;
> 
> 	/* newly added fields to handle the RCU issue. -jxiong */
> 	struct rcu_head			h_rcu;
> @@ -79,9 +78,9 @@ struct portals_handle {
> 
> /* Add a handle to the hash table */
> void class_handle_hash(struct portals_handle *,
> -		       struct portals_handle_ops *ops);
> +		       const struct portals_handle_ops *ops);
> void class_handle_unhash(struct portals_handle *);
> -void *class_handle2object(u64 cookie, const void *owner);
> +void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops);
> void class_handle_free_cb(struct rcu_head *rcu);
> int class_handle_init(void);
> void class_handle_cleanup(void);
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index 7ec5fc900da8..768cccc1fa82 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -515,7 +515,7 @@ struct ldlm_lock *__ldlm_handle2lock(const struct lustre_handle *handle,
> 
> 	LASSERT(handle);
> 
> -	lock = class_handle2object(handle->cookie, NULL);
> +	lock = class_handle2object(handle->cookie, &lock_handle_ops);
> 	if (!lock)
> 		return NULL;
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> index e206bb401fe3..42859fbca330 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -708,6 +708,7 @@ int obd_init_caches(void)
> 	return -ENOMEM;
> }
> 
> +static struct portals_handle_ops export_handle_ops;
> /* map connection to client */
> struct obd_export *class_conn2export(struct lustre_handle *conn)
> {
> @@ -724,7 +725,7 @@ struct obd_export *class_conn2export(struct lustre_handle *conn)
> 	}
> 
> 	CDEBUG(D_INFO, "looking for export cookie %#llx\n", conn->cookie);
> -	export = class_handle2object(conn->cookie, NULL);
> +	export = class_handle2object(conn->cookie, &export_handle_ops);
> 	return export;
> }
> EXPORT_SYMBOL(class_conn2export);
> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> index 0674afb0059f..32b70d613f71 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> @@ -59,7 +59,7 @@ static struct handle_bucket {
>  * global (per-node) hash-table.
>  */
> void class_handle_hash(struct portals_handle *h,
> -		       struct portals_handle_ops *ops)
> +		       const struct portals_handle_ops *ops)
> {
> 	struct handle_bucket *bucket;
> 
> @@ -132,7 +132,7 @@ void class_handle_unhash(struct portals_handle *h)
> }
> EXPORT_SYMBOL(class_handle_unhash);
> 
> -void *class_handle2object(u64 cookie, const void *owner)
> +void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
> {
> 	struct handle_bucket *bucket;
> 	struct portals_handle *h;
> @@ -147,7 +147,7 @@ void *class_handle2object(u64 cookie, const void *owner)
> 
> 	rcu_read_lock();
> 	list_for_each_entry_rcu(h, &bucket->head, h_link) {
> -		if (h->h_cookie != cookie || h->h_owner != owner)
> +		if (h->h_cookie != cookie || h->h_ops != ops)
> 			continue;
> 
> 		spin_lock(&h->h_lock);
> 
> 

Cheers, Andreas
---
Andreas Dilger
CTO Whamcloud

[lustre-devel] [PATCH 12/28] lustre: handle: move refcount into the lustre_handle.

[Andreas Dilger]

On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
> 
> Every object with a lustre_handle has (and must have) a refcount.
> The lustre_handles code needs a call-out to increment this.
> To simplify things, move the refcount into the lustre_handle
> and discard the call-out.
> 
> In order to preserve the same debug messages, we store and object type
> name in the portals_handle_ops, and use that in a CDEBUG() when
> incrementing the ref count.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

> ---
> drivers/staging/lustre/lustre/include/lustre_dlm.h |    6 --
> .../staging/lustre/lustre/include/lustre_export.h  |    1 
> .../staging/lustre/lustre/include/lustre_handles.h |    4 +-
> .../staging/lustre/lustre/include/lustre_import.h  |    2 -
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |   36 ++++++--------
> drivers/staging/lustre/lustre/obdclass/genops.c    |   50 ++++++++------------
> .../lustre/lustre/obdclass/lustre_handles.c        |    5 ++
> .../staging/lustre/lustre/obdecho/echo_client.c    |    2 -
> drivers/staging/lustre/lustre/ptlrpc/service.c     |    4 +-
> 9 files changed, 45 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
> index 1bd511906f69..fd9b0f870c93 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
> @@ -590,12 +590,6 @@ struct ldlm_lock {
> 	 * Must be first in the structure.
> 	 */
> 	struct portals_handle		l_handle;
> -	/**
> -	 * Lock reference count.
> -	 * This is how many users have pointers to actual structure, so that
> -	 * we do not accidentally free lock structure that is in use.
> -	 */
> -	atomic_t			l_refc;
> 	/**
> 	 * Internal spinlock protects l_resource.  We should hold this lock
> 	 * first before taking res_lock.
> diff --git a/drivers/staging/lustre/lustre/include/lustre_export.h b/drivers/staging/lustre/lustre/include/lustre_export.h
> index fb34e0b7de35..d05323313f55 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_export.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_export.h
> @@ -67,7 +67,6 @@ struct obd_export {
> 	 * what export they are talking to.
> 	 */
> 	struct portals_handle		exp_handle;
> -	refcount_t			exp_refcount;
> 	/**
> 	 * Set of counters below is to track where export references are
> 	 * kept. The exp_rpc_count is used for reconnect handling also,
> diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
> index 9a4b1a821e7b..be5d41b1a398 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_handles.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
> @@ -46,8 +46,9 @@
> #include <linux/types.h>
> 
> struct portals_handle_ops {
> -	void (*hop_addref)(void *object);
> 	void (*hop_free)(void *object, int size);
> +	/* hop_type is used for some debugging messages */
> +	char *hop_type;
> };
> 
> /* These handles are most easily used by having them appear at the very top of
> @@ -66,6 +67,7 @@ struct portals_handle {
> 	struct list_head		h_link;
> 	u64				h_cookie;
> 	const struct portals_handle_ops	*h_ops;
> +	refcount_t			h_ref;
> 
> 	/* newly added fields to handle the RCU issue. -jxiong */
> 	struct rcu_head			h_rcu;
> diff --git a/drivers/staging/lustre/lustre/include/lustre_import.h b/drivers/staging/lustre/lustre/include/lustre_import.h
> index fc1f87cc1172..6c830da797c1 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_import.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_import.h
> @@ -154,8 +154,6 @@ struct import_state_hist {
> struct obd_import {
> 	/** Local handle (== id) for this import. */
> 	struct portals_handle		imp_handle;
> -	/** Reference counter */
> -	atomic_t			imp_refcount;
> 	struct lustre_handle		imp_dlm_handle; /* client's ldlm export */
> 	/** Currently active connection */
> 	struct ptlrpc_connection       *imp_connection;
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index 768cccc1fa82..ecd3f4a93e8d 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -148,7 +148,7 @@ EXPORT_SYMBOL(ldlm_it2str);
>  */
> struct ldlm_lock *ldlm_lock_get(struct ldlm_lock *lock)
> {
> -	atomic_inc(&lock->l_refc);
> +	refcount_inc(&lock->l_handle.h_ref);
> 	return lock;
> }
> EXPORT_SYMBOL(ldlm_lock_get);
> @@ -161,8 +161,8 @@ EXPORT_SYMBOL(ldlm_lock_get);
> void ldlm_lock_put(struct ldlm_lock *lock)
> {
> 	LASSERT(lock->l_resource != LP_POISON);
> -	LASSERT(atomic_read(&lock->l_refc) > 0);
> -	if (atomic_dec_and_test(&lock->l_refc)) {
> +	LASSERT(refcount_read(&lock->l_handle.h_ref) > 0);
> +	if (refcount_dec_and_test(&lock->l_handle.h_ref)) {
> 		struct ldlm_resource *res;
> 
> 		LDLM_DEBUG(lock,
> @@ -356,12 +356,6 @@ void ldlm_lock_destroy_nolock(struct ldlm_lock *lock)
> 	}
> }
> 
> -/* this is called by portals_handle2object with the handle lock taken */
> -static void lock_handle_addref(void *lock)
> -{
> -	LDLM_LOCK_GET((struct ldlm_lock *)lock);
> -}
> -
> static void lock_handle_free(void *lock, int size)
> {
> 	LASSERT(size == sizeof(struct ldlm_lock));
> @@ -369,8 +363,8 @@ static void lock_handle_free(void *lock, int size)
> }
> 
> static struct portals_handle_ops lock_handle_ops = {
> -	.hop_addref = lock_handle_addref,
> 	.hop_free   = lock_handle_free,
> +	.hop_type   = "ldlm",
> };
> 
> /**
> @@ -395,7 +389,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
> 	lock->l_resource = resource;
> 	lu_ref_add(&resource->lr_reference, "lock", lock);
> 
> -	atomic_set(&lock->l_refc, 2);
> +	refcount_set(&lock->l_handle.h_ref, 2);
> 	INIT_LIST_HEAD(&lock->l_res_link);
> 	INIT_LIST_HEAD(&lock->l_lru);
> 	INIT_LIST_HEAD(&lock->l_pending_chain);
> @@ -1996,13 +1990,13 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 &vaf,
> 				 lock,
> 				 lock->l_handle.h_cookie,
> -				 atomic_read(&lock->l_refc),
> +				 refcount_read(&lock->l_handle.h_ref),
> 				 lock->l_readers, lock->l_writers,
> 				 ldlm_lockname[lock->l_granted_mode],
> 				 ldlm_lockname[lock->l_req_mode],
> 				 lock->l_flags, nid,
> 				 lock->l_remote_handle.cookie,
> -				 exp ? refcount_read(&exp->exp_refcount) : -99,
> +				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
> 				 lock->l_pid, lock->l_callback_timeout,
> 				 lock->l_lvb_type);
> 		va_end(args);
> @@ -2016,7 +2010,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 &vaf,
> 				 ldlm_lock_to_ns_name(lock), lock,
> 				 lock->l_handle.h_cookie,
> -				 atomic_read(&lock->l_refc),
> +				 refcount_read(&lock->l_handle.h_ref),
> 				 lock->l_readers, lock->l_writers,
> 				 ldlm_lockname[lock->l_granted_mode],
> 				 ldlm_lockname[lock->l_req_mode],
> @@ -2029,7 +2023,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 lock->l_req_extent.end,
> 				 lock->l_flags, nid,
> 				 lock->l_remote_handle.cookie,
> -				 exp ? refcount_read(&exp->exp_refcount) : -99,
> +				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
> 				 lock->l_pid, lock->l_callback_timeout,
> 				 lock->l_lvb_type);
> 		break;
> @@ -2040,7 +2034,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 &vaf,
> 				 ldlm_lock_to_ns_name(lock), lock,
> 				 lock->l_handle.h_cookie,
> -				 atomic_read(&lock->l_refc),
> +				 refcount_read(&lock->l_handle.h_ref),
> 				 lock->l_readers, lock->l_writers,
> 				 ldlm_lockname[lock->l_granted_mode],
> 				 ldlm_lockname[lock->l_req_mode],
> @@ -2052,7 +2046,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 lock->l_policy_data.l_flock.end,
> 				 lock->l_flags, nid,
> 				 lock->l_remote_handle.cookie,
> -				 exp ? refcount_read(&exp->exp_refcount) : -99,
> +				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
> 				 lock->l_pid, lock->l_callback_timeout);
> 		break;
> 
> @@ -2062,7 +2056,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 &vaf,
> 				 ldlm_lock_to_ns_name(lock),
> 				 lock, lock->l_handle.h_cookie,
> -				 atomic_read(&lock->l_refc),
> +				 refcount_read(&lock->l_handle.h_ref),
> 				 lock->l_readers, lock->l_writers,
> 				 ldlm_lockname[lock->l_granted_mode],
> 				 ldlm_lockname[lock->l_req_mode],
> @@ -2072,7 +2066,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 ldlm_typename[resource->lr_type],
> 				 lock->l_flags, nid,
> 				 lock->l_remote_handle.cookie,
> -				 exp ? refcount_read(&exp->exp_refcount) : -99,
> +				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
> 				 lock->l_pid, lock->l_callback_timeout,
> 				 lock->l_lvb_type);
> 		break;
> @@ -2083,7 +2077,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 &vaf,
> 				 ldlm_lock_to_ns_name(lock),
> 				 lock, lock->l_handle.h_cookie,
> -				 atomic_read(&lock->l_refc),
> +				 refcount_read(&lock->l_handle.h_ref),
> 				 lock->l_readers, lock->l_writers,
> 				 ldlm_lockname[lock->l_granted_mode],
> 				 ldlm_lockname[lock->l_req_mode],
> @@ -2092,7 +2086,7 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
> 				 ldlm_typename[resource->lr_type],
> 				 lock->l_flags, nid,
> 				 lock->l_remote_handle.cookie,
> -				 exp ? refcount_read(&exp->exp_refcount) : -99,
> +				 exp ? refcount_read(&exp->exp_handle.h_ref) : -99,
> 				 lock->l_pid, lock->l_callback_timeout,
> 				 lock->l_lvb_type);
> 		break;
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> index 42859fbca330..c6a5e6569c88 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -753,7 +753,7 @@ static void class_export_destroy(struct obd_export *exp)
> {
> 	struct obd_device *obd = exp->exp_obd;
> 
> -	LASSERT(refcount_read(&exp->exp_refcount) == 0);
> +	LASSERT(refcount_read(&exp->exp_handle.h_ref) == 0);
> 	LASSERT(obd);
> 
> 	CDEBUG(D_IOCTL, "destroying export %p/%s for %s\n", exp,
> @@ -777,33 +777,28 @@ static void class_export_destroy(struct obd_export *exp)
> 	OBD_FREE_RCU(exp, sizeof(*exp), &exp->exp_handle);
> }
> 
> -static void export_handle_addref(void *export)
> -{
> -	class_export_get(export);
> -}
> -
> static struct portals_handle_ops export_handle_ops = {
> -	.hop_addref	= export_handle_addref,
> 	.hop_free	= NULL,
> +	.hop_type	= "export",
> };
> 
> struct obd_export *class_export_get(struct obd_export *exp)
> {
> -	refcount_inc(&exp->exp_refcount);
> -	CDEBUG(D_INFO, "GETting export %p : new refcount %d\n", exp,
> -	       refcount_read(&exp->exp_refcount));
> +	refcount_inc(&exp->exp_handle.h_ref);
> +	CDEBUG(D_INFO, "GET export %p refcount=%d\n", exp,
> +	       refcount_read(&exp->exp_handle.h_ref));
> 	return exp;
> }
> EXPORT_SYMBOL(class_export_get);
> 
> void class_export_put(struct obd_export *exp)
> {
> -	LASSERT(refcount_read(&exp->exp_refcount) >  0);
> -	LASSERT(refcount_read(&exp->exp_refcount) < LI_POISON);
> +	LASSERT(refcount_read(&exp->exp_handle.h_ref) >  0);
> +	LASSERT(refcount_read(&exp->exp_handle.h_ref) < LI_POISON);
> 	CDEBUG(D_INFO, "PUTting export %p : new refcount %d\n", exp,
> -	       refcount_read(&exp->exp_refcount) - 1);
> +	       refcount_read(&exp->exp_handle.h_ref) - 1);
> 
> -	if (refcount_dec_and_test(&exp->exp_refcount)) {
> +	if (refcount_dec_and_test(&exp->exp_handle.h_ref)) {
> 		struct obd_device *obd = exp->exp_obd;
> 
> 		CDEBUG(D_IOCTL, "final put %p/%s\n",
> @@ -853,7 +848,7 @@ static struct obd_export *__class_new_export(struct obd_device *obd,
> 
> 	export->exp_conn_cnt = 0;
> 	/* 2 = class_handle_hash + last */
> -	refcount_set(&export->exp_refcount, 2);
> +	refcount_set(&export->exp_handle.h_ref, 2);
> 	atomic_set(&export->exp_rpc_count, 0);
> 	atomic_set(&export->exp_cb_count, 0);
> 	atomic_set(&export->exp_locks_count, 0);
> @@ -956,7 +951,7 @@ static void class_import_destroy(struct obd_import *imp)
> 	CDEBUG(D_IOCTL, "destroying import %p for %s\n", imp,
> 	       imp->imp_obd->obd_name);
> 
> -	LASSERT_ATOMIC_ZERO(&imp->imp_refcount);
> +	LASSERT(refcount_read(&imp->imp_handle.h_ref) == 0);
> 
> 	ptlrpc_put_connection_superhack(imp->imp_connection);
> 
> @@ -973,21 +968,16 @@ static void class_import_destroy(struct obd_import *imp)
> 	OBD_FREE_RCU(imp, sizeof(*imp), &imp->imp_handle);
> }
> 
> -static void import_handle_addref(void *import)
> -{
> -	class_import_get(import);
> -}
> -
> static struct portals_handle_ops import_handle_ops = {
> -	.hop_addref	= import_handle_addref,
> 	.hop_free	= NULL,
> +	.hop_type	= "import",
> };
> 
> struct obd_import *class_import_get(struct obd_import *import)
> {
> -	atomic_inc(&import->imp_refcount);
> -	CDEBUG(D_INFO, "import %p refcount=%d obd=%s\n", import,
> -	       atomic_read(&import->imp_refcount),
> +	refcount_inc(&import->imp_handle.h_ref);
> +	CDEBUG(D_INFO, "GET import %p refcount=%d obd=%s\n", import,
> +	       refcount_read(&import->imp_handle.h_ref),
> 	       import->imp_obd->obd_name);
> 	return import;
> }
> @@ -995,19 +985,19 @@ EXPORT_SYMBOL(class_import_get);
> 
> void class_import_put(struct obd_import *imp)
> {
> -	LASSERT_ATOMIC_GT_LT(&imp->imp_refcount, 0, LI_POISON);
> +	LASSERT(refcount_read(&imp->imp_handle.h_ref) > 0);
> 
> 	CDEBUG(D_INFO, "import %p refcount=%d obd=%s\n", imp,
> -	       atomic_read(&imp->imp_refcount) - 1,
> +	       refcount_read(&imp->imp_handle.h_ref) - 1,
> 	       imp->imp_obd->obd_name);
> 
> -	if (atomic_dec_and_test(&imp->imp_refcount)) {
> +	if (refcount_dec_and_test(&imp->imp_handle.h_ref)) {
> 		CDEBUG(D_INFO, "final put import %p\n", imp);
> 		obd_zombie_import_add(imp);
> 	}
> 
> 	/* catch possible import put race */
> -	LASSERT_ATOMIC_GE_LT(&imp->imp_refcount, 0, LI_POISON);
> +	LASSERT(refcount_read(&imp->imp_handle.h_ref) >= 0);
> }
> EXPORT_SYMBOL(class_import_put);
> 
> @@ -1057,7 +1047,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
> 	init_waitqueue_head(&imp->imp_recovery_waitq);
> 	INIT_WORK(&imp->imp_zombie_work, obd_zombie_imp_cull);
> 
> -	atomic_set(&imp->imp_refcount, 2);
> +	refcount_set(&imp->imp_handle.h_ref, 2);
> 	atomic_set(&imp->imp_unregistering, 0);
> 	atomic_set(&imp->imp_inflight, 0);
> 	atomic_set(&imp->imp_replay_inflight, 0);
> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> index 32b70d613f71..32cd6aedd5f6 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> @@ -152,7 +152,10 @@ void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
> 
> 		spin_lock(&h->h_lock);
> 		if (likely(h->h_in != 0)) {
> -			h->h_ops->hop_addref(h);
> +			refcount_inc(&h->h_ref);
> +			CDEBUG(D_INFO, "GET %s %p refcount=%d\n",
> +			       h->h_ops->hop_type, h,
> +			       refcount_read(&h->h_ref));
> 			retval = h;
> 		}
> 		spin_unlock(&h->h_lock);
> diff --git a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
> index baf34c85c4b5..6f00eee6170a 100644
> --- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
> +++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
> @@ -1638,7 +1638,7 @@ static int echo_client_cleanup(struct obd_device *obddev)
> 		return -EBUSY;
> 	}
> 
> -	LASSERT(refcount_read(&ec->ec_exp->exp_refcount) > 0);
> +	LASSERT(refcount_read(&ec->ec_exp->exp_handle.h_ref) > 0);
> 	rc = obd_disconnect(ec->ec_exp);
> 	if (rc != 0)
> 		CERROR("fail to disconnect device: %d\n", rc);
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
> index 5a7e9fa6e5c8..5e541ca0dc00 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/service.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
> @@ -1697,7 +1697,7 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt,
> 	       (request->rq_export ?
> 		(char *)request->rq_export->exp_client_uuid.uuid : "0"),
> 	       (request->rq_export ?
> -		refcount_read(&request->rq_export->exp_refcount) : -99),
> +		refcount_read(&request->rq_export->exp_handle.h_ref) : -99),
> 	       lustre_msg_get_status(request->rq_reqmsg), request->rq_xid,
> 	       libcfs_id2str(request->rq_peer),
> 	       lustre_msg_get_opc(request->rq_reqmsg));
> @@ -1741,7 +1741,7 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt,
> 	       (request->rq_export ?
> 		(char *)request->rq_export->exp_client_uuid.uuid : "0"),
> 	       (request->rq_export ?
> -		refcount_read(&request->rq_export->exp_refcount) : -99),
> +		refcount_read(&request->rq_export->exp_handle.h_ref) : -99),
> 	       lustre_msg_get_status(request->rq_reqmsg),
> 	       request->rq_xid,
> 	       libcfs_id2str(request->rq_peer),
> 
> 

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud

[lustre-devel] [PATCH 14/28] lustre: portals_handle: rename ops to owner

[Andreas Dilger]

On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
> 
> Now the portals_handle_ops contains only a char*,
> it is functioning primarily to identify the owner of each handle.
> So change the name to h_owner, and the type to char*.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

> ---
> .../staging/lustre/lustre/include/lustre_handles.h |   12 +++---------
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    8 +++-----
> drivers/staging/lustre/lustre/obdclass/genops.c    |   17 ++++++-----------
> .../lustre/lustre/obdclass/lustre_handles.c        |   15 +++++++--------
> 4 files changed, 19 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
> index 8fb42851f6d1..ebbbb01710e7 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_handles.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
> @@ -45,11 +45,6 @@
> #include <linux/spinlock.h>
> #include <linux/types.h>
> 
> -struct portals_handle_ops {
> -	/* hop_type is used for some debugging messages */
> -	char *hop_type;
> -};
> -
> /* These handles are most easily used by having them appear at the very top of
>  * whatever object that you want to make handles for.  ie:
>  *
> @@ -65,7 +60,7 @@ struct portals_handle_ops {
> struct portals_handle {
> 	struct list_head		h_link;
> 	u64				h_cookie;
> -	const struct portals_handle_ops	*h_ops;
> +	char				*h_owner;
> 	refcount_t			h_ref;
> 
> 	/* newly added fields to handle the RCU issue. -jxiong */
> @@ -77,10 +72,9 @@ struct portals_handle {
> /* handles.c */
> 
> /* Add a handle to the hash table */
> -void class_handle_hash(struct portals_handle *,
> -		       const struct portals_handle_ops *ops);
> +void class_handle_hash(struct portals_handle *, char *owner);
> void class_handle_unhash(struct portals_handle *);
> -void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops);
> +void *class_handle2object(u64 cookie, char *owner);
> int class_handle_init(void);
> void class_handle_cleanup(void);
> 
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index 18f018d27936..56a2d1dcd663 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -363,9 +363,7 @@ void ldlm_lock_destroy_nolock(struct ldlm_lock *lock)
> 	}
> }
> 
> -static struct portals_handle_ops lock_handle_ops = {
> -	.hop_type   = "ldlm",
> -};
> +static const char lock_handle_owner[] = "ldlm";
> 
> /**
>  *
> @@ -405,7 +403,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
> 	lprocfs_counter_incr(ldlm_res_to_ns(resource)->ns_stats,
> 			     LDLM_NSS_LOCKS);
> 	INIT_LIST_HEAD(&lock->l_handle.h_link);
> -	class_handle_hash(&lock->l_handle, &lock_handle_ops);
> +	class_handle_hash(&lock->l_handle, lock_handle_owner);
> 
> 	lu_ref_init(&lock->l_reference);
> 	lu_ref_add(&lock->l_reference, "hash", lock);
> @@ -509,7 +507,7 @@ struct ldlm_lock *__ldlm_handle2lock(const struct lustre_handle *handle,
> 
> 	LASSERT(handle);
> 
> -	lock = class_handle2object(handle->cookie, &lock_handle_ops);
> +	lock = class_handle2object(handle->cookie, lock_handle_owner);
> 	if (!lock)
> 		return NULL;
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> index ed306bbfbfb8..e0da46e7d355 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -708,7 +708,8 @@ int obd_init_caches(void)
> 	return -ENOMEM;
> }
> 
> -static struct portals_handle_ops export_handle_ops;
> +static const char export_handle_owner[] = "export";
> +
> /* map connection to client */
> struct obd_export *class_conn2export(struct lustre_handle *conn)
> {
> @@ -725,7 +726,7 @@ struct obd_export *class_conn2export(struct lustre_handle *conn)
> 	}
> 
> 	CDEBUG(D_INFO, "looking for export cookie %#llx\n", conn->cookie);
> -	export = class_handle2object(conn->cookie, &export_handle_ops);
> +	export = class_handle2object(conn->cookie, export_handle_owner);
> 	return export;
> }
> EXPORT_SYMBOL(class_conn2export);
> @@ -777,10 +778,6 @@ static void class_export_destroy(struct obd_export *exp)
> 	kfree_rcu(exp, exp_handle.h_rcu);
> }
> 
> -static struct portals_handle_ops export_handle_ops = {
> -	.hop_type	= "export",
> -};
> -
> struct obd_export *class_export_get(struct obd_export *exp)
> {
> 	refcount_inc(&exp->exp_handle.h_ref);
> @@ -863,7 +860,7 @@ static struct obd_export *__class_new_export(struct obd_device *obd,
> 	INIT_LIST_HEAD(&export->exp_req_replay_queue);
> 	INIT_LIST_HEAD(&export->exp_handle.h_link);
> 	INIT_LIST_HEAD(&export->exp_hp_rpcs);
> -	class_handle_hash(&export->exp_handle, &export_handle_ops);
> +	class_handle_hash(&export->exp_handle, export_handle_owner);
> 	spin_lock_init(&export->exp_lock);
> 	spin_lock_init(&export->exp_rpc_lock);
> 	spin_lock_init(&export->exp_bl_list_lock);
> @@ -967,9 +964,7 @@ static void class_import_destroy(struct obd_import *imp)
> 	kfree_rcu(imp, imp_handle.h_rcu);
> }
> 
> -static struct portals_handle_ops import_handle_ops = {
> -	.hop_type	= "import",
> -};
> +static const char import_handle_owner[] = "import";
> 
> struct obd_import *class_import_get(struct obd_import *import)
> {
> @@ -1052,7 +1047,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
> 	atomic_set(&imp->imp_inval_count, 0);
> 	INIT_LIST_HEAD(&imp->imp_conn_list);
> 	INIT_LIST_HEAD(&imp->imp_handle.h_link);
> -	class_handle_hash(&imp->imp_handle, &import_handle_ops);
> +	class_handle_hash(&imp->imp_handle, import_handle_owner);
> 	init_imp_at(&imp->imp_at);
> 
> 	/* the default magic is V2, will be used in connect RPC, and
> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> index 8aece57ec8c9..f41558ccdfcf 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> @@ -58,8 +58,7 @@ static struct handle_bucket {
>  * Generate a unique 64bit cookie (hash) for a handle and insert it into
>  * global (per-node) hash-table.
>  */
> -void class_handle_hash(struct portals_handle *h,
> -		       const struct portals_handle_ops *ops)
> +void class_handle_hash(struct portals_handle *h, char *owner)
> {
> 	struct handle_bucket *bucket;
> 
> @@ -85,7 +84,7 @@ void class_handle_hash(struct portals_handle *h,
> 	h->h_cookie = handle_base;
> 	spin_unlock(&handle_base_lock);
> 
> -	h->h_ops = ops;
> +	h->h_owner = owner;
> 	spin_lock_init(&h->h_lock);
> 
> 	bucket = &handle_hash[h->h_cookie & HANDLE_HASH_MASK];
> @@ -132,7 +131,7 @@ void class_handle_unhash(struct portals_handle *h)
> }
> EXPORT_SYMBOL(class_handle_unhash);
> 
> -void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
> +void *class_handle2object(u64 cookie, char *owner)
> {
> 	struct handle_bucket *bucket;
> 	struct portals_handle *h;
> @@ -147,14 +146,14 @@ void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
> 
> 	rcu_read_lock();
> 	list_for_each_entry_rcu(h, &bucket->head, h_link) {
> -		if (h->h_cookie != cookie || h->h_ops != ops)
> +		if (h->h_cookie != cookie || h->h_owner != owner)
> 			continue;
> 
> 		spin_lock(&h->h_lock);
> 		if (likely(h->h_in != 0)) {
> 			refcount_inc(&h->h_ref);
> 			CDEBUG(D_INFO, "GET %s %p refcount=%d\n",
> -			       h->h_ops->hop_type, h,
> +			       h->h_owner, h,
> 			       refcount_read(&h->h_ref));
> 			retval = h;
> 		}
> @@ -201,8 +200,8 @@ static int cleanup_all_handles(void)
> 
> 		spin_lock(&handle_hash[i].lock);
> 		list_for_each_entry_rcu(h, &handle_hash[i].head, h_link) {
> -			CERROR("force clean handle %#llx addr %p ops %p\n",
> -			       h->h_cookie, h, h->h_ops);
> +			CERROR("force clean handle %#llx addr %p owner %p\n",
> +			       h->h_cookie, h, h->h_owner);
> 
> 			class_handle_unhash_nolock(h);
> 			rc++;
> 
> 

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud

[lustre-devel] [PATCH 16/28] lustre: portals_handle: use hlist for hash lists.

[Andreas Dilger]

On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
> 
> hlist_head/hlist_node is the preferred data structure
> for hash tables. Not only does it make the 'head' smaller,
> but is also provides hlist_unhashed() which can be used to
> check if an object is in the list.  This means that
> we don't need h_in any more.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

> ---
> .../staging/lustre/lustre/include/lustre_handles.h |    3 +--
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    2 +-
> drivers/staging/lustre/lustre/obdclass/genops.c    |    4 ++--
> .../lustre/lustre/obdclass/lustre_handles.c        |   20 +++++++++-----------
> 4 files changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
> index ebbbb01710e7..cc433f48d367 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_handles.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
> @@ -58,7 +58,7 @@
>  * to compute the start of the structure based on the handle field.
>  */
> struct portals_handle {
> -	struct list_head		h_link;
> +	struct hlist_node		h_link;
> 	u64				h_cookie;
> 	char				*h_owner;
> 	refcount_t			h_ref;
> @@ -66,7 +66,6 @@ struct portals_handle {
> 	/* newly added fields to handle the RCU issue. -jxiong */
> 	struct rcu_head			h_rcu;
> 	spinlock_t			h_lock;
> -	unsigned int			h_in:1;
> };
> 
> /* handles.c */
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index 56a2d1dcd663..5ac77238e5f2 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -402,7 +402,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
> 
> 	lprocfs_counter_incr(ldlm_res_to_ns(resource)->ns_stats,
> 			     LDLM_NSS_LOCKS);
> -	INIT_LIST_HEAD(&lock->l_handle.h_link);
> +	INIT_HLIST_NODE(&lock->l_handle.h_link);
> 	class_handle_hash(&lock->l_handle, lock_handle_owner);
> 
> 	lu_ref_init(&lock->l_reference);
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
> index e0da46e7d355..562e8a9f35c9 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -858,7 +858,7 @@ static struct obd_export *__class_new_export(struct obd_device *obd,
> 	spin_lock_init(&export->exp_uncommitted_replies_lock);
> 	INIT_LIST_HEAD(&export->exp_uncommitted_replies);
> 	INIT_LIST_HEAD(&export->exp_req_replay_queue);
> -	INIT_LIST_HEAD(&export->exp_handle.h_link);
> +	INIT_HLIST_NODE(&export->exp_handle.h_link);
> 	INIT_LIST_HEAD(&export->exp_hp_rpcs);
> 	class_handle_hash(&export->exp_handle, export_handle_owner);
> 	spin_lock_init(&export->exp_lock);
> @@ -1046,7 +1046,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
> 	atomic_set(&imp->imp_replay_inflight, 0);
> 	atomic_set(&imp->imp_inval_count, 0);
> 	INIT_LIST_HEAD(&imp->imp_conn_list);
> -	INIT_LIST_HEAD(&imp->imp_handle.h_link);
> +	INIT_HLIST_NODE(&imp->imp_handle.h_link);
> 	class_handle_hash(&imp->imp_handle, import_handle_owner);
> 	init_imp_at(&imp->imp_at);
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> index 45e5eac47292..72de668c879b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> @@ -48,7 +48,7 @@ static spinlock_t handle_base_lock;
> 
> static struct handle_bucket {
> 	spinlock_t		lock;
> -	struct list_head	head;
> +	struct hlist_head	head;
> } *handle_hash;
> 
> #define HANDLE_HASH_SIZE (1 << 16)
> @@ -63,7 +63,7 @@ void class_handle_hash(struct portals_handle *h, char *owner)
> 	struct handle_bucket *bucket;
> 
> 	LASSERT(h);
> -	LASSERT(list_empty(&h->h_link));
> +	LASSERT(hlist_unhashed(&h->h_link));
> 
> 	/*
> 	 * This is fast, but simplistic cookie generation algorithm, it will
> @@ -89,8 +89,7 @@ void class_handle_hash(struct portals_handle *h, char *owner)
> 
> 	bucket = &handle_hash[h->h_cookie & HANDLE_HASH_MASK];
> 	spin_lock(&bucket->lock);
> -	list_add_rcu(&h->h_link, &bucket->head);
> -	h->h_in = 1;
> +	hlist_add_head_rcu(&h->h_link, &bucket->head);
> 	spin_unlock(&bucket->lock);
> 
> 	CDEBUG(D_INFO, "added object %p with handle %#llx to hash\n",
> @@ -100,7 +99,7 @@ EXPORT_SYMBOL(class_handle_hash);
> 
> static void class_handle_unhash_nolock(struct portals_handle *h)
> {
> -	if (list_empty(&h->h_link)) {
> +	if (hlist_unhashed(&h->h_link)) {
> 		CERROR("removing an already-removed handle (%#llx)\n",
> 		       h->h_cookie);
> 		return;
> @@ -110,13 +109,12 @@ static void class_handle_unhash_nolock(struct portals_handle *h)
> 	       h, h->h_cookie);
> 
> 	spin_lock(&h->h_lock);
> -	if (h->h_in == 0) {
> +	if (hlist_unhashed(&h->h_link)) {
> 		spin_unlock(&h->h_lock);
> 		return;
> 	}
> -	h->h_in = 0;
> +	hlist_del_init_rcu(&h->h_link);
> 	spin_unlock(&h->h_lock);
> -	list_del_rcu(&h->h_link);
> }
> 
> void class_handle_unhash(struct portals_handle *h)
> @@ -145,7 +143,7 @@ void *class_handle2object(u64 cookie, char *owner)
> 	bucket = handle_hash + (cookie & HANDLE_HASH_MASK);
> 
> 	rcu_read_lock();
> -	list_for_each_entry_rcu(h, &bucket->head, h_link) {
> +	hlist_for_each_entry_rcu(h, &bucket->head, h_link) {
> 		if (h->h_cookie != cookie || h->h_owner != owner)
> 			continue;
> 
> @@ -176,7 +174,7 @@ int class_handle_init(void)
> 	spin_lock_init(&handle_base_lock);
> 	for (bucket = handle_hash + HANDLE_HASH_SIZE - 1; bucket >= handle_hash;
> 	     bucket--) {
> -		INIT_LIST_HEAD(&bucket->head);
> +		INIT_HLIST_HEAD(&bucket->head);
> 		spin_lock_init(&bucket->lock);
> 	}
> 
> @@ -195,7 +193,7 @@ static int cleanup_all_handles(void)
> 		struct portals_handle *h;
> 
> 		spin_lock(&handle_hash[i].lock);
> -		list_for_each_entry_rcu(h, &handle_hash[i].head, h_link) {
> +		hlist_for_each_entry_rcu(h, &handle_hash[i].head, h_link) {
> 			CERROR("force clean handle %#llx addr %p owner %p\n",
> 			       h->h_cookie, h, h->h_owner);
> 
> 
> 

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud

[lustre-devel] [PATCH 18/28] lustre: remove unused fields from struct obd_device

[Andreas Dilger]

On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
> 
> One field is set but never access.
> Another is accessed but never set.
> Other are never mentioned at all.

Well, not on the client at least...

> Also remove the comment
> 	/* use separate field as it is set in interrupt to don't mess with
> 	 * protection of other bits using _bh lock
> 	 */
> 
> This is not correct - were it used, obd_recovery_expired would be part
> of the same 'unsigned long' as the other fields.

It probably should have been just an "unsigned long" rather than a bitfield.
In any case, it is no longer updated in interrupt context so it is irrelevant.

> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

> ---
> drivers/staging/lustre/lustre/include/obd.h        |   33 +++++++-------------
> drivers/staging/lustre/lustre/mgc/mgc_request.c    |    4 +-
> drivers/staging/lustre/lustre/obdclass/class_obd.c |    1 -
> .../staging/lustre/lustre/obdclass/obd_config.c    |    1 -
> 4 files changed, 14 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
> index 3bdde312e4a6..93a47cf0ef68 100644
> --- a/drivers/staging/lustre/lustre/include/obd.h
> +++ b/drivers/staging/lustre/lustre/include/obd.h
> @@ -539,27 +539,18 @@ struct obd_device {
> 	char			 obd_name[MAX_OBD_NAME];
> 
> 	/* bitfield modification is protected by obd_dev_lock */
> -	unsigned long obd_attached:1,      /* finished attach */
> -		      obd_set_up:1,	/* finished setup */
> -		      obd_version_recov:1, /* obd uses version checking */
> -		      obd_replayable:1,/* recovery is enabled; inform clients */
> -		      obd_no_transno:1,  /* no committed-transno notification */
> -		      obd_no_recov:1,      /* fail instead of retry messages */
> -		      obd_stopping:1,      /* started cleanup */
> -		      obd_starting:1,      /* started setup */
> -		      obd_force:1,	 /* cleanup with > 0 obd refcount */
> -		      obd_fail:1,	 /* cleanup with failover */
> -		      obd_no_conn:1,       /* deny new connections */
> -		      obd_inactive:1,      /* device active/inactive
> -					    * (for sysfs status only!!)
> -					    */
> -		      obd_no_ir:1,	 /* no imperative recovery. */
> -		      obd_process_conf:1,  /* device is processing mgs config */
> -		      obd_checksum_dump:1; /* dump pages upon cksum error */
> -	/* use separate field as it is set in interrupt to don't mess with
> -	 * protection of other bits using _bh lock
> -	 */
> -	unsigned long obd_recovery_expired:1;
> +	unsigned long obd_attached:1,	  /* finished attach */
> +		      obd_set_up:1,	  /* finished setup */
> +		      obd_no_recov:1,	  /* fail instead of retry messages */
> +		      obd_stopping:1,	  /* started cleanup */
> +		      obd_starting:1,	  /* started setup */
> +		      obd_force:1,	  /* cleanup with > 0 obd refcount */
> +		      obd_fail:1,	  /* cleanup with failover */
> +		      obd_inactive:1,	  /* device active/inactive
> +					   * (for sysfs status only!!)
> +					   */
> +		      obd_process_conf:1;/* device is processing mgs config */
> +
> 	/* uuid-export hash body */
> 	struct rhashtable	 obd_uuid_hash;
> 	wait_queue_head_t	 obd_refcount_waitq;
> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> index 5ce49e708287..6daadf24b894 100644
> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> @@ -991,10 +991,10 @@ static int mgc_set_info_async(const struct lu_env *env, struct obd_export *exp,
> 		if (vallen != sizeof(int))
> 			return -EINVAL;
> 		value = *(int *)val;
> -		CDEBUG(D_MGC, "InitRecov %s %d/d%d:i%d:r%d:or%d:%s\n",
> +		CDEBUG(D_MGC, "InitRecov %s %d/d%d:i%d:r%d:%s\n",
> 		       imp->imp_obd->obd_name, value,
> 		       imp->imp_deactive, imp->imp_invalid,
> -		       imp->imp_replayable, imp->imp_obd->obd_replayable,
> +		       imp->imp_replayable,
> 		       ptlrpc_import_state_name(imp->imp_state));
> 		/* Resurrect if we previously died */
> 		if ((imp->imp_state != LUSTRE_IMP_FULL &&
> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> index b8fc74044fe3..1fcbda128a58 100644
> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> @@ -525,7 +525,6 @@ int class_handle_ioctl(unsigned int cmd, unsigned long arg)
> 		}
> 		CDEBUG(D_HA, "%s: disabling committed-transno notification\n",
> 		       obd->obd_name);
> -		obd->obd_no_transno = 1;
> 		err = 0;
> 		break;
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> index 0cdadea4e63c..7b10206d6e52 100644
> --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> @@ -487,7 +487,6 @@ static int class_cleanup(struct obd_device *obd, struct lustre_cfg *lcfg)
> 				LCONSOLE_WARN("Failing over %s\n",
> 					      obd->obd_name);
> 				obd->obd_fail = 1;
> -				obd->obd_no_transno = 1;
> 				obd->obd_no_recov = 1;
> 				if (OBP(obd, iocontrol)) {
> 					obd_iocontrol(OBD_IOC_SYNC,
> 
> 

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud

[lustre-devel] [PATCH 11/28] lustre: handles: discard h_owner in favour of h_ops

[NeilBrown]

On Wed, Apr 03 2019, Andreas Dilger wrote:

> On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
>> 
>> lustre_handles  assigned a  64bit  unique identifier  (a 'cookie')  to
>> objects of  various types and  stored them  in a hash  table, allowing
>> them to be accessed by the cookie.
>> 
>> The is a facility for type checking by recording an 'owner' for each
>> object, and checking the owner on lookup.  Unfortunately this is not
>> used - owner is always zero.
>> 
>> Eahc object also contains an h_ops pointer which can be used to
>> reliably identify an owner.
>> 
>> So discard h_owner, pass and 'ops' pointer to class_handle2object(),
>> and only return objects for which the h_ops matches.
>> 
>> Signed-off-by: NeilBrown <neilb@suse.com>
>
> Probably a bit late to the party here, but it would be useful to add a
> portability note here, that the pointer to "struct mdt_export_data"
> that is currently stored in h_owner should move to "struct mdt_file_data"
> in the server code.

It is never to late to provide review!
The tricky think with notes it putting them somewhere that they'll
be read at the write time.
I've put this note in the commit message

    Note that server code uses h_owner slightly differently - it
    identifies not only the type but also the "struct mdt_export_data"
    that the cookie is associated with.
    This can be changed to store the mdt_export_data  pointer in
    the mdt_file_data, and  check it to validate the candidate returned by
    class_handle2object() finds a candidate.

Hopefully when someone (me?) ports the server code, they'll notice that
they cannot use h_owner the same way, check the commit which changed
things, and find this.

>
> Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

Thanks!

NeilBrown

>
>> ---
>> .../staging/lustre/lustre/include/lustre_handles.h |    7 +++----
>> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    2 +-
>> drivers/staging/lustre/lustre/obdclass/genops.c    |    3 ++-
>> .../lustre/lustre/obdclass/lustre_handles.c        |    6 +++---
>> 4 files changed, 9 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
>> index 683680891e4c..9a4b1a821e7b 100644
>> --- a/drivers/staging/lustre/lustre/include/lustre_handles.h
>> +++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
>> @@ -65,8 +65,7 @@ struct portals_handle_ops {
>> struct portals_handle {
>> 	struct list_head		h_link;
>> 	u64				h_cookie;
>> -	const void			*h_owner;
>> -	struct portals_handle_ops	*h_ops;
>> +	const struct portals_handle_ops	*h_ops;
>> 
>> 	/* newly added fields to handle the RCU issue. -jxiong */
>> 	struct rcu_head			h_rcu;
>> @@ -79,9 +78,9 @@ struct portals_handle {
>> 
>> /* Add a handle to the hash table */
>> void class_handle_hash(struct portals_handle *,
>> -		       struct portals_handle_ops *ops);
>> +		       const struct portals_handle_ops *ops);
>> void class_handle_unhash(struct portals_handle *);
>> -void *class_handle2object(u64 cookie, const void *owner);
>> +void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops);
>> void class_handle_free_cb(struct rcu_head *rcu);
>> int class_handle_init(void);
>> void class_handle_cleanup(void);
>> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
>> index 7ec5fc900da8..768cccc1fa82 100644
>> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
>> @@ -515,7 +515,7 @@ struct ldlm_lock *__ldlm_handle2lock(const struct lustre_handle *handle,
>> 
>> 	LASSERT(handle);
>> 
>> -	lock = class_handle2object(handle->cookie, NULL);
>> +	lock = class_handle2object(handle->cookie, &lock_handle_ops);
>> 	if (!lock)
>> 		return NULL;
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
>> index e206bb401fe3..42859fbca330 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
>> @@ -708,6 +708,7 @@ int obd_init_caches(void)
>> 	return -ENOMEM;
>> }
>> 
>> +static struct portals_handle_ops export_handle_ops;
>> /* map connection to client */
>> struct obd_export *class_conn2export(struct lustre_handle *conn)
>> {
>> @@ -724,7 +725,7 @@ struct obd_export *class_conn2export(struct lustre_handle *conn)
>> 	}
>> 
>> 	CDEBUG(D_INFO, "looking for export cookie %#llx\n", conn->cookie);
>> -	export = class_handle2object(conn->cookie, NULL);
>> +	export = class_handle2object(conn->cookie, &export_handle_ops);
>> 	return export;
>> }
>> EXPORT_SYMBOL(class_conn2export);
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
>> index 0674afb0059f..32b70d613f71 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
>> @@ -59,7 +59,7 @@ static struct handle_bucket {
>>  * global (per-node) hash-table.
>>  */
>> void class_handle_hash(struct portals_handle *h,
>> -		       struct portals_handle_ops *ops)
>> +		       const struct portals_handle_ops *ops)
>> {
>> 	struct handle_bucket *bucket;
>> 
>> @@ -132,7 +132,7 @@ void class_handle_unhash(struct portals_handle *h)
>> }
>> EXPORT_SYMBOL(class_handle_unhash);
>> 
>> -void *class_handle2object(u64 cookie, const void *owner)
>> +void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
>> {
>> 	struct handle_bucket *bucket;
>> 	struct portals_handle *h;
>> @@ -147,7 +147,7 @@ void *class_handle2object(u64 cookie, const void *owner)
>> 
>> 	rcu_read_lock();
>> 	list_for_each_entry_rcu(h, &bucket->head, h_link) {
>> -		if (h->h_cookie != cookie || h->h_owner != owner)
>> +		if (h->h_cookie != cookie || h->h_ops != ops)
>> 			continue;
>> 
>> 		spin_lock(&h->h_lock);
>> 
>> 
>
> 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/20190404/2cfa7a12/attachment.sig>

[lustre-devel] [PATCH 14/28] lustre: portals_handle: rename ops to owner

[NeilBrown]

On Wed, Apr 03 2019, Andreas Dilger wrote:

> On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
>> 
>> Now the portals_handle_ops contains only a char*,
>> it is functioning primarily to identify the owner of each handle.
>> So change the name to h_owner, and the type to char*.
>> 
>> Signed-off-by: NeilBrown <neilb@suse.com>
>
> Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

Thanks.
I've also added a note here:

    Note: this h_owner is now quiet different from the similar h_owner
    in the server code.  When server code it merged the
    "med" pointer should be stored in the "mfd" and validated separately.

NeilBrown

>
>> ---
>> .../staging/lustre/lustre/include/lustre_handles.h |   12 +++---------
>> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |    8 +++-----
>> drivers/staging/lustre/lustre/obdclass/genops.c    |   17 ++++++-----------
>> .../lustre/lustre/obdclass/lustre_handles.c        |   15 +++++++--------
>> 4 files changed, 19 insertions(+), 33 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/include/lustre_handles.h b/drivers/staging/lustre/lustre/include/lustre_handles.h
>> index 8fb42851f6d1..ebbbb01710e7 100644
>> --- a/drivers/staging/lustre/lustre/include/lustre_handles.h
>> +++ b/drivers/staging/lustre/lustre/include/lustre_handles.h
>> @@ -45,11 +45,6 @@
>> #include <linux/spinlock.h>
>> #include <linux/types.h>
>> 
>> -struct portals_handle_ops {
>> -	/* hop_type is used for some debugging messages */
>> -	char *hop_type;
>> -};
>> -
>> /* These handles are most easily used by having them appear at the very top of
>>  * whatever object that you want to make handles for.  ie:
>>  *
>> @@ -65,7 +60,7 @@ struct portals_handle_ops {
>> struct portals_handle {
>> 	struct list_head		h_link;
>> 	u64				h_cookie;
>> -	const struct portals_handle_ops	*h_ops;
>> +	char				*h_owner;
>> 	refcount_t			h_ref;
>> 
>> 	/* newly added fields to handle the RCU issue. -jxiong */
>> @@ -77,10 +72,9 @@ struct portals_handle {
>> /* handles.c */
>> 
>> /* Add a handle to the hash table */
>> -void class_handle_hash(struct portals_handle *,
>> -		       const struct portals_handle_ops *ops);
>> +void class_handle_hash(struct portals_handle *, char *owner);
>> void class_handle_unhash(struct portals_handle *);
>> -void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops);
>> +void *class_handle2object(u64 cookie, char *owner);
>> int class_handle_init(void);
>> void class_handle_cleanup(void);
>> 
>> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
>> index 18f018d27936..56a2d1dcd663 100644
>> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
>> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
>> @@ -363,9 +363,7 @@ void ldlm_lock_destroy_nolock(struct ldlm_lock *lock)
>> 	}
>> }
>> 
>> -static struct portals_handle_ops lock_handle_ops = {
>> -	.hop_type   = "ldlm",
>> -};
>> +static const char lock_handle_owner[] = "ldlm";
>> 
>> /**
>>  *
>> @@ -405,7 +403,7 @@ static struct ldlm_lock *ldlm_lock_new(struct ldlm_resource *resource)
>> 	lprocfs_counter_incr(ldlm_res_to_ns(resource)->ns_stats,
>> 			     LDLM_NSS_LOCKS);
>> 	INIT_LIST_HEAD(&lock->l_handle.h_link);
>> -	class_handle_hash(&lock->l_handle, &lock_handle_ops);
>> +	class_handle_hash(&lock->l_handle, lock_handle_owner);
>> 
>> 	lu_ref_init(&lock->l_reference);
>> 	lu_ref_add(&lock->l_reference, "hash", lock);
>> @@ -509,7 +507,7 @@ struct ldlm_lock *__ldlm_handle2lock(const struct lustre_handle *handle,
>> 
>> 	LASSERT(handle);
>> 
>> -	lock = class_handle2object(handle->cookie, &lock_handle_ops);
>> +	lock = class_handle2object(handle->cookie, lock_handle_owner);
>> 	if (!lock)
>> 		return NULL;
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c b/drivers/staging/lustre/lustre/obdclass/genops.c
>> index ed306bbfbfb8..e0da46e7d355 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
>> @@ -708,7 +708,8 @@ int obd_init_caches(void)
>> 	return -ENOMEM;
>> }
>> 
>> -static struct portals_handle_ops export_handle_ops;
>> +static const char export_handle_owner[] = "export";
>> +
>> /* map connection to client */
>> struct obd_export *class_conn2export(struct lustre_handle *conn)
>> {
>> @@ -725,7 +726,7 @@ struct obd_export *class_conn2export(struct lustre_handle *conn)
>> 	}
>> 
>> 	CDEBUG(D_INFO, "looking for export cookie %#llx\n", conn->cookie);
>> -	export = class_handle2object(conn->cookie, &export_handle_ops);
>> +	export = class_handle2object(conn->cookie, export_handle_owner);
>> 	return export;
>> }
>> EXPORT_SYMBOL(class_conn2export);
>> @@ -777,10 +778,6 @@ static void class_export_destroy(struct obd_export *exp)
>> 	kfree_rcu(exp, exp_handle.h_rcu);
>> }
>> 
>> -static struct portals_handle_ops export_handle_ops = {
>> -	.hop_type	= "export",
>> -};
>> -
>> struct obd_export *class_export_get(struct obd_export *exp)
>> {
>> 	refcount_inc(&exp->exp_handle.h_ref);
>> @@ -863,7 +860,7 @@ static struct obd_export *__class_new_export(struct obd_device *obd,
>> 	INIT_LIST_HEAD(&export->exp_req_replay_queue);
>> 	INIT_LIST_HEAD(&export->exp_handle.h_link);
>> 	INIT_LIST_HEAD(&export->exp_hp_rpcs);
>> -	class_handle_hash(&export->exp_handle, &export_handle_ops);
>> +	class_handle_hash(&export->exp_handle, export_handle_owner);
>> 	spin_lock_init(&export->exp_lock);
>> 	spin_lock_init(&export->exp_rpc_lock);
>> 	spin_lock_init(&export->exp_bl_list_lock);
>> @@ -967,9 +964,7 @@ static void class_import_destroy(struct obd_import *imp)
>> 	kfree_rcu(imp, imp_handle.h_rcu);
>> }
>> 
>> -static struct portals_handle_ops import_handle_ops = {
>> -	.hop_type	= "import",
>> -};
>> +static const char import_handle_owner[] = "import";
>> 
>> struct obd_import *class_import_get(struct obd_import *import)
>> {
>> @@ -1052,7 +1047,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
>> 	atomic_set(&imp->imp_inval_count, 0);
>> 	INIT_LIST_HEAD(&imp->imp_conn_list);
>> 	INIT_LIST_HEAD(&imp->imp_handle.h_link);
>> -	class_handle_hash(&imp->imp_handle, &import_handle_ops);
>> +	class_handle_hash(&imp->imp_handle, import_handle_owner);
>> 	init_imp_at(&imp->imp_at);
>> 
>> 	/* the default magic is V2, will be used in connect RPC, and
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
>> index 8aece57ec8c9..f41558ccdfcf 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
>> @@ -58,8 +58,7 @@ static struct handle_bucket {
>>  * Generate a unique 64bit cookie (hash) for a handle and insert it into
>>  * global (per-node) hash-table.
>>  */
>> -void class_handle_hash(struct portals_handle *h,
>> -		       const struct portals_handle_ops *ops)
>> +void class_handle_hash(struct portals_handle *h, char *owner)
>> {
>> 	struct handle_bucket *bucket;
>> 
>> @@ -85,7 +84,7 @@ void class_handle_hash(struct portals_handle *h,
>> 	h->h_cookie = handle_base;
>> 	spin_unlock(&handle_base_lock);
>> 
>> -	h->h_ops = ops;
>> +	h->h_owner = owner;
>> 	spin_lock_init(&h->h_lock);
>> 
>> 	bucket = &handle_hash[h->h_cookie & HANDLE_HASH_MASK];
>> @@ -132,7 +131,7 @@ void class_handle_unhash(struct portals_handle *h)
>> }
>> EXPORT_SYMBOL(class_handle_unhash);
>> 
>> -void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
>> +void *class_handle2object(u64 cookie, char *owner)
>> {
>> 	struct handle_bucket *bucket;
>> 	struct portals_handle *h;
>> @@ -147,14 +146,14 @@ void *class_handle2object(u64 cookie, const struct portals_handle_ops *ops)
>> 
>> 	rcu_read_lock();
>> 	list_for_each_entry_rcu(h, &bucket->head, h_link) {
>> -		if (h->h_cookie != cookie || h->h_ops != ops)
>> +		if (h->h_cookie != cookie || h->h_owner != owner)
>> 			continue;
>> 
>> 		spin_lock(&h->h_lock);
>> 		if (likely(h->h_in != 0)) {
>> 			refcount_inc(&h->h_ref);
>> 			CDEBUG(D_INFO, "GET %s %p refcount=%d\n",
>> -			       h->h_ops->hop_type, h,
>> +			       h->h_owner, h,
>> 			       refcount_read(&h->h_ref));
>> 			retval = h;
>> 		}
>> @@ -201,8 +200,8 @@ static int cleanup_all_handles(void)
>> 
>> 		spin_lock(&handle_hash[i].lock);
>> 		list_for_each_entry_rcu(h, &handle_hash[i].head, h_link) {
>> -			CERROR("force clean handle %#llx addr %p ops %p\n",
>> -			       h->h_cookie, h, h->h_ops);
>> +			CERROR("force clean handle %#llx addr %p owner %p\n",
>> +			       h->h_cookie, h, h->h_owner);
>> 
>> 			class_handle_unhash_nolock(h);
>> 			rc++;
>> 
>> 
>
> Cheers, Andreas
> ---
> Andreas Dilger
> Principal Lustre Architect
> 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/20190404/bdd8c89c/attachment.sig>

[lustre-devel] [PATCH 18/28] lustre: remove unused fields from struct obd_device

[NeilBrown]

On Wed, Apr 03 2019, Andreas Dilger wrote:

> On Mar 3, 2019, at 23:31, NeilBrown <neilb@suse.com> wrote:
>> 
>> One field is set but never access.
>> Another is accessed but never set.
>> Other are never mentioned at all.
>
> Well, not on the client at least...

Yes ... there will be lots to (review and) put back when we add the
server.

>
>> Also remove the comment
>> 	/* use separate field as it is set in interrupt to don't mess with
>> 	 * protection of other bits using _bh lock
>> 	 */
>> 
>> This is not correct - were it used, obd_recovery_expired would be part
>> of the same 'unsigned long' as the other fields.
>
> It probably should have been just an "unsigned long" rather than a bitfield.
> In any case, it is no longer updated in interrupt context so it is irrelevant.
>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>
> Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

Thanks,
NeilBrown

>
>> ---
>> drivers/staging/lustre/lustre/include/obd.h        |   33 +++++++-------------
>> drivers/staging/lustre/lustre/mgc/mgc_request.c    |    4 +-
>> drivers/staging/lustre/lustre/obdclass/class_obd.c |    1 -
>> .../staging/lustre/lustre/obdclass/obd_config.c    |    1 -
>> 4 files changed, 14 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
>> index 3bdde312e4a6..93a47cf0ef68 100644
>> --- a/drivers/staging/lustre/lustre/include/obd.h
>> +++ b/drivers/staging/lustre/lustre/include/obd.h
>> @@ -539,27 +539,18 @@ struct obd_device {
>> 	char			 obd_name[MAX_OBD_NAME];
>> 
>> 	/* bitfield modification is protected by obd_dev_lock */
>> -	unsigned long obd_attached:1,      /* finished attach */
>> -		      obd_set_up:1,	/* finished setup */
>> -		      obd_version_recov:1, /* obd uses version checking */
>> -		      obd_replayable:1,/* recovery is enabled; inform clients */
>> -		      obd_no_transno:1,  /* no committed-transno notification */
>> -		      obd_no_recov:1,      /* fail instead of retry messages */
>> -		      obd_stopping:1,      /* started cleanup */
>> -		      obd_starting:1,      /* started setup */
>> -		      obd_force:1,	 /* cleanup with > 0 obd refcount */
>> -		      obd_fail:1,	 /* cleanup with failover */
>> -		      obd_no_conn:1,       /* deny new connections */
>> -		      obd_inactive:1,      /* device active/inactive
>> -					    * (for sysfs status only!!)
>> -					    */
>> -		      obd_no_ir:1,	 /* no imperative recovery. */
>> -		      obd_process_conf:1,  /* device is processing mgs config */
>> -		      obd_checksum_dump:1; /* dump pages upon cksum error */
>> -	/* use separate field as it is set in interrupt to don't mess with
>> -	 * protection of other bits using _bh lock
>> -	 */
>> -	unsigned long obd_recovery_expired:1;
>> +	unsigned long obd_attached:1,	  /* finished attach */
>> +		      obd_set_up:1,	  /* finished setup */
>> +		      obd_no_recov:1,	  /* fail instead of retry messages */
>> +		      obd_stopping:1,	  /* started cleanup */
>> +		      obd_starting:1,	  /* started setup */
>> +		      obd_force:1,	  /* cleanup with > 0 obd refcount */
>> +		      obd_fail:1,	  /* cleanup with failover */
>> +		      obd_inactive:1,	  /* device active/inactive
>> +					   * (for sysfs status only!!)
>> +					   */
>> +		      obd_process_conf:1;/* device is processing mgs config */
>> +
>> 	/* uuid-export hash body */
>> 	struct rhashtable	 obd_uuid_hash;
>> 	wait_queue_head_t	 obd_refcount_waitq;
>> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> index 5ce49e708287..6daadf24b894 100644
>> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>> @@ -991,10 +991,10 @@ static int mgc_set_info_async(const struct lu_env *env, struct obd_export *exp,
>> 		if (vallen != sizeof(int))
>> 			return -EINVAL;
>> 		value = *(int *)val;
>> -		CDEBUG(D_MGC, "InitRecov %s %d/d%d:i%d:r%d:or%d:%s\n",
>> +		CDEBUG(D_MGC, "InitRecov %s %d/d%d:i%d:r%d:%s\n",
>> 		       imp->imp_obd->obd_name, value,
>> 		       imp->imp_deactive, imp->imp_invalid,
>> -		       imp->imp_replayable, imp->imp_obd->obd_replayable,
>> +		       imp->imp_replayable,
>> 		       ptlrpc_import_state_name(imp->imp_state));
>> 		/* Resurrect if we previously died */
>> 		if ((imp->imp_state != LUSTRE_IMP_FULL &&
>> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> index b8fc74044fe3..1fcbda128a58 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> @@ -525,7 +525,6 @@ int class_handle_ioctl(unsigned int cmd, unsigned long arg)
>> 		}
>> 		CDEBUG(D_HA, "%s: disabling committed-transno notification\n",
>> 		       obd->obd_name);
>> -		obd->obd_no_transno = 1;
>> 		err = 0;
>> 		break;
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
>> index 0cdadea4e63c..7b10206d6e52 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
>> @@ -487,7 +487,6 @@ static int class_cleanup(struct obd_device *obd, struct lustre_cfg *lcfg)
>> 				LCONSOLE_WARN("Failing over %s\n",
>> 					      obd->obd_name);
>> 				obd->obd_fail = 1;
>> -				obd->obd_no_transno = 1;
>> 				obd->obd_no_recov = 1;
>> 				if (OBP(obd, iocontrol)) {
>> 					obd_iocontrol(OBD_IOC_SYNC,
>> 
>> 
>
> Cheers, Andreas
> ---
> Andreas Dilger
> Principal Lustre Architect
> 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/20190404/542ffcc4/attachment-0001.sig>