driver model for scsi upper drivers, take 2
Subsystems (3)
PATCH 1/1 driver model for scsi upper drivers, take 2
- removed the tape sysfs pseudodevice crap that caused hangs
- switched sg to a class_interface. This means sg can be used on devices already claimed be an upper driver again. This also means I had to remove the sg sysfs attributes temporarily because the old mechanism is gone, but I'll restore them differently in a followon patch.
drivers/scsi/hosts.c +6 -15
@@ -229,27 +229,18 @@int scsi_add_host (struct Scsi_Host *shost,struct device *dev) {Scsi_Host_Template *sht = shost->hostt; -struct scsi_device *sdev; -int error =0 , saved_error =0 ; +int error;printk (KERN_INFO "scsi%d : %s\n" , shost->host_no, sht->info ? sht->info (shost) : sht->name); error =scsi_sysfs_add_host (shost, dev); -if (error) -return error; - -scsi_proc_host_add (shost); - -scsi_scan_host (shost); +if (!error) { +scsi_proc_host_add (shost); +scsi_scan_host (shost); + }; - list_for_each_entry (sdev, &shost->my_devices, siblings) { - error =scsi_attach_device (sdev); -if (error) - saved_error = error; - } - -return saved_error; +return error; }/**
===== drivers/scsi/hosts.h 1.63 vs edited =====
drivers/scsi/hosts.h +10 -17
@@ -532,25 +532,18 @@return shost->host_gendev.parent; } -struct Scsi_Device_Template -{ -struct list_head list; -const char * name; -struct module * module;/* Used for loadable modules */ -unsigned char scsi_type; -int (*attach)(Scsi_Device *);/* Attach devices to arrays */ -void (*detach)(Scsi_Device *); -int (*init_command)(Scsi_Cmnd *);/* Used by new queueing code. -Selects command for blkdevs */ -void (*rescan)(Scsi_Device *); -struct device_driver scsi_driverfs_driver; +struct scsi_driver { +struct module *owner; +struct device_driver gendrv; + +int (*init_command)(struct scsi_cmnd *); +void (*rescan)(struct device *); }; +#define to_scsi_driver (drv) \ + container_of((drv), struct scsi_driver, gendrv) -/* -* Highlevel driver registration/unregistration. -*/ -extern int scsi_register_device (struct Scsi_Device_Template *); -extern int scsi_unregister_device (struct Scsi_Device_Template *); +extern int scsi_register_driver (struct scsi_driver *); +extern void scsi_unregister_driver (struct scsi_driver *);/* * HBA allocation/freeing.
===== drivers/scsi/ide-scsi.c 1.23 vs edited =====
drivers/scsi/ide-scsi.c +2 -2
@@ -770,8 +770,8 @@struct gendisk *disk = cmd->request->rq_disk;if (disk) { -struct Scsi_Device_Template **p = disk->private_data; -if (strcmp ((*p)->scsi_driverfs_driver.name,"sg" ) ==0 ) +struct scsi_driver **p = disk->private_data; +if (strcmp ((*p)->gendrv.name,"sg" ) ==0 )return test_bit (IDESCSI_SG_TRANSFORM , &scsi->transform); }return test_bit (IDESCSI_TRANSFORM , &scsi->transform);
===== drivers/scsi/osst.c 1.43 vs edited =====
drivers/scsi/osst.c +20 -22
100 lines not on this page · raw message
===== drivers/scsi/osst.h 1.9 vs edited =====
drivers/scsi/osst.h +1 -1
@@ -530,7 +530,7 @@/* The OnStream tape drive descriptor */ typedef struct { - struct Scsi_Device_Template *driver; + struct scsi_driver *driver; unsigned capacity; Scsi_Device* device; struct semaphore lock; /* for serialization */
===== drivers/scsi/scsi.c 1.112 vs edited =====
drivers/scsi/scsi.c +0 -115
133 lines not on this page · raw message
===== drivers/scsi/scsi_lib.c 1.90 vs edited =====
drivers/scsi/scsi_lib.c +4 -32
@@ -613,29 +613,6 @@ }/* -* Function: scsi_get_request_dev() -* -* Purpose: Find the upper-level driver that is responsible for this -* request -* -* Arguments: request - I/O request we are preparing to queue. -* -* Lock status: No locks assumed to be held, but as it happens the -* q->queue_lock is held when this is called. -* -* Returns: Nothing -* -* Notes: The requests in the request queue may have originated -* from any block device driver. We need to find out which -* one so that we can later form the appropriate command. -*/ -static struct Scsi_Device_Template *scsi_get_request_dev (struct request *req) -{ -struct gendisk *p = req->rq_disk; -return p ? *(struct Scsi_Device_Template **)p->private_data :NULL ; -} - -/* * Function: scsi_io_completion() * * Purpose: Completion processing for block device I/O requests.
@@ -847,11 +824,7 @@return ; }if (result) { -struct Scsi_Device_Template *sdt; - - sdt =scsi_get_request_dev (cmd->request); - printk("SCSI %s error : <%d %d %d %d> return code = 0x%x\n", - (sdt ? sdt->name : "device"), + printk("SCSI error : <%d %d %d %d> return code = 0x%x\n", cmd->device->host->host_no, cmd->device->channel, cmd->device->id,
@@ -945,7 +918,6 @@static int scsi_prep_fn (struct request_queue *q,struct request *req) { -struct Scsi_Device_Template *sdt;struct scsi_device *sdev = q->queuedata;struct scsi_cmnd *cmd;
@@ -1001,6 +973,7 @@* happening now. */ if (req->flags & (REQ_CMD |REQ_BLOCK_PC )) { +struct scsi_driver *drv;int ret;/*
@@ -1015,8 +988,6 @@* some kinds of consistency checking may cause the * request to be rejected immediately. */ - sdt =scsi_get_request_dev (req); -BUG_ON (!sdt);/* * This sets up the scatter-gather table (allocating if
@@ -1029,7 +1000,8 @@/* * Initialize the actual SCSI command for this request. */ -if (unlikely (!sdt->init_command (cmd))) { + drv = *(struct scsi_driver **)req->rq_disk->private_data; +if (unlikely (!drv->init_command (cmd))) {scsi_release_buffers (cmd);scsi_put_command (cmd);return BLKPREP_KILL ;
===== drivers/scsi/scsi_priv.h 1.6 vs edited =====
drivers/scsi/scsi_priv.h +1 -8
@@ -41,9 +41,6 @@#define SCSI_SENSE_VALID (scmd) \ (((scmd)->sense_buffer[0] & 0x70) == 0x70) -struct Scsi_Device_Template ; - -/* * scsi_target: representation of a scsi target, for now, this is only * used for single_lun devices. If no one has active IO to the target,
@@ -68,9 +65,6 @@extern void scsi_done (struct scsi_cmnd *cmd);extern void scsi_finish_command (struct scsi_cmnd *cmd);extern int scsi_retry_command (struct scsi_cmnd *cmd); -extern int scsi_attach_device (struct scsi_device *sdev); -extern void scsi_detach_device (struct scsi_device *sdev); -extern void scsi_rescan_device (struct scsi_device *sdev);extern int scsi_insert_special_req (struct scsi_request *sreq,int );extern void scsi_init_cmd_from_req (struct scsi_cmnd *cmd,struct scsi_request *sreq);
@@ -117,12 +111,11 @@extern void scsi_free_sdev (struct scsi_device *);extern void scsi_free_shost (struct Scsi_Host *);extern void scsi_host_get (struct Scsi_Host *); +extern void scsi_rescan_device (struct device *dev);/* scsi_sysfs.c */ extern int scsi_device_register (struct scsi_device *);extern void scsi_device_unregister (struct scsi_device *); -extern int scsi_upper_driver_register (struct Scsi_Device_Template *); -extern void scsi_upper_driver_unregister (struct Scsi_Device_Template *);extern void scsi_sysfs_init_host (struct Scsi_Host *);extern int scsi_sysfs_add_host (struct Scsi_Host *,struct device *);extern void scsi_sysfs_remove_host (struct Scsi_Host *);
===== drivers/scsi/scsi_scan.c 1.85 vs edited =====
drivers/scsi/scsi_scan.c +14 -7
@@ -1091,22 +1091,29 @@ uint channel, uint id, uint lun) {struct scsi_device *sdev; -int error = -ENODEV , res; +int res; res =scsi_probe_and_add_lun (shost, channel, id, lun,NULL , &sdev); -if (res ==SCSI_SCAN_LUN_PRESENT ) - error =scsi_attach_device (sdev); - -if (error) - sdev =ERR_PTR (error); +if (res !=SCSI_SCAN_LUN_PRESENT ) + sdev =ERR_PTR (-ENODEV );return sdev; }int scsi_remove_device (struct scsi_device *sdev) { -scsi_detach_device (sdev);scsi_device_unregister (sdev);return 0 ; +} + +void scsi_rescan_device (struct device *dev) +{ +struct scsi_driver *drv =to_scsi_driver (dev->driver); + +if (try_module_get (drv->owner)) { +if (drv->rescan) + drv->rescan (dev); +module_put (drv->owner); + } }/**
===== drivers/scsi/scsi_syms.c 1.36 vs edited =====
drivers/scsi/scsi_syms.c +2 -2
@@ -29,8 +29,8 @@* This source file contains the symbol table used by scsi loadable * modules. */ -EXPORT_SYMBOL (scsi_register_device); -EXPORT_SYMBOL (scsi_unregister_device); +EXPORT_SYMBOL (scsi_register_driver); +EXPORT_SYMBOL (scsi_unregister_driver);EXPORT_SYMBOL (scsi_register_host);EXPORT_SYMBOL (scsi_unregister_host);EXPORT_SYMBOL (scsi_add_host);
===== drivers/scsi/scsi_sysfs.c 1.16 vs edited =====
drivers/scsi/scsi_sysfs.c +17 -53
103 lines not on this page · raw message
===== drivers/scsi/sd.c 1.112 vs edited =====
drivers/scsi/sd.c +48 -114
273 lines not on this page · raw message
===== drivers/scsi/sg.c 1.54 vs edited =====
drivers/scsi/sg.c +34 -51
187 lines not on this page · raw message
===== drivers/scsi/sr.c 1.78 vs edited =====
drivers/scsi/sr.c +20 -58
138 lines not on this page · raw message
===== drivers/scsi/sr.h 1.9 vs edited =====
drivers/scsi/sr.h +1 -2
@@ -25,8 +25,7 @@#define IOCTL_TIMEOUT 30*HZ typedef struct scsi_cd { - struct Scsi_Device_Template *driver; - struct list_head list; + struct scsi_driver *driver; unsigned capacity; /* size in blocks */ Scsi_Device *device; unsigned int vendor; /* vendor code, see sr_vendor.c */
===== drivers/scsi/st.c 1.61 vs edited =====
drivers/scsi/st.c +22 -33
129 lines not on this page · raw message
===== drivers/scsi/st.h 1.13 vs edited =====
drivers/scsi/st.h +1 -1
@@ -70,7 +70,7 @@/* The tape drive descriptor */ typedef struct { - struct Scsi_Device_Template *driver; + struct scsi_driver *driver; Scsi_Device *device; struct semaphore lock; /* For serialization */ struct completion wait; /* For SCSI commands */
Quoted 6 lines · Christoph Hellwig
- removed the tape sysfs pseudodevice crap that caused hangs
- switched sg to aclass_interface. This means sg can be used on devices already claimed be an upper driver again. This also means I had to remove the sg sysfs attributes temporarily because the old mechanism is gone, but I'll restore them differently in a followon patch.
Yikes, this was the old patch again. Here's the right one:
drivers/scsi/hosts.c +6 -15
@@ -233,27 +233,18 @@int scsi_add_host (struct Scsi_Host *shost,struct device *dev) {Scsi_Host_Template *sht = shost->hostt; -struct scsi_device *sdev; -int error =0 , saved_error =0 ; +int error;printk (KERN_INFO "scsi%d : %s\n" , shost->host_no, sht->info ? sht->info (shost) : sht->name); error =scsi_sysfs_add_host (shost, dev); -if (error) -return error; - -scsi_proc_host_add (shost); - -scsi_scan_host (shost); +if (!error) { +scsi_proc_host_add (shost); +scsi_scan_host (shost); + }; - list_for_each_entry (sdev, &shost->my_devices, siblings) { - error =scsi_attach_device (sdev); -if (error) - saved_error = error; - } - -return saved_error; +return error; }/**
drivers/scsi/hosts.h +15 -17
@@ -542,25 +542,23 @@return shost->host_gendev.parent; } -struct Scsi_Device_Template -{ -struct list_head list; -const char * name; -struct module * module;/* Used for loadable modules */ -unsigned char scsi_type; -int (*attach)(Scsi_Device *);/* Attach devices to arrays */ -void (*detach)(Scsi_Device *); -int (*init_command)(Scsi_Cmnd *);/* Used by new queueing code. -Selects command for blkdevs */ -void (*rescan)(Scsi_Device *); -struct device_driver scsi_driverfs_driver; +struct scsi_driver { +struct module *owner; +struct device_driver gendrv; + +int (*init_command)(struct scsi_cmnd *); +void (*rescan)(struct device *); }; +#define to_scsi_driver (drv) \ + container_of((drv), struct scsi_driver, gendrv) + +extern int scsi_register_driver (struct device_driver *); +#define scsi_unregister_driver (drv) \ + driver_unregister(drv); -/* -* Highlevel driver registration/unregistration. -*/ -extern int scsi_register_device (struct Scsi_Device_Template *); -extern int scsi_unregister_device (struct Scsi_Device_Template *); +extern int scsi_register_interface (struct class_interface *); +#define scsi_unregister_interface (intf) \ + class_interface_unregister(intf)/* * HBA allocation/freeing.
drivers/scsi/ide-scsi.c +5 -0
@@ -771,6 +771,10 @@static inline int should_transform (ide_drive_t *drive,Scsi_Cmnd *cmd) {idescsi_scsi_t *scsi =drive_to_idescsi (drive); + +/* this was a layering violation and we can't support it +anymore, sorry. */ +#if 0 struct gendisk *disk = cmd->request->rq_disk;if (disk) {
@@ -778,6 +782,7 @@if (strcmp ((*p)->scsi_driverfs_driver.name,"sg" ) ==0 ) return test_bit(IDESCSI_SG_TRANSFORM, &scsi->transform); } +#endif return test_bit (IDESCSI_TRANSFORM , &scsi->transform); }
drivers/scsi/osst.c +20 -83
185 lines not on this page · raw message
drivers/scsi/osst.h +1 -3
@@ -530,7 +530,7 @@/* The OnStream tape drive descriptor */ typedef struct { - struct Scsi_Device_Template *driver; + struct scsi_driver *driver; unsigned capacity; Scsi_Device* device; struct semaphore lock; /* for serialization */
@@ -555,8 +555,6 @@/* Mode characteristics */ ST_mode modes[ST_NBR_MODES ];int current_mode; -struct device driverfs_dev_r[ST_NBR_MODES ]; -struct device driverfs_dev_n[ST_NBR_MODES ];/* Status variables */ int partition;
drivers/scsi/scsi.c +0 -115
133 lines not on this page · raw message
drivers/scsi/scsi.h +1 -0
@@ -316,6 +316,7 @@*/ struct scsi_device { +struct class_device sdev_classdev;/* * This information is private to the scsi mid-layer. */
drivers/scsi/scsi_lib.c +4 -32
@@ -613,29 +613,6 @@ }/* -* Function: scsi_get_request_dev() -* -* Purpose: Find the upper-level driver that is responsible for this -* request -* -* Arguments: request - I/O request we are preparing to queue. -* -* Lock status: No locks assumed to be held, but as it happens the -* q->queue_lock is held when this is called. -* -* Returns: Nothing -* -* Notes: The requests in the request queue may have originated -* from any block device driver. We need to find out which -* one so that we can later form the appropriate command. -*/ -static struct Scsi_Device_Template *scsi_get_request_dev (struct request *req) -{ -struct gendisk *p = req->rq_disk; -return p ? *(struct Scsi_Device_Template **)p->private_data :NULL ; -} - -/* * Function: scsi_io_completion() * * Purpose: Completion processing for block device I/O requests.
@@ -849,11 +826,7 @@return ; }if (result) { -struct Scsi_Device_Template *sdt; - - sdt =scsi_get_request_dev (cmd->request); - printk("SCSI %s error : <%d %d %d %d> return code = 0x%x\n", - (sdt ? sdt->name : "device"), + printk("SCSI error : <%d %d %d %d> return code = 0x%x\n", cmd->device->host->host_no, cmd->device->channel, cmd->device->id,
@@ -947,7 +920,6 @@static int scsi_prep_fn (struct request_queue *q,struct request *req) { -struct Scsi_Device_Template *sdt;struct scsi_device *sdev = q->queuedata;struct scsi_cmnd *cmd;
@@ -1003,6 +975,7 @@* happening now. */ if (req->flags & (REQ_CMD |REQ_BLOCK_PC )) { +struct scsi_driver *drv;int ret;/*
@@ -1017,8 +990,6 @@* some kinds of consistency checking may cause the * request to be rejected immediately. */ - sdt =scsi_get_request_dev (req); -BUG_ON (!sdt);/* * This sets up the scatter-gather table (allocating if
@@ -1031,7 +1002,8 @@/* * Initialize the actual SCSI command for this request. */ -if (unlikely (!sdt->init_command (cmd))) { + drv = *(struct scsi_driver **)req->rq_disk->private_data; +if (unlikely (!drv->init_command (cmd))) {scsi_release_buffers (cmd);scsi_put_command (cmd);return BLKPREP_KILL ;
drivers/scsi/scsi_priv.h +1 -8
@@ -41,9 +41,6 @@#define SCSI_SENSE_VALID (scmd) \ (((scmd)->sense_buffer[0] & 0x70) == 0x70) -struct Scsi_Device_Template ; - -/* * scsi_target: representation of a scsi target, for now, this is only * used for single_lun devices. If no one has active IO to the target,
@@ -70,9 +67,6 @@extern void scsi_done (struct scsi_cmnd *cmd);extern void scsi_finish_command (struct scsi_cmnd *cmd);extern int scsi_retry_command (struct scsi_cmnd *cmd); -extern int scsi_attach_device (struct scsi_device *sdev); -extern void scsi_detach_device (struct scsi_device *sdev); -extern void scsi_rescan_device (struct scsi_device *sdev);extern int scsi_insert_special_req (struct scsi_request *sreq,int );extern void scsi_init_cmd_from_req (struct scsi_cmnd *cmd,struct scsi_request *sreq);
@@ -119,12 +113,11 @@extern void scsi_free_sdev (struct scsi_device *);extern void scsi_free_shost (struct Scsi_Host *);extern void scsi_host_get (struct Scsi_Host *); +extern void scsi_rescan_device (struct device *dev);/* scsi_sysfs.c */ extern int scsi_device_register (struct scsi_device *);extern void scsi_device_unregister (struct scsi_device *); -extern int scsi_upper_driver_register (struct Scsi_Device_Template *); -extern void scsi_upper_driver_unregister (struct Scsi_Device_Template *);extern void scsi_sysfs_init_host (struct Scsi_Host *);extern int scsi_sysfs_add_host (struct Scsi_Host *,struct device *);extern void scsi_sysfs_remove_host (struct Scsi_Host *);
drivers/scsi/scsi_scan.c +14 -7
@@ -1098,22 +1098,29 @@ uint channel, uint id, uint lun) {struct scsi_device *sdev; -int error = -ENODEV , res; +int res; res =scsi_probe_and_add_lun (shost, channel, id, lun,NULL , &sdev); -if (res ==SCSI_SCAN_LUN_PRESENT ) - error =scsi_attach_device (sdev); - -if (error) - sdev =ERR_PTR (error); +if (res !=SCSI_SCAN_LUN_PRESENT ) + sdev =ERR_PTR (-ENODEV );return sdev; }int scsi_remove_device (struct scsi_device *sdev) { -scsi_detach_device (sdev);scsi_device_unregister (sdev);return 0 ; +} + +void scsi_rescan_device (struct device *dev) +{ +struct scsi_driver *drv =to_scsi_driver (dev->driver); + +if (try_module_get (drv->owner)) { +if (drv->rescan) + drv->rescan (dev); +module_put (drv->owner); + } }/**
drivers/scsi/scsi_syms.c +2 -2
@@ -29,8 +29,8 @@* This source file contains the symbol table used by scsi loadable * modules. */ -EXPORT_SYMBOL (scsi_register_device); -EXPORT_SYMBOL (scsi_unregister_device); +EXPORT_SYMBOL (scsi_register_driver); +EXPORT_SYMBOL (scsi_register_interface);EXPORT_SYMBOL (scsi_register_host);EXPORT_SYMBOL (scsi_unregister_host);EXPORT_SYMBOL (scsi_add_host);
drivers/scsi/scsi_sysfs.c +51 -56
166 lines not on this page · raw message
drivers/scsi/sd.c +48 -114
273 lines not on this page · raw message
drivers/scsi/sg.c +18 -128
237 lines not on this page · raw message
drivers/scsi/sr.c +20 -58
138 lines not on this page · raw message
drivers/scsi/sr.h +1 -2
@@ -25,8 +25,7 @@#define IOCTL_TIMEOUT 30*HZ typedef struct scsi_cd { - struct Scsi_Device_Template *driver; - struct list_head list; + struct scsi_driver *driver; unsigned capacity; /* size in blocks */ Scsi_Device *device; unsigned int vendor; /* vendor code, see sr_vendor.c */
drivers/scsi/st.c +22 -75
188 lines not on this page · raw message
===== drivers/scsi/st.h 1.13 vs edited =====
drivers/scsi/st.h +1 -3
@@ -70,7 +70,7 @@/* The tape drive descriptor */ typedef struct { - struct Scsi_Device_Template *driver; + struct scsi_driver *driver; Scsi_Device *device; struct semaphore lock; /* For serialization */ struct completion wait; /* For SCSI commands */
@@ -103,8 +103,6 @@/* Mode characteristics */ ST_mode modes[ST_NBR_MODES ];int current_mode; -struct device driverfs_dev_r[ST_NBR_MODES ]; -struct device driverfs_dev_n[ST_NBR_MODES ];/* Status variables */ int partition;
Quoted 10 lines · Christoph Hellwig
Quoted 6 lines · Christoph Hellwig
- removed the tape sysfs pseudodevice crap that caused hangs
- switched sg to aclass_interface. This means sg can be used on devices already claimed be an upper driver again. This also means I had to remove the sg sysfs attributes temporarily because the old mechanism is gone, but I'll restore them differently in a followon patch.Yikes, this was the old patch again. Here's the right one:
I have done some tests with a disk, a tape, and different combinations (in kernel/module) of the sd, st, sg, aic7xxx, and sym53c8xx_2 drivers. Looks good: devices are found and seem to work, no oopses when loading/unloading modules. sysfs contents look correct (except for the missing sg entries you said will re-appear in future).
Signature · 1 line
Kai
A message that is not in the archive (sent off-list or not yet received): "from hch"@lst.de
Quoted 25 lines · Christoph Hellwig
Quoted 6 lines · Christoph Hellwig
- removed the tape sysfs pseudodevice crap that caused hangs
- switched sg to aclass_interface. This means sg can be used on devices already claimed be an upper driver again. This also means I had to remove the sg sysfs attributes temporarily because the old mechanism is gone, but I'll restore them differently in a followon patch.Yikes, this was the old patch again. Here's the right one:
drivers/scsi/ide-scsi.c +4
-0@@ -771,6 +771,10 @@static inline int should_transform (ide_drive_t *drive,Scsi_Cmnd *cmd) {idescsi_scsi_t *scsi =drive_to_idescsi (drive); + +/* this was a layering violation and we can't support it +anymore, sorry. */ +#if 0 struct gendisk *disk = cmd->request->rq_disk;if (disk) {
drivers/scsi/ide-scsi.c line 785
Quoted 4 lines
drivers/scsi/ide-scsi.c +1
-0@@ -778,6 +782,7 @@if (strcmp ((*p)->scsi_driverfs_driver.name,"sg" ) ==0 ) return test_bit(IDESCSI_SG_TRANSFORM, &scsi->transform); } +#endif
You removed whatever capability was provided by IDESCSI_SG_TRANSFORM. Just curious, what did that do? And, does that capability need to be replaced, and if so, any suggestions how to do so?
Thanks, Willem Riede.