kobject tricky lifetime races fixed

git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@1323 d57e44dd-8a1f-0410-8b47-8ef2f437770f

scst/include/scst.h

@@ -1145,6 +1145,12 @@ struct scst_tgt {
 	/* Set if tgt_kobj was initialized */
 	unsigned int tgt_kobj_initialized:1;
 
+	/*
+	 * Used to protect sysfs attributes to be called after this
+	 * object was unregistered.
+	 */
+	struct rw_semaphore tgt_attr_rwsem;
+
 	struct kobject tgt_kobj; /* main targets/target kobject */
 	struct kobject *tgt_sess_kobj; /* target/sessions/ */
 	struct kobject *tgt_luns_kobj; /* target/luns/ */
@@ -1274,6 +1280,12 @@ struct scst_session {
 	/* Set if sess_kobj was initialized */
 	unsigned int sess_kobj_initialized:1;
 
+	/*
+	 * Used to protect sysfs attributes to be called after this
+	 * object was unregistered.
+	 */
+	struct rw_semaphore sess_attr_rwsem;
+
 	struct kobject sess_kobj; /* kobject for this struct */
 
 	/*
@@ -1793,6 +1805,12 @@ struct scst_device {
 	/* Set if tgt_kobj was initialized */
 	unsigned int dev_kobj_initialized:1;
 
+	/*
+	 * Used to protect sysfs attributes to be called after this
+	 * object was unregistered.
+	 */
+	struct rw_semaphore dev_attr_rwsem;
+
 	struct kobject dev_kobj; /* kobject for this struct */
 	struct kobject *dev_exp_kobj; /* exported groups */
 
@@ -2008,8 +2026,10 @@ struct scst_aen {
 #endif
 
 /*
- * Registers target template
- * Returns 0 on success or appropriate error code otherwise
+ * Registers target template.
+ * Returns 0 on success or appropriate error code otherwise.
+ *
+ * Note: *vtt must be static!
  */
 int __scst_register_target_template(struct scst_tgt_template *vtt,
 	const char *version);
@@ -2120,8 +2140,10 @@ void scst_unregister_session(struct scst_session *sess, int wait,
 	void (*unreg_done_fn) (struct scst_session *sess));
 
 /*
- * Registers dev handler driver
- * Returns 0 on success or appropriate error code otherwise
+ * Registers dev handler driver.
+ * Returns 0 on success or appropriate error code otherwise.
+ *
+ * Note: *dev_type must be static!
  */
 int __scst_register_dev_driver(struct scst_dev_type *dev_type,
 	const char *version);
@@ -2136,8 +2158,10 @@ static inline int scst_register_dev_driver(struct scst_dev_type *dev_type)
 void scst_unregister_dev_driver(struct scst_dev_type *dev_type);
 
 /*
- * Registers dev handler driver for virtual devices (eg VDISK)
- * Returns 0 on success or appropriate error code otherwise
+ * Registers dev handler driver for virtual devices (eg VDISK).
+ * Returns 0 on success or appropriate error code otherwise.
+ *
+ * Note: *dev_type must be static!
  */
 int __scst_register_virtual_dev_driver(struct scst_dev_type *dev_type,
 	const char *version);

scst/src/scst_mem.c

@@ -1438,8 +1438,6 @@ EXPORT_SYMBOL(sgv_pool_flush);
 
 static void sgv_pool_deinit_put(struct sgv_pool *pool)
 {
-	int i;
-
 	TRACE_ENTRY();
 
 	cancel_delayed_work_sync(&pool->sgv_purge_work);
@@ -1452,12 +1450,6 @@ static void sgv_pool_deinit_put(struct sgv_pool *pool)
 	spin_unlock_bh(&sgv_pools_lock);
 	mutex_unlock(&sgv_pools_mutex);
 
-	for (i = 0; i < pool->max_caches; i++) {
-		if (pool->caches[i])
-			kmem_cache_destroy(pool->caches[i]);
-		pool->caches[i] = NULL;
-	}
-
 	scst_sgv_sysfs_put(pool);
 
 	/* pool can be dead here */
@@ -1541,8 +1533,16 @@ EXPORT_SYMBOL(sgv_pool_create);
 
 void sgv_pool_destroy(struct sgv_pool *pool)
 {
+	int i;
+
 	TRACE_ENTRY();
 
+	for (i = 0; i < pool->max_caches; i++) {
+		if (pool->caches[i])
+			kmem_cache_destroy(pool->caches[i]);
+		pool->caches[i] = NULL;
+	}
+
 	kfree(pool);
 
 	TRACE_EXIT();

scst/src/scst_sysfs.c

@@ -280,14 +280,70 @@ static void scst_tgt_release(struct kobject *kobj)
 
 	tgt = container_of(kobj, struct scst_tgt, tgt_kobj);
 
+	/* Let's make lockdep happy */
+	up_write(&tgt->tgt_attr_rwsem);
+
 	scst_free_tgt(tgt);
 
 	TRACE_EXIT();
 	return;
 }
 
+static ssize_t scst_tgt_attr_show(struct kobject *kobj, struct attribute *attr,
+			 char *buf)
+{
+	int res;
+	struct kobj_attribute *kobj_attr;
+	struct scst_tgt *tgt;
+
+	tgt = container_of(kobj, struct scst_tgt, tgt_kobj);
+
+	if (down_read_trylock(&tgt->tgt_attr_rwsem) == 0) {
+		res = -ENOENT;
+		goto out;
+	}
+
+	kobj_attr = container_of(attr, struct kobj_attribute, attr);
+
+	res = kobj_attr->show(kobj, kobj_attr, buf);
+
+	up_read(&tgt->tgt_attr_rwsem);
+
+out:
+	return res;
+}
+
+static ssize_t scst_tgt_attr_store(struct kobject *kobj, struct attribute *attr,
+			  const char *buf, size_t count)
+{
+	int res;
+	struct kobj_attribute *kobj_attr;
+	struct scst_tgt *tgt;
+
+	tgt = container_of(kobj, struct scst_tgt, tgt_kobj);
+
+	if (down_read_trylock(&tgt->tgt_attr_rwsem) == 0) {
+		res = -ENOENT;
+		goto out;
+	}
+
+	kobj_attr = container_of(attr, struct kobj_attribute, attr);
+
+	res = kobj_attr->store(kobj, kobj_attr, buf, count);
+
+	up_read(&tgt->tgt_attr_rwsem);
+
+out:
+	return res;
+}
+
+static struct sysfs_ops scst_tgt_sysfs_ops = {
+	.show = scst_tgt_attr_show,
+	.store = scst_tgt_attr_store,
+};
+
 static struct kobj_type tgt_ktype = {
-	.sysfs_ops = &scst_sysfs_ops,
+	.sysfs_ops = &scst_tgt_sysfs_ops,
 	.release = scst_tgt_release,
 };
 
@@ -350,6 +406,8 @@ int scst_create_tgt_sysfs(struct scst_tgt *tgt)
 
 	TRACE_ENTRY();
 
+	init_rwsem(&tgt->tgt_attr_rwsem);
+
 	tgt->tgt_kobj_initialized = 1;
 
 	retval = kobject_init_and_add(&tgt->tgt_kobj, &tgt_ktype,
@@ -441,6 +499,8 @@ void scst_tgt_sysfs_put(struct scst_tgt *tgt)
 		kobject_put(tgt->tgt_ini_grp_kobj);
 
 		kobject_del(&tgt->tgt_kobj);
+
+		down_write(&tgt->tgt_attr_rwsem);
 		kobject_put(&tgt->tgt_kobj);
 	} else
 		scst_free_tgt(tgt);
@@ -483,6 +543,9 @@ static void scst_sysfs_device_release(struct kobject *kobj)
 
 	dev = container_of(kobj, struct scst_device, dev_kobj);
 
+	/* Let's make lockdep happy */
+	up_write(&dev->dev_attr_rwsem);
+
 	scst_free_device(dev);
 
 	TRACE_EXIT();
@@ -558,8 +621,61 @@ out:
 	return;
 }
 
+static ssize_t scst_dev_attr_show(struct kobject *kobj, struct attribute *attr,
+			 char *buf)
+{
+	int res;
+	struct kobj_attribute *kobj_attr;
+	struct scst_device *dev;
+
+	dev = container_of(kobj, struct scst_device, dev_kobj);
+
+	if (down_read_trylock(&dev->dev_attr_rwsem) == 0) {
+		res = -ENOENT;
+		goto out;
+	}
+
+	kobj_attr = container_of(attr, struct kobj_attribute, attr);
+
+	res = kobj_attr->show(kobj, kobj_attr, buf);
+
+	up_read(&dev->dev_attr_rwsem);
+
+out:
+	return res;
+}
+
+static ssize_t scst_dev_attr_store(struct kobject *kobj, struct attribute *attr,
+			  const char *buf, size_t count)
+{
+	int res;
+	struct kobj_attribute *kobj_attr;
+	struct scst_device *dev;
+
+	dev = container_of(kobj, struct scst_device, dev_kobj);
+
+	if (down_read_trylock(&dev->dev_attr_rwsem) == 0) {
+		res = -ENOENT;
+		goto out;
+	}
+
+	kobj_attr = container_of(attr, struct kobj_attribute, attr);
+
+	res = kobj_attr->store(kobj, kobj_attr, buf, count);
+
+	up_read(&dev->dev_attr_rwsem);
+
+out:
+	return res;
+}
+
+static struct sysfs_ops scst_dev_sysfs_ops = {
+	.show = scst_dev_attr_show,
+	.store = scst_dev_attr_store,
+};
+
 static struct kobj_type scst_device_ktype = {
-	.sysfs_ops = &scst_sysfs_ops,
+	.sysfs_ops = &scst_dev_sysfs_ops,
 	.release = scst_sysfs_device_release,
 	.default_attrs = scst_device_attrs,
 };
@@ -570,6 +686,8 @@ int scst_create_device_sysfs(struct scst_device *dev)
 
 	TRACE_ENTRY();
 
+	init_rwsem(&dev->dev_attr_rwsem);
+
 	dev->dev_kobj_initialized = 1;
 
 	retval = kobject_init_and_add(&dev->dev_kobj, &scst_device_ktype,
@@ -618,6 +736,8 @@ void scst_device_sysfs_put(struct scst_device *dev)
 			kobject_put(dev->dev_exp_kobj);
 		}
 		kobject_del(&dev->dev_kobj);
+
+		down_write(&dev->dev_attr_rwsem);
 		kobject_put(&dev->dev_kobj);
 	} else
 		scst_free_device(dev);
@@ -671,14 +791,70 @@ static void scst_sysfs_session_release(struct kobject *kobj)
 
 	sess = container_of(kobj, struct scst_session, sess_kobj);
 
+	/* Let's make lockdep happy */
+	up_write(&sess->sess_attr_rwsem);
+
 	scst_release_session(sess);
 
 	TRACE_EXIT();
 	return;
 }
 
+static ssize_t scst_sess_attr_show(struct kobject *kobj, struct attribute *attr,
+			 char *buf)
+{
+	int res;
+	struct kobj_attribute *kobj_attr;
+	struct scst_session *sess;
+
+	sess = container_of(kobj, struct scst_session, sess_kobj);
+
+	if (down_read_trylock(&sess->sess_attr_rwsem) == 0) {
+		res = -ENOENT;
+		goto out;
+	}
+
+	kobj_attr = container_of(attr, struct kobj_attribute, attr);
+
+	res = kobj_attr->show(kobj, kobj_attr, buf);
+
+	up_read(&sess->sess_attr_rwsem);
+
+out:
+	return res;
+}
+
+static ssize_t scst_sess_attr_store(struct kobject *kobj, struct attribute *attr,
+			  const char *buf, size_t count)
+{
+	int res;
+	struct kobj_attribute *kobj_attr;
+	struct scst_session *sess;
+
+	sess = container_of(kobj, struct scst_session, sess_kobj);
+
+	if (down_read_trylock(&sess->sess_attr_rwsem) == 0) {
+		res = -ENOENT;
+		goto out;
+	}
+
+	kobj_attr = container_of(attr, struct kobj_attribute, attr);
+
+	res = kobj_attr->store(kobj, kobj_attr, buf, count);
+
+	up_read(&sess->sess_attr_rwsem);
+
+out:
+	return res;
+}
+
+static struct sysfs_ops scst_sess_sysfs_ops = {
+	.show = scst_sess_attr_show,
+	.store = scst_sess_attr_store,
+};
+
 static struct kobj_type scst_session_ktype = {
-	.sysfs_ops = &scst_sysfs_ops,
+	.sysfs_ops = &scst_sess_sysfs_ops,
 	.release = scst_sysfs_session_release,
 	.default_attrs = scst_session_attrs,
 };
@@ -699,7 +875,7 @@ restart:
 		if (!s->sess_kobj_initialized)
 			continue;
 
-		if (strcmp(name, s->sess_kobj.name) == 0) {
+		if (strcmp(name, kobject_name(&s->sess_kobj)) == 0) {
 			if (s == sess)
 				continue;
 
@@ -723,6 +899,8 @@ restart:
 		}
 	}
 
+	init_rwsem(&sess->sess_attr_rwsem);
+
 	sess->sess_kobj_initialized = 1;
 
 	retval = kobject_init_and_add(&sess->sess_kobj, &scst_session_ktype,
@@ -765,6 +943,8 @@ void scst_sess_sysfs_put(struct scst_session *sess)
 
 	if (sess->sess_kobj_initialized) {
 		kobject_del(&sess->sess_kobj);
+
+		down_write(&sess->sess_attr_rwsem);
 		kobject_put(&sess->sess_kobj);
 	} else
 		scst_release_session(sess);
@@ -1854,6 +2034,14 @@ void __exit scst_sysfs_cleanup(void)
 	kobject_put(&scst_sysfs_root_kobj);
 
 	wait_for_completion(&scst_sysfs_root_release_completion);
+	/*
+	 * There is a race, when in the release() schedule happens just after
+	 * calling complete(), so if we exit and unload scst module immediately,
+	 * there will be oops there. So let's give it a chance to quit
+	 * gracefully. Unfortunately, current kobjects implementation
+	 * doesn't allow better ways to handle it.
+	 */
+	msleep(3000);
 
 	PRINT_INFO("%s", "Exiting SCST sysfs hierarchy done");