From: NeilBrown Date: Sun, 4 Nov 2018 20:42:51 +0000 (-0500) Subject: LU-11617 mdc: fix possible deadlock in chlg_open() X-Git-Tag: 2.12.57~42 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=206b21741b07a10269bbcfdac28743591b64ab2f 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. Change-Id: Ibe099f23a08fa3ef846d4ef8fcb85659f8f99245 Signed-off-by: NeilBrown Signed-off-by: James Simmons Reviewed-on: https://review.whamcloud.com/33572 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Quentin Bouget Reviewed-by: Andreas Dilger Reviewed-by: Mike Pershin Reviewed-by: Oleg Drokin --- diff --git a/lustre/mdc/mdc_changelog.c b/lustre/mdc/mdc_changelog.c index 4eeac84..fe1ef6e 100644 --- a/lustre/mdc/mdc_changelog.c +++ b/lustre/mdc/mdc_changelog.c @@ -484,31 +484,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. @@ -518,13 +493,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) @@ -729,13 +708,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 */