Integrate scsi_dh better into the scsi core V4
Subsystems (2)
cover Integrate scsi_dh better into the scsi core V4
This series ties scsi_dh deeper into the scsi core, and fixes all kinds of issues in it, most importantly the race between using and detaching device handlers.
Changes since V1:
- updated comments / strings based on review feedback
- moved scsi_dh.c to drivers/scsi to fix the srcdir = objdir build
- changed the old patch 2 to have saner handling of mismatching device handlers in dm.
- dropped patch 1: not having a hw handler is fine if we don't plan to change it anyway
- dropped patch 3: not necessary anymore.
Changes since V2:
- Fixup issues during attaching device_handler from dm-multipath
- Add patch to clarify scsi_dh_activate() error codes
Changes since V3:
- Fixup minor bugs discovered during testing
- Fixup patch description
Christoph Hellwig (8):
dm-mpath, scsi_dh: don't let dm detach device handlers dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath
scsi_dh: move to drivers/scsi
scsi_dh: integrate into the core SCSI code
scsi_dh: move device matching to the core code
scsi_dh: kill struct scsi_dh_data
scsi_dh: add a common helper to get a scsi_device from a request_queue
scsi_dh: don't allow to detach device handlers at runtime
Hannes Reinecke (2):
scsi_dh: return SCSI_DH_NOTCONN in scsi_dh_activate()
scsi_dh: move 'dh_state' sysfs attribute to generic code
drivers/md/dm-mpath.c | 27 +- drivers/scsi/Makefile | 1 + drivers/scsi/device_handler/Kconfig | 2 +- drivers/scsi/device_handler/Makefile | 1 - drivers/scsi/device_handler/scsi_dh.c | 621 ---------------------------- drivers/scsi/device_handler/scsi_dh_alua.c | 31 +- drivers/scsi/device_handler/scsi_dh_emc.c | 58 +-- drivers/scsi/device_handler/scsi_dh_hp_sw.c | 55 +-- drivers/scsi/device_handler/scsi_dh_rdac.c | 80 +--- drivers/scsi/scsi_dh.c | 371 +++++++++++++++++ drivers/scsi/scsi_error.c | 6 +- drivers/scsi/scsi_lib.c | 6 +- drivers/scsi/scsi_priv.h | 9 + drivers/scsi/scsi_sysfs.c | 68 +++ include/scsi/scsi_device.h | 27 +- include/scsi/scsi_dh.h | 29 +- 16 files changed, 527 insertions(+), 865 deletions(-) delete mode 100644 drivers/scsi/device_handler/scsi_dh.c create mode 100644 drivers/scsi/scsi_dh.c
Signature · 1 line
1.8.5.6
PATCH 1/10 dm-mpath, scsi_dh: don't let dm detach device handlers
From: Christoph Hellwig <[email protected]>
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.
--- drivers/md/dm-mpath.c | 21 +++----- drivers/scsi/device_handler/scsi_dh.c | 96 ++++++++--------------------------- include/scsi/scsi_device.h | 1 - include/scsi/scsi_dh.h | 5 -- 4 files changed, 28 insertions(+), 95 deletions(-)
drivers/md/dm-mpath.c +6 -15
@@ -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 *psif (r <0 ) { ti->error ="unable to set hardware " "handler parameters" ; -scsi_dh_detach (q);dm_put_device (ti, p->path.dev); goto bad; }
drivers/scsi/device_handler/scsi_dh.c +22 -74
151 lines not on this page · raw message
include/scsi/scsi_device.h +0 -1
@@ -220,7 +220,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) \
include/scsi/scsi_dh.h +0 -5
@@ -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) {
Signature · 1 line
1.8.5.6
PATCH 2/10 dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath
In reply to Hannes ReineckeFrom: Christoph Hellwig <[email protected]>
This way we can reused the same code any attachment method, not just those requested from dm-mpath.
--- drivers/md/dm-mpath.c | 6 ------ drivers/scsi/device_handler/scsi_dh.c | 35 ++++++++++++++++++----------------- include/scsi/scsi_dh.h | 5 ----- 3 files changed, 18 insertions(+), 28 deletions(-)
drivers/md/dm-mpath.c +0 -6
@@ -725,12 +725,6 @@ static int parse_hw_handler(struct dm_arg_set *as, struct multipath *m)return 0 ; m->hw_handler_name =kstrdup (dm_shift_arg (as),GFP_KERNEL ); -if (!try_then_request_module (scsi_dh_handler_exist (m->hw_handler_name), -"scsi_dh_%s" , m->hw_handler_name)) { - ti->error ="unknown hardware handler type" ; - ret = -EINVAL ; - goto fail; - }if (hw_argc >1 ) {char *p;
drivers/scsi/device_handler/scsi_dh.c +18 -17
@@ -29,7 +29,7 @@static DEFINE_SPINLOCK (list_lock);static LIST_HEAD (scsi_dh_list); -static struct scsi_device_handler *get_device_handler (const char *name) +static struct scsi_device_handler *__scsi_dh_lookup (const char *name) {struct scsi_device_handler *tmp, *found =NULL ;
@@ -44,6 +44,19 @@ static struct scsi_device_handler *get_device_handler(const char *name)return found; } +static struct scsi_device_handler *scsi_dh_lookup (const char *name) +{ +struct scsi_device_handler *dh; + + dh =__scsi_dh_lookup (name); +if (!dh) { +request_module (name); + dh =__scsi_dh_lookup (name); + } + +return dh; +} +/* * device_handler_match_function - Match a device handler to a device * @sdev - SCSI device to be tested
@@ -158,7 +171,7 @@ store_dh_state(struct device *dev, struct device_attribute *attr,/* * Attach to a device handler */ -if (!(scsi_dh =get_device_handler (buf))) +if (!(scsi_dh =scsi_dh_lookup (buf)))return err; err =scsi_dh_handler_attach (sdev, scsi_dh); }else {
@@ -322,8 +335,7 @@ static int scsi_dh_notifier_remove(struct device *dev, void *data)*/ int scsi_register_device_handler (struct scsi_device_handler *scsi_dh) { - -if (get_device_handler (scsi_dh->name)) +if (__scsi_dh_lookup (scsi_dh->name))return -EBUSY ;if (!scsi_dh->attach || !scsi_dh->detach)
@@ -350,7 +362,7 @@ EXPORT_SYMBOL_GPL(scsi_register_device_handler);int scsi_unregister_device_handler (struct scsi_device_handler *scsi_dh) { -if (!get_device_handler (scsi_dh->name)) +if (!__scsi_dh_lookup (scsi_dh->name))return -ENODEV ; bus_for_each_dev(&scsi_bus_type, NULL, scsi_dh,
@@ -455,17 +467,6 @@ int scsi_dh_set_params(struct request_queue *q, const char *params)EXPORT_SYMBOL_GPL (scsi_dh_set_params);/* -* scsi_dh_handler_exist - Return TRUE(1) if a device handler exists for -* the given name. FALSE(0) otherwise. -* @name - name of the device handler. -*/ -int scsi_dh_handler_exist (const char *name) -{ -return (get_device_handler (name) !=NULL ); -} -EXPORT_SYMBOL_GPL (scsi_dh_handler_exist); - -/* * scsi_dh_attach - Attach device handler * @q - Request queue that is associated with the scsi_device * the handler should be attached to
@@ -478,7 +479,7 @@ int scsi_dh_attach(struct request_queue *q, const char *name)struct scsi_device_handler *scsi_dh;int err =0 ; - scsi_dh =get_device_handler (name); + scsi_dh =scsi_dh_lookup (name);if (!scsi_dh)return -EINVAL ;
include/scsi/scsi_dh.h +0 -5
@@ -57,7 +57,6 @@ enum { };#if defined(CONFIG_SCSI_DH ) || defined(CONFIG_SCSI_DH_MODULE )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 const char *scsi_dh_attached_handler_name (struct request_queue *,gfp_t );extern int scsi_dh_set_params (struct request_queue *,const char *);
@@ -68,10 +67,6 @@ static inline int scsi_dh_activate(struct request_queue *req,fn (data,0 );return 0 ; } -static inline int scsi_dh_handler_exist (const char *name) -{ -return 0 ; -}static inline int scsi_dh_attach (struct request_queue *req,const char *name) {return SCSI_DH_NOSYS ;
Signature · 1 line
1.8.5.6
James Bottomley
maintainerRe: [PATCH 02/10] dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath
drivers/scsi/device_handler/scsi_dh.c store_dh_state(struct device *dev, struct device_attribute *attr, · line 174
Quoted 6 lines · Hannes Reinecke
drivers/scsi/device_handler/scsi_dh.c +1
-1@@ -158,7 +171,7 @@ store_dh_state(struct device *dev, struct device_attribute *attr,/* * Attach to a device handler */ -if (!(scsi_dh =get_device_handler (buf))) +if (!(scsi_dh =scsi_dh_lookup (buf)))
This one's a checkpatch error:
ERROR: do not use assignment in if condition #71: FILE: drivers/scsi/device_handler/scsi_dh.c:174:
+ if (!(scsi_dh = scsi_dh_lookup(buf)))
For very good reason. The meaning is obvious from the previous code, so I fixed it up as
scsi_dh = scsi_dh_lookup(buf); if (!scsi_dh)
But running the series through checkpatch and acting on at least the errors would have been appreciated.
James
Re: [PATCH 02/10] dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath
Quoted 15 lines · James Bottomley
drivers/scsi/device_handler/scsi_dh.c store_dh_state(struct device *dev, struct device_attribute *attr, · line 174
Quoted 6 lines · Hannes Reinecke
drivers/scsi/device_handler/scsi_dh.c +1
-1@@ -158,7 +171,7 @@ store_dh_state(struct device *dev, struct device_attribute *attr,/* * Attach to a device handler */ -if (!(scsi_dh =get_device_handler (buf))) +if (!(scsi_dh =scsi_dh_lookup (buf)))This one's a checkpatch error:
ERROR: do not use assignment in if condition #71: FILE:
drivers/scsi/device_handler/scsi_dh.c:174:
+ if (!(scsi_dh =scsi_dh_lookup(buf)))
Quoted 9 lines
For very good reason. The meaning is obvious from the previous code, so I fixed it up as
scsi_dh = scsi_dh_lookup(buf); if (!scsi_dh)But running the series through checkpatch and acting on at least the errors would have been appreciated.
You're right, sorry. Thanks for fixing it up.
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 3/10 scsi_dh: move to drivers/scsi
In reply to Hannes ReineckeFrom: Christoph Hellwig <[email protected]>
Prepare for building scsi_dh.c into the core SCSI module by moving it to drivers/scsi.
--- drivers/scsi/Makefile | 1 + drivers/scsi/device_handler/Makefile | 1 - drivers/scsi/device_handler/scsi_dh.c | 570 ---------------------------------- drivers/scsi/scsi_dh.c | 570 ++++++++++++++++++++++++++++++++++ 4 files changed, 571 insertions(+), 571 deletions(-) delete mode 100644 drivers/scsi/device_handler/scsi_dh.c create mode 100644 drivers/scsi/scsi_dh.c
drivers/scsi/Makefile +1 -0
@@ -171,6 +171,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o scsi_mod-y += scsi_trace.o scsi_logging.o scsi_mod-$(CONFIG_PM) += scsi_pm.o +obj-$(CONFIG_SCSI_DH) += scsi_dh.o hv_storvsc-y := storvsc_drv.o
drivers/scsi/device_handler/Makefile +0 -1
@@ -1,7 +1,6 @@ # # SCSI Device Handler # -obj-$(CONFIG_SCSI_DH) += scsi_dh.o obj-$(CONFIG_SCSI_DH_RDAC) += scsi_dh_rdac.o obj-$(CONFIG_SCSI_DH_HP_SW) += scsi_dh_hp_sw.o obj-$(CONFIG_SCSI_DH_EMC) += scsi_dh_emc.o
drivers/scsi/device_handler/scsi_dh.c deleted +0 -570
570 lines not on this page · raw message
drivers/scsi/scsi_dh.c new file +570 -0
570 lines not on this page · raw message
Signature · 1 line
1.8.5.6
PATCH 4/10 scsi_dh: integrate into the core SCSI code
In reply to Hannes ReineckeFrom: Christoph Hellwig <[email protected]>
Stop building scsi_dh as a separate module and integrate it fully into the core SCSI code with explicit callouts at bus scan time. For now the callouts are placed at the same point as the old bus notifiers were called, but in the future we will be able to look at ALUA INQUIRY data earlier on.
Note that this also means that the device handler modules need to be loaded by the time we scan the bus. The next patches will add support for autoloading device handlers at bus scan time to make sure they are always loaded if they are enabled in the kernel config.
--- drivers/scsi/Makefile | 2 +- drivers/scsi/device_handler/Kconfig | 2 +- drivers/scsi/scsi_dh.c | 183 +++--------------------------------- drivers/scsi/scsi_priv.h | 9 ++ drivers/scsi/scsi_sysfs.c | 10 ++ include/scsi/scsi_dh.h | 2 +- 6 files changed, 34 insertions(+), 174 deletions(-)
drivers/scsi/Makefile +1 -1
@@ -171,7 +171,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o scsi_mod-y += scsi_trace.o scsi_logging.o scsi_mod-$(CONFIG_PM) += scsi_pm.o -obj-$(CONFIG_SCSI_DH) += scsi_dh.o +scsi_mod-$(CONFIG_SCSI_DH) += scsi_dh.o hv_storvsc-y := storvsc_drv.o
drivers/scsi/device_handler/Kconfig +1 -1
@@ -3,7 +3,7 @@ # menuconfig SCSI_DH - tristate "SCSI Device Handlers" + bool "SCSI Device Handlers" depends on SCSI default n help
drivers/scsi/scsi_dh.c +12 -171
229 lines not on this page · raw message
drivers/scsi/scsi_priv.h +9 -0
@@ -170,6 +170,15 @@ static inline void scsi_autopm_put_host(struct Scsi_Host *h) {}extern struct async_domain scsi_sd_pm_domain;extern struct async_domain scsi_sd_probe_domain; +/* scsi_dh.c */ +#ifdef CONFIG_SCSI_DH +int scsi_dh_add_device (struct scsi_device *sdev); +void scsi_dh_remove_device (struct scsi_device *sdev); +#else +static inline int scsi_dh_add_device (struct scsi_device *sdev) {return 0 ; } +static inline void scsi_dh_remove_device (struct scsi_device *sdev) { } +#endif +/* * internal scsi timeout functions: for use by mid-layer and transport * classes.
drivers/scsi/scsi_sysfs.c +10 -0
@@ -1030,11 +1030,20 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev) "failed to add device: %d\n", error);return error; } + + error =scsi_dh_add_device (sdev); +if (error) { +sdev_printk (KERN_INFO , sdev, +"failed to add device handler: %d\n" , error); +return error; + } +device_enable_async_suspend (&sdev->sdev_dev); error =device_add (&sdev->sdev_dev);if (error) {sdev_printk (KERN_INFO , sdev,"failed to add class device: %d\n" , error); +scsi_dh_remove_device (sdev);device_del (&sdev->sdev_gendev);return error; }
@@ -1074,6 +1083,7 @@ void __scsi_remove_device(struct scsi_device *sdev)bsg_unregister_queue (sdev->request_queue);device_unregister (&sdev->sdev_dev);transport_remove_device (dev); +scsi_dh_remove_device (sdev);device_del (dev); }else put_device (&sdev->sdev_dev);
include/scsi/scsi_dh.h +1 -1
@@ -55,7 +55,7 @@ enum {SCSI_DH_NOSYS , SCSI_DH_DRIVER_MAX, }; -#if defined(CONFIG_SCSI_DH ) || defined(CONFIG_SCSI_DH_MODULE ) +#ifdef CONFIG_SCSI_DH extern int scsi_dh_activate (struct request_queue *,activate_complete ,void *);extern int scsi_dh_attach (struct request_queue *,const char *);extern const char *scsi_dh_attached_handler_name (struct request_queue *,gfp_t );
Signature · 1 line
1.8.5.6
PATCH 5/10 scsi_dh: move device matching to the core code
In reply to Hannes ReineckeFrom: Christoph Hellwig <[email protected]>
Add a single list of devices that need non-ALUA device handlers to the core scsi_dh code so that we can autoload the modules for them at probe time.
While this is a little ugly in terms of architecture it actually significantly simplifies the code in addition to the new autoloading functionality.
--- drivers/scsi/device_handler/scsi_dh_alua.c | 6 --- drivers/scsi/device_handler/scsi_dh_emc.c | 29 ---------- drivers/scsi/device_handler/scsi_dh_hp_sw.c | 30 ----------- drivers/scsi/device_handler/scsi_dh_rdac.c | 50 ----------------- drivers/scsi/scsi_dh.c | 84 ++++++++++++++++++++++------- include/scsi/scsi_device.h | 1 - 6 files changed, 66 insertions(+), 134 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +0 -6
@@ -819,11 +819,6 @@ static int alua_prep_fn(struct scsi_device *sdev, struct request *req) } -static bool alua_match (struct scsi_device *sdev) -{ -return (scsi_device_tpgs (sdev) !=0 ); -} -/* * alua_bus_attach - Attach device handler * @sdev: device to be attached to
@@ -877,7 +872,6 @@ static struct scsi_device_handler alua_dh = { .check_sense = alua_check_sense, .activate = alua_activate, .set_params = alua_set_params, - .match = alua_match, };static int __init alua_init (void )
drivers/scsi/device_handler/scsi_dh_emc.c +0 -29
@@ -622,34 +622,6 @@ done:return result; } -static const struct { -char *vendor; -char *model; -} clariion_dev_list[] = { - {"DGC" ,"RAID" }, - {"DGC" ,"DISK" }, - {"DGC" ,"VRAID" }, - {NULL ,NULL }, -}; - -static bool clariion_match (struct scsi_device *sdev) -{ -int i; - -if (scsi_device_tpgs (sdev)) -return false; - -for (i =0 ; clariion_dev_list[i].vendor; i++) { -if (!strncmp (sdev->vendor, clariion_dev_list[i].vendor, -strlen (clariion_dev_list[i].vendor)) && - !strncmp (sdev->model, clariion_dev_list[i].model, -strlen (clariion_dev_list[i].model))) { -return true; - } - } -return false; -} -static struct scsi_dh_data *clariion_bus_attach (struct scsi_device *sdev) {struct clariion_dh_data *h;
@@ -698,7 +670,6 @@ static struct scsi_device_handler clariion_dh = { .activate = clariion_activate, .prep_fn = clariion_prep_fn, .set_params = clariion_set_params, - .match = clariion_match, };static int __init clariion_init (void )
drivers/scsi/device_handler/scsi_dh_hp_sw.c +0 -30
@@ -311,35 +311,6 @@ static int hp_sw_activate(struct scsi_device *sdev,return 0 ; } -static const struct { -char *vendor; -char *model; -} hp_sw_dh_data_list[] = { - {"COMPAQ" ,"MSA1000 VOLUME" }, - {"COMPAQ" ,"HSV110" }, - {"HP" ,"HSV100" }, - {"DEC" ,"HSG80" }, - {NULL ,NULL }, -}; - -static bool hp_sw_match (struct scsi_device *sdev) -{ -int i; - -if (scsi_device_tpgs (sdev)) -return false; - -for (i =0 ; hp_sw_dh_data_list[i].vendor; i++) { -if (!strncmp (sdev->vendor, hp_sw_dh_data_list[i].vendor, -strlen (hp_sw_dh_data_list[i].vendor)) && - !strncmp (sdev->model, hp_sw_dh_data_list[i].model, -strlen (hp_sw_dh_data_list[i].model))) { -return true; - } - } -return false; -} -static struct scsi_dh_data *hp_sw_bus_attach (struct scsi_device *sdev) {struct hp_sw_dh_data *h;
@@ -379,7 +350,6 @@ static struct scsi_device_handler hp_sw_dh = { .detach = hp_sw_bus_detach, .activate = hp_sw_activate, .prep_fn = hp_sw_prep_fn, - .match = hp_sw_match, };static int __init hp_sw_init (void )
drivers/scsi/device_handler/scsi_dh_rdac.c +0 -50
62 lines not on this page · raw message
drivers/scsi/scsi_dh.c +66 -18
108 lines not on this page · raw message
include/scsi/scsi_device.h +0 -1
@@ -214,7 +214,6 @@ struct scsi_device_handler {int (*activate)(struct scsi_device *,activate_complete ,void *);int (*prep_fn)(struct scsi_device *,struct request *);int (*set_params)(struct scsi_device *,const char *); -bool (*match)(struct scsi_device *); };struct scsi_dh_data {
Signature · 1 line
1.8.5.6
PATCH 6/10 scsi_dh: kill struct scsi_dh_data
In reply to Hannes ReineckeFrom: Christoph Hellwig <[email protected]>
Add a ->handler and a ->handler_data field to struct scsi_device and kill this indirection. Also move struct scsi_device_handler to scsi_dh.h so that changes to it don't require rebuilding every SCSI LLDD.
--- drivers/scsi/device_handler/scsi_dh_alua.c | 25 ++++------ drivers/scsi/device_handler/scsi_dh_emc.c | 29 +++++------ drivers/scsi/device_handler/scsi_dh_hp_sw.c | 25 ++++------ drivers/scsi/device_handler/scsi_dh_rdac.c | 30 +++++------- drivers/scsi/scsi_dh.c | 75 +++++++++++------------------ drivers/scsi/scsi_error.c | 6 +-- drivers/scsi/scsi_lib.c | 6 +-- include/scsi/scsi_device.h | 25 ++-------- include/scsi/scsi_dh.h | 17 +++++++ 9 files changed, 98 insertions(+), 140 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +10 -15
@@ -62,7 +62,6 @@#define ALUA_OPTIMIZE_STPG 1struct alua_dh_data { -struct scsi_dh_data dh_data;int group_id;int rel_port;int tpgs;
@@ -86,11 +85,6 @@ struct alua_dh_data {static char print_alua_state (int );static int alua_check_sense (struct scsi_device *,struct scsi_sense_hdr *); -static inline struct alua_dh_data *get_alua_data (struct scsi_device *sdev) -{ - return container_of(sdev->scsi_dh_data, struct alua_dh_data, dh_data); -} -static int realloc_buffer (struct alua_dh_data *h,unsigned len) {if (h->buff && h->buff != h->inq)
@@ -708,7 +702,7 @@ out:*/ static int alua_set_params (struct scsi_device *sdev,const char *params) { -struct alua_dh_data *h =get_alua_data (sdev); +struct alua_dh_data *h = sdev->handler_data;unsigned int optimize =0 , argc;const char *p = params;int result =SCSI_DH_OK ;
@@ -746,7 +740,7 @@ MODULE_PARM_DESC(optimize_stpg, "Allow use of a non-optimized path, rather thanstatic int alua_activate (struct scsi_device *sdev,activate_complete fn,void *data) { -struct alua_dh_data *h =get_alua_data (sdev); +struct alua_dh_data *h = sdev->handler_data;int err =SCSI_DH_OK ;int stpg =0 ;
@@ -804,7 +798,7 @@ out:*/ static int alua_prep_fn (struct scsi_device *sdev,struct request *req) { -struct alua_dh_data *h =get_alua_data (sdev); +struct alua_dh_data *h = sdev->handler_data;int ret =BLKPREP_OK ;if (h->state ==TPGS_STATE_TRANSITIONING )
@@ -823,14 +817,14 @@ static int alua_prep_fn(struct scsi_device *sdev, struct request *req)* alua_bus_attach - Attach device handler * @sdev: device to be attached to */ -static struct scsi_dh_data *alua_bus_attach (struct scsi_device *sdev) +static int alua_bus_attach (struct scsi_device *sdev) {struct alua_dh_data *h;int err; h =kzalloc (sizeof (*h) ,GFP_KERNEL );if (!h) -return ERR_PTR (-ENOMEM ); +return -ENOMEM ; h->tpgs =TPGS_MODE_UNINITIALIZED ; h->state =TPGS_STATE_OPTIMIZED ; h->group_id =-1 ;
@@ -843,11 +837,11 @@ static struct scsi_dh_data *alua_bus_attach(struct scsi_device *sdev)if (err !=SCSI_DH_OK && err !=SCSI_DH_DEV_OFFLINED ) goto failed; -sdev_printk (KERN_NOTICE , sdev,"%s: Attached\n" ,ALUA_DH_NAME ); -return &h->dh_data; + sdev->handler_data = h; +return 0 ; failed:kfree (h); -return ERR_PTR (-EINVAL ); +return -EINVAL ; }/*
@@ -856,10 +850,11 @@ failed:*/ static void alua_bus_detach (struct scsi_device *sdev) { -struct alua_dh_data *h =get_alua_data (sdev); +struct alua_dh_data *h = sdev->handler_data;if (h->buff && h->inq != h->buff)kfree (h->buff); + sdev->handler_data =NULL ;kfree (h); }
drivers/scsi/device_handler/scsi_dh_emc.c +11 -18
@@ -72,7 +72,6 @@ static const char * lun_state[] = };struct clariion_dh_data { -struct scsi_dh_data dh_data;/* * Flags: * CLARIION_SHORT_TRESPASS
@@ -114,13 +113,6 @@ struct clariion_dh_data {int current_sp; }; -static inline struct clariion_dh_data - *get_clariion_data (struct scsi_device *sdev) -{ - return container_of(sdev->scsi_dh_data, struct clariion_dh_data, - dh_data); -} -/* * Parse MODE_SELECT cmd reply. */
@@ -450,7 +442,7 @@ static int clariion_check_sense(struct scsi_device *sdev,static int clariion_prep_fn (struct scsi_device *sdev,struct request *req) { -struct clariion_dh_data *h =get_clariion_data (sdev); +struct clariion_dh_data *h = sdev->handler_data;int ret =BLKPREP_OK ;if (h->lun_state !=CLARIION_LUN_OWNED ) {
@@ -533,7 +525,7 @@ retry:static int clariion_activate (struct scsi_device *sdev,activate_complete fn,void *data) { -struct clariion_dh_data *csdev =get_clariion_data (sdev); +struct clariion_dh_data *csdev = sdev->handler_data;int result; result =clariion_send_inquiry (sdev, csdev);
@@ -574,7 +566,7 @@ done:*/ static int clariion_set_params (struct scsi_device *sdev,const char *params) { -struct clariion_dh_data *csdev =get_clariion_data (sdev); +struct clariion_dh_data *csdev = sdev->handler_data;unsigned int hr =0 , st =0 , argc;const char *p = params;int result =SCSI_DH_OK ;
@@ -622,14 +614,14 @@ done:return result; } -static struct scsi_dh_data *clariion_bus_attach (struct scsi_device *sdev) +static int clariion_bus_attach (struct scsi_device *sdev) {struct clariion_dh_data *h;int err; h =kzalloc (sizeof (*h) ,GFP_KERNEL );if (!h) -return ERR_PTR (-ENOMEM ); +return -ENOMEM ; h->lun_state =CLARIION_LUN_UNINITIALIZED ; h->default_sp =CLARIION_UNBOUND_LU ; h->current_sp =CLARIION_UNBOUND_LU ;
@@ -647,18 +639,19 @@ static struct scsi_dh_data *clariion_bus_attach(struct scsi_device *sdev)CLARIION_NAME , h->current_sp +'A' , h->port, lun_state[h->lun_state], h->default_sp + 'A'); -return &h->dh_data; + + sdev->handler_data = h; +return 0 ; failed:kfree (h); -return ERR_PTR (-EINVAL ); +return -EINVAL ; }static void clariion_bus_detach (struct scsi_device *sdev) { -struct clariion_dh_data *h =get_clariion_data (sdev); - -kfree (h); +kfree (sdev->handler_data); + sdev->handler_data =NULL ; }static struct scsi_device_handler clariion_dh = {
drivers/scsi/device_handler/scsi_dh_hp_sw.c +10 -15
@@ -38,7 +38,6 @@#define HP_SW_PATH_PASSIVE 1struct hp_sw_dh_data { -struct scsi_dh_data dh_data;unsigned char sense[SCSI_SENSE_BUFFERSIZE ];int path_state;int retries;
@@ -50,11 +49,6 @@ struct hp_sw_dh_data {static int hp_sw_start_stop (struct hp_sw_dh_data *); -static inline struct hp_sw_dh_data *get_hp_sw_data (struct scsi_device *sdev) -{ - return container_of(sdev->scsi_dh_data, struct hp_sw_dh_data, dh_data); -} -/* * tur_done - Handle TEST UNIT READY return status * @sdev: sdev the command has been sent to
@@ -267,7 +261,7 @@ static int hp_sw_start_stop(struct hp_sw_dh_data *h)static int hp_sw_prep_fn (struct scsi_device *sdev,struct request *req) { -struct hp_sw_dh_data *h =get_hp_sw_data (sdev); +struct hp_sw_dh_data *h = sdev->handler_data;int ret =BLKPREP_OK ;if (h->path_state !=HP_SW_PATH_ACTIVE ) {
@@ -292,7 +286,7 @@ static int hp_sw_activate(struct scsi_device *sdev, activate_complete fn, void *data) { int ret = SCSI_DH_OK; -struct hp_sw_dh_data *h =get_hp_sw_data (sdev); +struct hp_sw_dh_data *h = sdev->handler_data; ret =hp_sw_tur (sdev, h);
@@ -311,14 +305,14 @@ static int hp_sw_activate(struct scsi_device *sdev,return 0 ; } -static struct scsi_dh_data *hp_sw_bus_attach (struct scsi_device *sdev) +static int hp_sw_bus_attach (struct scsi_device *sdev) {struct hp_sw_dh_data *h;int ret; h =kzalloc (sizeof (*h),GFP_KERNEL );if (!h) -return ERR_PTR (-ENOMEM ); +return -ENOMEM ; h->path_state =HP_SW_PATH_UNINITIALIZED ; h->retries =HP_SW_RETRIES ; h->sdev = sdev;
@@ -330,17 +324,18 @@ static struct scsi_dh_data *hp_sw_bus_attach(struct scsi_device *sdev)sdev_printk (KERN_INFO , sdev,"%s: attached to %s path\n" ,HP_SW_NAME , h->path_state ==HP_SW_PATH_ACTIVE ?"active" :"passive" ); -return &h->dh_data; + + sdev->handler_data = h; +return 0 ; failed:kfree (h); -return ERR_PTR (-EINVAL ); +return -EINVAL ; }static void hp_sw_bus_detach (struct scsi_device *sdev ) { -struct hp_sw_dh_data *h =get_hp_sw_data (sdev); - -kfree (h); +kfree (sdev->handler_data); + sdev->handler_data =NULL ; }static struct scsi_device_handler hp_sw_dh = {
drivers/scsi/device_handler/scsi_dh_rdac.c +13 -17
@@ -181,7 +181,6 @@ struct c2_inquiry { };struct rdac_dh_data { -struct scsi_dh_data dh_data;struct rdac_controller *ctlr;#define UNINITIALIZED_LUN (1 << 8)unsigned lun;
@@ -260,11 +259,6 @@ do { \ sdev_printk(KERN_INFO, sdev, RDAC_NAME ": " f "\n", ## arg); \ }while (0 ); -static inline struct rdac_dh_data *get_rdac_data (struct scsi_device *sdev) -{ - return container_of(sdev->scsi_dh_data, struct rdac_dh_data, dh_data); -} -static struct request *get_rdac_req (struct scsi_device *sdev,void *buffer,unsigned buflen,int rw) {
@@ -544,7 +538,7 @@ static int mode_select_handle_sense(struct scsi_device *sdev, {struct scsi_sense_hdr sense_hdr;int err =SCSI_DH_IO , ret; -struct rdac_dh_data *h =get_rdac_data (sdev); +struct rdac_dh_data *h = sdev->handler_data; ret =scsi_normalize_sense (sensebuf,SCSI_SENSE_BUFFERSIZE , &sense_hdr);if (!ret)
@@ -589,7 +583,7 @@ static void send_mode_select(struct work_struct *work) container_of(work, struct rdac_controller, ms_work);struct request *rq;struct scsi_device *sdev = ctlr->ms_sdev; -struct rdac_dh_data *h =get_rdac_data (sdev); +struct rdac_dh_data *h = sdev->handler_data;struct request_queue *q = sdev->request_queue;int err, retry_cnt =RDAC_RETRY_COUNT ;struct rdac_queue_data *tmp, *qdata;
@@ -648,7 +642,7 @@ static int queue_mode_select(struct scsi_device *sdev,if (!qdata)return SCSI_DH_RETRY ; - qdata->h =get_rdac_data (sdev); + qdata->h = sdev->handler_data; qdata->callback_fn = fn; qdata->callback_data = data;
@@ -667,7 +661,7 @@ static int queue_mode_select(struct scsi_device *sdev,static int rdac_activate (struct scsi_device *sdev,activate_complete fn,void *data) { -struct rdac_dh_data *h =get_rdac_data (sdev); +struct rdac_dh_data *h = sdev->handler_data;int err =SCSI_DH_OK ;int act =0 ;
@@ -702,7 +696,7 @@ done:static int rdac_prep_fn (struct scsi_device *sdev,struct request *req) { -struct rdac_dh_data *h =get_rdac_data (sdev); +struct rdac_dh_data *h = sdev->handler_data;int ret =BLKPREP_OK ;if (h->state !=RDAC_STATE_ACTIVE ) {
@@ -716,7 +710,7 @@ static int rdac_prep_fn(struct scsi_device *sdev, struct request *req)static int rdac_check_sense (struct scsi_device *sdev,struct scsi_sense_hdr *sense_hdr) { -struct rdac_dh_data *h =get_rdac_data (sdev); +struct rdac_dh_data *h = sdev->handler_data; RDAC_LOG(RDAC_LOG_SENSE, sdev, "array %s, ctlr %d, " "I/O returned with sense %02x/%02x/%02x",
@@ -778,7 +772,7 @@ static int rdac_check_sense(struct scsi_device *sdev,return SCSI_RETURN_NOT_HANDLED ; } -static struct scsi_dh_data *rdac_bus_attach (struct scsi_device *sdev) +static int rdac_bus_attach (struct scsi_device *sdev) {struct rdac_dh_data *h;int err;
@@ -787,7 +781,7 @@ static struct scsi_dh_data *rdac_bus_attach(struct scsi_device *sdev) h =kzalloc (sizeof (*h) ,GFP_KERNEL );if (!h) -return ERR_PTR (-ENOMEM ); +return -ENOMEM ; h->lun =UNINITIALIZED_LUN ; h->state =RDAC_STATE_ACTIVE ;
@@ -812,7 +806,8 @@ static struct scsi_dh_data *rdac_bus_attach(struct scsi_device *sdev)RDAC_NAME , h->lun, mode[(int )h->mode], lun_state[(int)h->lun_state]); -return &h->dh_data; + sdev->handler_data = h; +return 0 ; clean_ctlr:spin_lock (&list_lock);
@@ -821,12 +816,12 @@ clean_ctlr: failed:kfree (h); -return ERR_PTR (-EINVAL ); +return -EINVAL ; }static void rdac_bus_detach (struct scsi_device *sdev ) { -struct rdac_dh_data *h =get_rdac_data (sdev); +struct rdac_dh_data *h = sdev->handler_data;if (h->ctlr && h->ctlr->ms_queued)flush_workqueue (kmpath_rdacd);
@@ -835,6 +830,7 @@ static void rdac_bus_detach( struct scsi_device *sdev )if (h->ctlr)kref_put (&h->ctlr->kref, release_controller);spin_unlock (&list_lock); + sdev->handler_data =NULL ;kfree (h); }
drivers/scsi/scsi_dh.c +28 -47
170 lines not on this page · raw message
drivers/scsi/scsi_error.c +3 -3
@@ -36,6 +36,7 @@#include <scsi/scsi_transport.h> #include <scsi/scsi_host.h> #include <scsi/scsi_ioctl.h> +#include <scsi/scsi_dh.h> #include <scsi/sg.h> #include "scsi_priv.h"
@@ -459,11 +460,10 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)if (scsi_sense_is_deferred (&sshdr))return NEEDS_RETRY ; -if (sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh && - sdev->scsi_dh_data->scsi_dh->check_sense) { +if (sdev->handler && sdev->handler->check_sense) {int rc; - rc = sdev->scsi_dh_data->scsi_dh->check_sense (sdev, &sshdr); + rc = sdev->handler->check_sense (sdev, &sshdr);if (rc !=SCSI_RETURN_NOT_HANDLED )return rc;/* handler does not care. Drop down to default handling */
drivers/scsi/scsi_lib.c +3 -3
@@ -31,6 +31,7 @@#include <scsi/scsi_driver.h> #include <scsi/scsi_eh.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_dh.h> #include <trace/events/scsi.h>
@@ -1248,9 +1249,8 @@ static int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req) {struct scsi_cmnd *cmd = req->special; -if (unlikely (sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh - && sdev->scsi_dh_data->scsi_dh->prep_fn)) { -int ret = sdev->scsi_dh_data->scsi_dh->prep_fn (sdev, req); +if (unlikely (sdev->handler && sdev->handler->prep_fn)) { +int ret = sdev->handler->prep_fn (sdev, req);if (ret !=BLKPREP_OK )return ret; }
include/scsi/scsi_device.h +3 -22
@@ -195,32 +195,13 @@ struct scsi_device {struct execute_work ew;/* used to get process context on put */ struct work_struct requeue_work; -struct scsi_dh_data *scsi_dh_data; +struct scsi_device_handler *handler; +void *handler_data; +enum scsi_device_state sdev_state;unsigned long sdev_data[0 ]; } __attribute__((aligned(sizeof(unsigned long)))); -typedef void (*activate_complete )(void *,int ); -struct scsi_device_handler { -/* Used by the infrastructure */ -struct list_head list;/* list of scsi_device_handlers */ - -/* Filled by the hardware handler */ -struct module *module; -const char *name; -int (*check_sense)(struct scsi_device *,struct scsi_sense_hdr *); -struct scsi_dh_data *(*attach)(struct scsi_device *); -void (*detach)(struct scsi_device *); -int (*activate)(struct scsi_device *,activate_complete ,void *); -int (*prep_fn)(struct scsi_device *,struct request *); -int (*set_params)(struct scsi_device *,const char *); -}; - -struct scsi_dh_data { -struct scsi_device_handler *scsi_dh; -struct scsi_device *sdev; -}; -#define to_scsi_device (d) \ container_of(d, struct scsi_device, sdev_gendev)#define class_to_sdev (d) \
include/scsi/scsi_dh.h +17 -0
@@ -55,6 +55,23 @@ enum {SCSI_DH_NOSYS , SCSI_DH_DRIVER_MAX, }; + +typedef void (*activate_complete )(void *,int ); +struct scsi_device_handler { +/* Used by the infrastructure */ +struct list_head list;/* list of scsi_device_handlers */ + +/* Filled by the hardware handler */ +struct module *module; +const char *name; +int (*check_sense)(struct scsi_device *,struct scsi_sense_hdr *); +int (*attach)(struct scsi_device *); +void (*detach)(struct scsi_device *); +int (*activate)(struct scsi_device *,activate_complete ,void *); +int (*prep_fn)(struct scsi_device *,struct request *); +int (*set_params)(struct scsi_device *,const char *); +}; +#ifdef CONFIG_SCSI_DH extern int scsi_dh_activate (struct request_queue *,activate_complete ,void *);extern int scsi_dh_attach (struct request_queue *,const char *);
Signature · 1 line
1.8.5.6
PATCH 7/10 scsi_dh: add a common helper to get a scsi_device from a request_queue
In reply to Hannes ReineckeFrom: Christoph Hellwig <[email protected]>
And cleanup the various messy opencoded versions of this. Note that this moves the sdev_state checks outside the queue_lock coverage, but as we don't hold the lock over the activation they are only advisory anyway.
--- drivers/scsi/scsi_dh.c | 99 +++++++++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 53 deletions(-)
drivers/scsi/scsi_dh.c +46 -53
@@ -282,6 +282,20 @@ int scsi_unregister_device_handler(struct scsi_device_handler *scsi_dh) }EXPORT_SYMBOL_GPL (scsi_unregister_device_handler); +static struct scsi_device *get_sdev_from_queue (struct request_queue *q) +{ +struct scsi_device *sdev; +unsigned long flags; + +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); + +return sdev; +} +/* * scsi_dh_activate - activate the path associated with the scsi_device * corresponding to the given request queue.
@@ -297,41 +311,37 @@ EXPORT_SYMBOL_GPL(scsi_unregister_device_handler);*/ int scsi_dh_activate (struct request_queue *q,activate_complete fn,void *data) { -int err =0 ; -unsigned long flags;struct scsi_device *sdev; -struct device *dev =NULL ; +int err =SCSI_DH_NOSYS ; -spin_lock_irqsave (q->queue_lock, flags); - sdev = q->queuedata; + sdev =get_sdev_from_queue (q);if (!sdev) { -spin_unlock_irqrestore (q->queue_lock, flags); - err =SCSI_DH_NOSYS ;if (fn)fn (data, err);return err; } - dev =get_device (&sdev->sdev_gendev); -if (!sdev->handler || !dev || - sdev->sdev_state ==SDEV_CANCEL || +if (!sdev->handler) + goto out_fn; +if (sdev->sdev_state ==SDEV_CANCEL || sdev->sdev_state ==SDEV_DEL ) - err =SCSI_DH_NOSYS ; -if (sdev->sdev_state ==SDEV_OFFLINE ) - err =SCSI_DH_DEV_OFFLINED ; -spin_unlock_irqrestore (q->queue_lock, flags); + goto out_fn; -if (err) { -if (fn) -fn (data, err); - goto out; - } + err =SCSI_DH_DEV_OFFLINED ; +if (sdev->sdev_state ==SDEV_OFFLINE ) + goto out_fn;if (sdev->handler->activate) err = sdev->handler->activate (sdev, fn, data); -out: -put_device (dev); + +out_put_device: +put_device (&sdev->sdev_gendev);return err; + +out_fn: +if (fn) +fn (data, err); + goto out_put_device; }EXPORT_SYMBOL_GPL (scsi_dh_activate);
@@ -347,21 +357,15 @@ EXPORT_SYMBOL_GPL(scsi_dh_activate);*/ int scsi_dh_set_params (struct request_queue *q,const char *params) { -int err = -SCSI_DH_NOSYS ; -unsigned long flags;struct scsi_device *sdev; +int err = -SCSI_DH_NOSYS ; -spin_lock_irqsave (q->queue_lock, flags); - sdev = q->queuedata; -if (sdev->handler && - sdev->handler->set_params && -get_device (&sdev->sdev_gendev)) - err =0 ; -spin_unlock_irqrestore (q->queue_lock, flags); - -if (err) + sdev =get_sdev_from_queue (q); +if (!sdev)return err; - err = sdev->handler->set_params (sdev, params); + +if (sdev->handler && sdev->handler->set_params) + err = sdev->handler->set_params (sdev, params);put_device (&sdev->sdev_gendev);return err; }
@@ -375,23 +379,19 @@ EXPORT_SYMBOL_GPL(scsi_dh_set_params);*/ int scsi_dh_attach (struct request_queue *q,const char *name) { -unsigned long flags;struct scsi_device *sdev;struct scsi_device_handler *scsi_dh;int err =0 ; - scsi_dh =scsi_dh_lookup (name); -if (!scsi_dh) -return -EINVAL ; - -spin_lock_irqsave (q->queue_lock, flags); - sdev = q->queuedata; -if (!sdev || !get_device (&sdev->sdev_gendev)) - err = -ENODEV ; -spin_unlock_irqrestore (q->queue_lock, flags); + sdev =get_sdev_from_queue (q); +if (!sdev) +return -ENODEV ; -if (err) -return err; + scsi_dh =scsi_dh_lookup (name); +if (!scsi_dh) { + err = -EINVAL ; + goto out_put_device; + }if (sdev->handler) {if (sdev->handler != scsi_dh)
@@ -418,22 +418,15 @@ EXPORT_SYMBOL_GPL(scsi_dh_attach);*/ const char *scsi_dh_attached_handler_name (struct request_queue *q,gfp_t gfp) { -unsigned long flags;struct scsi_device *sdev;const char *handler_name =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); - + sdev =get_sdev_from_queue (q);if (!sdev)return NULL ;if (sdev->handler) handler_name =kstrdup (sdev->handler->name, gfp); -put_device (&sdev->sdev_gendev);return handler_name; }
Signature · 1 line
1.8.5.6
PATCH 8/10 scsi_dh: don't allow to detach device handlers at runtime
In reply to Hannes ReineckeFrom: Christoph Hellwig <[email protected]>
The I/O submission and completion pathes call into the device handler without any synchronization agains detachment. So disallow detaching device handlers at runtime.
--- drivers/scsi/scsi_dh.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/scsi/scsi_dh.c +4 -2
@@ -180,8 +180,10 @@ store_dh_state(struct device *dev, struct device_attribute *attr,/* * Detach from a device handler */ -scsi_dh_handler_detach (sdev); - err =0 ; +sdev_printk (KERN_WARNING , sdev, +"can't detach handler %s.\n" , + sdev->handler->name); + err = -EINVAL ; }else if (!strncmp (buf,"activate" ,8 )) {/* * Activate a device handler
Signature · 1 line
1.8.5.6
PATCH 9/10 scsi_dh: return SCSI_DH_NOTCONN in scsi_dh_activate()
In reply to Hannes ReineckeWhen calling scsi_dh_activate() we should be returning SCSI_DH_NOTCONN if the device handler couldn't be attached.
--- drivers/scsi/scsi_dh.c | 1 + 1 file changed, 1 insertion(+)
drivers/scsi/scsi_dh.c +1 -0
@@ -325,6 +325,7 @@ int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)if (!sdev->handler) goto out_fn; + err =SCSI_DH_NOTCONN ;if (sdev->sdev_state ==SDEV_CANCEL || sdev->sdev_state ==SDEV_DEL ) goto out_fn;
Signature · 1 line
1.8.5.6
PATCH 10/10 scsi_dh: move 'dh_state' sysfs attribute to generic code
In reply to Hannes ReineckeAs scsi_dh.c is now always compiled in we should be moving the 'dh_state' attribute to the generic code.
--- drivers/scsi/scsi_dh.c | 67 +---------------------------------------------- drivers/scsi/scsi_sysfs.c | 58 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 66 deletions(-)
drivers/scsi/scsi_dh.c +1 -66
@@ -153,75 +153,11 @@ static void scsi_dh_handler_detach(struct scsi_device *sdev)module_put (sdev->handler->module); } -/* -* Functions for sysfs attribute 'dh_state' -*/ -static ssize_t -store_dh_state (struct device *dev,struct device_attribute *attr, -const char *buf,size_t count) -{ -struct scsi_device *sdev =to_scsi_device (dev); -struct scsi_device_handler *scsi_dh; -int err = -EINVAL ; - -if (sdev->sdev_state ==SDEV_CANCEL || - sdev->sdev_state ==SDEV_DEL ) -return -ENODEV ; - -if (!sdev->handler) { -/* -* Attach to a device handler -*/ -if (!(scsi_dh =scsi_dh_lookup (buf))) -return err; - err =scsi_dh_handler_attach (sdev, scsi_dh); - }else { -if (!strncmp (buf,"detach" ,6 )) { -/* -* Detach from a device handler -*/ -sdev_printk (KERN_WARNING , sdev, -"can't detach handler %s.\n" , - sdev->handler->name); - err = -EINVAL ; - }else if (!strncmp (buf,"activate" ,8 )) { -/* -* Activate a device handler -*/ -if (sdev->handler->activate) - err = sdev->handler->activate (sdev,NULL ,NULL ); -else - err =0 ; - } - } - -return err<0 ?err:count; -} - -static ssize_t -show_dh_state (struct device *dev,struct device_attribute *attr,char *buf) -{ -struct scsi_device *sdev =to_scsi_device (dev); - -if (!sdev->handler) -return snprintf (buf,20 ,"detached\n" ); - -return snprintf (buf,20 ,"%s\n" , sdev->handler->name); -} - -static struct device_attribute scsi_dh_state_attr = -__ATTR (dh_state,S_IRUGO |S_IWUSR , show_dh_state, - store_dh_state); -int scsi_dh_add_device (struct scsi_device *sdev) {struct scsi_device_handler *devinfo =NULL ;const char *drv; -int err; - - err =device_create_file (&sdev->sdev_gendev, &scsi_dh_state_attr); -if (err) -return err; +int err =0 ; drv =scsi_dh_find_driver (sdev);if (drv)
@@ -235,7 +171,6 @@ void scsi_dh_remove_device(struct scsi_device *sdev) {if (sdev->handler)scsi_dh_handler_detach (sdev); -device_remove_file (&sdev->sdev_gendev, &scsi_dh_state_attr); }/*
drivers/scsi/scsi_sysfs.c +58 -0
76 lines not on this page · raw message
Signature · 1 line
1.8.5.6
James Bottomley
maintainerRe: [PATCH 10/10] scsi_dh: move 'dh_state' sysfs attribute to generic code
Quoted 4 lines · Hannes Reinecke
As
scsi_dh.cis now always compiled in we should be moving the 'dh_state' attribute to the generic code.Signed-off-by Hannes Reinecke <[email protected]>
This looks OK, but needs another reviewer. If you could also fix up the rejections caused by the if (!(scsi_dh = ...)) as well, that would be great.
It's independent, so I pulled in the rest.
Thanks,
James
Re: [PATCH 10/10] scsi_dh: move 'dh_state' sysfs attribute to generic code
In reply to Hannes ReineckeQuoted 4 lines · Hannes Reinecke
As
scsi_dh.cis now always compiled in we should be moving the 'dh_state' attribute to the generic code.Signed-off-by Hannes Reinecke <[email protected]>
Looks good,