remove target parent limitiation
PATCH 1/1 remove target parent limitiation
When James Smart fixed the issue of the userspace scan atributes crashing the system with the FC transport class he added a patch to let the transport class check if the parent is valid for a given transport class.
When adding support for the integrated raid of fusion sas devices we ran into a problem with that, as it didn't allow adding virtual raid volumes without the transport class knowing about it.
So this patch adds a user_scan attribute instead, that takes over from scsi_scan_host_selected if the transport class sets it and thus lets the transport class control the user-initiated scanning. As this plugs the hole about user-initiated scanning the target_parent hook goes away and we rely on callers of the scanning routines to do something sensible.
For SAS this meant I had to switch from a spinlock to a mutex to synchronize the topology linked lists, in FC they were completely unsynchronized which seems wrong.
drivers/scsi/scsi_priv.h +0 -6
@@ -26,12 +26,6 @@#define SCSI_SENSE_VALID (scmd) \ (((scmd)->sense_buffer[0] & 0x70) == 0x70) -/* -* Special value for scanning to specify scanning or rescanning of all -* possible channels, (target) ids, or luns on a given shost. -*/ -#define SCAN_WILD_CARD ~0 -/* hosts.c */ extern int scsi_init_hosts (void );extern void scsi_exit_hosts (void );
drivers/scsi/scsi_scan.c +2 -14
@@ -334,19 +334,6 @@struct scsi_target *starget;struct scsi_target *found_target; -/* -* Obtain the real parent from the transport. The transport -* is allowed to fail (no error) if there is nothing at that -* target id. -*/ -if (shost->transportt->target_parent) { -spin_lock_irqsave (shost->host_lock, flags); - parent = shost->transportt->target_parent (shost, channel, id); -spin_unlock_irqrestore (shost->host_lock, flags); -if (!parent) -return NULL ; - } - starget =kmalloc (size,GFP_KERNEL );if (!starget) {printk (KERN_ERR "%s: allocation failure\n" , __FUNCTION__);
@@ -1283,8 +1270,9 @@struct scsi_device *sdev;struct device *parent = &shost->shost_gendev;int res; -struct scsi_target *starget =scsi_alloc_target (parent, channel, id); +struct scsi_target *starget; + starget =scsi_alloc_target (parent, channel, id);if (!starget)return ERR_PTR (-ENOMEM );
drivers/scsi/scsi_transport_sas.c +24 -18
106 lines not on this page · raw message
include/scsi/scsi.h +6 -0
@@ -32,6 +32,12 @@extern const char *const scsi_device_types[MAX_SCSI_DEVICE_CODE ];/* +* Special value for scanning to specify scanning or rescanning of all +* possible channels, (target) ids, or luns on a given shost. +*/ +#define SCAN_WILD_CARD ~0 + +/* * SCSI opcodes */
include/scsi/scsi_transport.h +2 -5
@@ -30,12 +30,9 @@struct transport_container device_attrs;/* -* If set, call target_parent prior to allocating a scsi_target, -* so we get the appropriate parent for the target. This function -* is required for transports like FC and iSCSI that do not put the -* scsi_target under scsi_host. +* If set, called from sysfs and legacy procfs rescanning code. */ -struct device *(*target_parent)(struct Scsi_Host *,int ,uint ); +int (*user_scan)(struct Scsi_Host *,uint ,uint ,uint );/* The size of the specific transport attribute structure (a * space of this size will be left at the end of the
drivers/scsi/scsi_proc.c +5 -1
@@ -31,6 +31,7 @@#include <scsi/scsi.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_transport.h> #include "scsi_priv.h" #include "scsi_logging.h"
@@ -200,7 +201,10 @@if (IS_ERR (shost))return PTR_ERR (shost); - error =scsi_scan_host_selected (shost, channel, id, lun,1 ); +if (shost->transportt->user_scan) + error = shost->transportt->user_scan (shost, channel, id, lun); +else + error =scsi_scan_host_selected (shost, channel, id, lun,1 );scsi_host_put (shost);return error; }
drivers/scsi/scsi_sysfs.c +4 -1
@@ -106,7 +106,10 @@return -EINVAL ;if (check_set (&lun, s3))return -EINVAL ; - res =scsi_scan_host_selected (shost, channel, id, lun,1 ); +if (shost->transportt->user_scan) + res = shost->transportt->user_scan (shost, channel, id, lun); +else + res =scsi_scan_host_selected (shost, channel, id, lun,1 );return res; }
drivers/scsi/scsi_transport_fc.c +14 -8
@@ -1090,17 +1090,23 @@/* * Must be called with shost->host_lock held */ -static struct device *fc_target_parent (struct Scsi_Host *shost, -int channel,uint id) +static int fc_user_scan (struct Scsi_Host *shost,uint channel, +uint id,uint lun) {struct fc_rport *rport; -list_for_each_entry (rport, &fc_host_rports (shost), peers) -if ((rport->channel == channel) && - (rport->scsi_target_id == id)) -return &rport->dev; +list_for_each_entry (rport, &fc_host_rports (shost), peers) { +if (rport->scsi_target_id ==-1 ) +continue ; -return NULL ; +if ((channel ==SCAN_WILD_CARD || channel == rport->channel) && + (id ==SCAN_WILD_CARD || id == rport->scsi_target_id)) { +scsi_scan_target (&rport->dev, rport->channel, + rport->scsi_target_id, lun,1 ); + } + } + +return 0 ; } struct scsi_transport_template *
@@ -1139,7 +1145,7 @@/* Transport uses the shost workq for scsi scanning */ i->t.create_work_queue =1 ; - i->t.target_parent = fc_target_parent; + i->t.user_scan = fc_user_scan;/* * Setup SCSI Target Attributes.
Quoted 9 lines · Christoph Hellwig
===================================================================
drivers/scsi/scsi_scan.c +0
-2@@ -334,19 +334,6 @@struct scsi_target *starget;struct scsi_target *found_target; -/* -* Obtain the real parent from the transport. The transport
drivers/scsi/scsi_scan.c line 348
Quoted 10 lines
drivers/scsi/scsi_scan.c +0
-10-* is allowed to fail (no error) if there is nothing at that -* target id. -*/ -if (shost->transportt->target_parent) { -spin_lock_irqsave (shost->host_lock, flags); - parent = shost->transportt->target_parent (shost, channel, id); -spin_unlock_irqrestore (shost->host_lock, flags); -if (!parent) -return NULL ; - }
Should we remove the target_parent callout from the trasnport_template since it is no longer used or does it make sense to keep it as part of the API?
Quoted 3 lines
Should we remove the
target_parentcallout from thetrasnport_templatesince it is no longer used or does it make sense to keep it as part of the API?
In the patch I have hear I did that:
- struct device *(*target_parent)(struct Scsi_Host *, int, uint);
+ int (*user_scan)(struct Scsi_Host *, uint, uint, uint);
include/scsi/scsi_transport.h line 35
Quoted 7 lines · Christoph Hellwig
Quoted 1 line
Should we remove the
target_parentcallout from thetrasnport_templatesince it is no longer used or does it make sense to keep it as part of the API?In the patch I have hear I did that:
include/scsi/scsi_transport.h +1
-1-struct device *(*target_parent)(struct Scsi_Host *,int ,uint ); +int (*user_scan)(struct Scsi_Host *,uint ,uint ,uint );
oops sorry.