kobject tricky lifetime races fixed
authorvlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Fri, 6 Nov 2009 18:39:52 +0000 (18:39 +0000)
committervlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Fri, 6 Nov 2009 18:39:52 +0000 (18:39 +0000)
git-svn-id: https://scst.svn.sourceforge.net/svnroot/scst/trunk@1323 d57e44dd-8a1f-0410-8b47-8ef2f437770f

scst/include/scst.h
scst/src/scst_mem.c
scst/src/scst_sysfs.c

index ed68cf0..096ce5e 100644 (file)
@@ -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);
index 23be97a..25baf09 100644 (file)
@@ -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();
index c13a812..c4d7a59 100644 (file)
@@ -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");