LU-11617 mdc: fix possible deadlock in chlg_open()

Lockdep reports a possible deadlock between chlg_open() and
mdc_changelog_cdev_init()

mdc_changelog_cdev_init() takes chlg_registered_dev_lock and then
calls misc_register() which takes misc_mtx.
chlg_open() is called while misc_mtx is held, and tries to take
chlg_registered_dev_lock.
If these two functions race, a deadlock can occur as each thread will
hold one of the locks while trying to take the other.

chlg_open() does not need to take a lock.  It only uses the
lock to stablize a list while looking for the matching
chlg_registered_dev, and this can be found directly by examining
file->private_data.

So remove chlg_obd_get(), and use file->private_data to find the
obd_device.
Also ensure the device is fully initialized before calling
misc_register().  This means setting up some list linkage before the
call, and tearing it down if there is an error.

Lustre-change: https://review.whamcloud.com/33572
Lustre-commit: 206b21741b07a10269bbcfdac28743591b64ab2f

Change-Id: Icffdebcee656ee6199297ba2a28ba57dcbc51ae1
Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Quentin Bouget <quentin.bouget@cea.fr>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/36230
Reviewed-by: Neil Brown <neilb@suse.de>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/mdc/mdc_changelog.c b/lustre/mdc/mdc_changelog.c
index 833588b..11bf370 100644 (file)
--- a/lustre/mdc/mdc_changelog.c
+++ b/lustre/mdc/mdc_changelog.c
@@ -466,31 +466,6 @@ out_kbuf:
 }
 
 /**
- * Find the OBD device associated to a changelog character device.
- * @param[in]  cdev  character device instance descriptor
- * @return corresponding OBD device or NULL if none was found.
- */
-static struct obd_device *chlg_obd_get(dev_t cdev)
-{
-       int minor = MINOR(cdev);
-       struct obd_device *obd = NULL;
-       struct chlg_registered_dev *curr;
-
-       mutex_lock(&chlg_registered_dev_lock);
-       list_for_each_entry(curr, &chlg_registered_devices, ced_link) {
-               if (curr->ced_misc.minor == minor) {
-                       /* take the first available OBD device attached */
-                       obd = list_first_entry(&curr->ced_obds,
-                                              struct obd_device,
-                                              u.cli.cl_chg_dev_linkage);
-                       break;
-               }
-       }
-       mutex_unlock(&chlg_registered_dev_lock);
-       return obd;
-}
-
-/**
  * Open handler, initialize internal CRS state and spawn prefetch thread if
  * needed.
  * @param[in]  inode  Inode struct for the open character device.
@@ -500,13 +475,17 @@ static struct obd_device *chlg_obd_get(dev_t cdev)
 static int chlg_open(struct inode *inode, struct file *file)
 {
        struct chlg_reader_state *crs;
-       struct obd_device *obd = chlg_obd_get(inode->i_rdev);
+       struct miscdevice *misc = file->private_data;
+       struct chlg_registered_dev *dev;
+       struct obd_device *obd;
        struct task_struct *task;
        int rc;
        ENTRY;
 
-       if (!obd)
-               RETURN(-ENODEV);
+       dev = container_of(misc, struct chlg_registered_dev, ced_misc);
+       obd = list_first_entry(&dev->ced_obds,
+                              struct obd_device,
+                              u.cli.cl_chg_dev_linkage);
 
        OBD_ALLOC_PTR(crs);
        if (!crs)
@@ -695,13 +674,16 @@ int mdc_changelog_cdev_init(struct obd_device *obd)
                GOTO(out_unlock, rc = 0);
        }
 
+       list_add_tail(&obd->u.cli.cl_chg_dev_linkage, &entry->ced_obds);
+       list_add_tail(&entry->ced_link, &chlg_registered_devices);
+
        /* Register new character device */
        rc = misc_register(&entry->ced_misc);
-       if (rc != 0)
+       if (rc != 0) {
+               list_del_init(&obd->u.cli.cl_chg_dev_linkage);
+               list_del(&entry->ced_link);
                GOTO(out_unlock, rc);
-
-       list_add_tail(&obd->u.cli.cl_chg_dev_linkage, &entry->ced_obds);
-       list_add_tail(&entry->ced_link, &chlg_registered_devices);
+       }
 
        entry = NULL;   /* prevent it from being freed below */