From 1bab0de0274fbe8c8ac92179e6705584c55ed169 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 27 Aug 2015 14:16:54 +0200 Subject: [PATCH] dm-mpath, scsi_dh: don't let dm detach device handlers While allowing dm-mpath to attach device handlers is a functionality we need for backwards compatibility reason there is no reason to reference count them and detach them if dm-mpath stops using the device for some reason. If the device handler works for the given device it can just stay attached, and we can take the retain_hw_handler codepath. Signed-off-by: Christoph Hellwig Acked-by: Mike Snitzer Acked-by: Hannes Reinecke Signed-off-by: James Bottomley --- drivers/md/dm-mpath.c | 21 ++---- drivers/scsi/device_handler/scsi_dh.c | 98 +++++++-------------------- include/scsi/scsi_device.h | 1 - include/scsi/scsi_dh.h | 5 -- 4 files changed, 29 insertions(+), 96 deletions(-) diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index eff7bdd7731d..a9f58fdb5d69 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -159,12 +159,9 @@ static struct priority_group *alloc_priority_group(void) static void free_pgpaths(struct list_head *pgpaths, struct dm_target *ti) { struct pgpath *pgpath, *tmp; - struct multipath *m = ti->private; list_for_each_entry_safe(pgpath, tmp, pgpaths, list) { list_del(&pgpath->list); - if (m->hw_handler_name) - scsi_dh_detach(bdev_get_queue(pgpath->path.dev->bdev)); dm_put_device(ti, pgpath->path.dev); free_pgpath(pgpath); } @@ -580,6 +577,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps q = bdev_get_queue(p->path.dev->bdev); if (m->retain_attached_hw_handler) { +retain: attached_handler_name = scsi_dh_attached_handler_name(q, GFP_KERNEL); if (attached_handler_name) { /* @@ -599,20 +597,14 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps } if (m->hw_handler_name) { - /* - * Increments scsi_dh reference, even when using an - * already-attached handler. - */ r = scsi_dh_attach(q, m->hw_handler_name); if (r == -EBUSY) { - /* - * Already attached to different hw_handler: - * try to reattach with correct one. - */ - scsi_dh_detach(q); - r = scsi_dh_attach(q, m->hw_handler_name); - } + char b[BDEVNAME_SIZE]; + printk(KERN_INFO "dm-mpath: retaining handler on device %s\n", + bdevname(p->path.dev->bdev, b)); + goto retain; + } if (r < 0) { ti->error = "error attaching hardware handler"; dm_put_device(ti, p->path.dev); @@ -624,7 +616,6 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps if (r < 0) { ti->error = "unable to set hardware " "handler parameters"; - scsi_dh_detach(q); dm_put_device(ti, p->path.dev); goto bad; } diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c index 1efebc9eedfb..869b5bd1ad25 100644 --- a/drivers/scsi/device_handler/scsi_dh.c +++ b/drivers/scsi/device_handler/scsi_dh.c @@ -100,14 +100,6 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev, { struct scsi_dh_data *d; - if (sdev->scsi_dh_data) { - if (sdev->scsi_dh_data->scsi_dh != scsi_dh) - return -EBUSY; - - kref_get(&sdev->scsi_dh_data->kref); - return 0; - } - if (!try_module_get(scsi_dh->module)) return -EINVAL; @@ -120,7 +112,6 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev, } d->scsi_dh = scsi_dh; - kref_init(&d->kref); d->sdev = sdev; spin_lock_irq(sdev->request_queue->queue_lock); @@ -129,12 +120,14 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev, return 0; } -static void __detach_handler (struct kref *kref) +/* + * scsi_dh_handler_detach - Detach a device handler from a device + * @sdev - SCSI device the device handler should be detached from + */ +static void scsi_dh_handler_detach(struct scsi_device *sdev) { - struct scsi_dh_data *scsi_dh_data = - container_of(kref, struct scsi_dh_data, kref); + struct scsi_dh_data *scsi_dh_data = sdev->scsi_dh_data; struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh; - struct scsi_device *sdev = scsi_dh_data->sdev; scsi_dh->detach(sdev); @@ -146,30 +139,6 @@ static void __detach_handler (struct kref *kref) module_put(scsi_dh->module); } -/* - * scsi_dh_handler_detach - Detach a device handler from a device - * @sdev - SCSI device the device handler should be detached from - * @scsi_dh - Device handler to be detached - * - * Detach from a device handler. If a device handler is specified, - * only detach if the currently attached handler matches @scsi_dh. - */ -static void scsi_dh_handler_detach(struct scsi_device *sdev, - struct scsi_device_handler *scsi_dh) -{ - if (!sdev->scsi_dh_data) - return; - - if (scsi_dh && scsi_dh != sdev->scsi_dh_data->scsi_dh) - return; - - if (!scsi_dh) - scsi_dh = sdev->scsi_dh_data->scsi_dh; - - if (scsi_dh) - kref_put(&sdev->scsi_dh_data->kref, __detach_handler); -} - /* * Functions for sysfs attribute 'dh_state' */ @@ -198,7 +167,7 @@ store_dh_state(struct device *dev, struct device_attribute *attr, /* * Detach from a device handler */ - scsi_dh_handler_detach(sdev, scsi_dh); + scsi_dh_handler_detach(sdev); err = 0; } else if (!strncmp(buf, "activate", 8)) { /* @@ -290,7 +259,8 @@ static int scsi_dh_notifier(struct notifier_block *nb, err = scsi_dh_handler_attach(sdev, devinfo); } else if (action == BUS_NOTIFY_DEL_DEVICE) { device_remove_file(dev, &scsi_dh_state_attr); - scsi_dh_handler_detach(sdev, NULL); + if (sdev->scsi_dh_data) + scsi_dh_handler_detach(sdev); } return err; } @@ -335,7 +305,8 @@ static int scsi_dh_notifier_remove(struct device *dev, void *data) sdev = to_scsi_device(dev); - scsi_dh_handler_detach(sdev, scsi_dh); + if (sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh == scsi_dh) + scsi_dh_handler_detach(sdev); put_device(dev); @@ -517,46 +488,23 @@ int scsi_dh_attach(struct request_queue *q, const char *name) err = -ENODEV; spin_unlock_irqrestore(q->queue_lock, flags); - if (!err) { - err = scsi_dh_handler_attach(sdev, scsi_dh); - put_device(&sdev->sdev_gendev); + if (err) + return err; + + if (sdev->scsi_dh_data) { + if (sdev->scsi_dh_data->scsi_dh != scsi_dh) + err = -EBUSY; + goto out_put_device; } + + err = scsi_dh_handler_attach(sdev, scsi_dh); + +out_put_device: + put_device(&sdev->sdev_gendev); return err; } EXPORT_SYMBOL_GPL(scsi_dh_attach); -/* - * scsi_dh_detach - Detach device handler - * @q - Request queue that is associated with the scsi_device - * the handler should be detached from - * - * This function will detach the device handler only - * if the sdev is not part of the internal list, ie - * if it has been attached manually. - */ -void scsi_dh_detach(struct request_queue *q) -{ - unsigned long flags; - struct scsi_device *sdev; - struct scsi_device_handler *scsi_dh = NULL; - - spin_lock_irqsave(q->queue_lock, flags); - sdev = q->queuedata; - if (!sdev || !get_device(&sdev->sdev_gendev)) - sdev = NULL; - spin_unlock_irqrestore(q->queue_lock, flags); - - if (!sdev) - return; - - if (sdev->scsi_dh_data) { - scsi_dh = sdev->scsi_dh_data->scsi_dh; - scsi_dh_handler_detach(sdev, scsi_dh); - } - put_device(&sdev->sdev_gendev); -} -EXPORT_SYMBOL_GPL(scsi_dh_detach); - /* * scsi_dh_attached_handler_name - Get attached device handler's name * @q - Request queue that is associated with the scsi_device diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 50c2a363bc8f..798d67994aeb 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -221,7 +221,6 @@ struct scsi_device_handler { struct scsi_dh_data { struct scsi_device_handler *scsi_dh; struct scsi_device *sdev; - struct kref kref; }; #define to_scsi_device(d) \ diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h index 620c723ee8ed..99c9196fe606 100644 --- a/include/scsi/scsi_dh.h +++ b/include/scsi/scsi_dh.h @@ -59,7 +59,6 @@ enum { extern int scsi_dh_activate(struct request_queue *, activate_complete, void *); extern int scsi_dh_handler_exist(const char *); extern int scsi_dh_attach(struct request_queue *, const char *); -extern void scsi_dh_detach(struct request_queue *); extern const char *scsi_dh_attached_handler_name(struct request_queue *, gfp_t); extern int scsi_dh_set_params(struct request_queue *, const char *); #else @@ -77,10 +76,6 @@ static inline int scsi_dh_attach(struct request_queue *req, const char *name) { return SCSI_DH_NOSYS; } -static inline void scsi_dh_detach(struct request_queue *q) -{ - return; -} static inline const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp) {