Skip to content

Commit

Permalink
[SCSI] remove target parent limitiation
Browse files Browse the repository at this point in the history
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.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
  • Loading branch information
Christoph Hellwig authored and James Bottomley committed Jan 14, 2006
1 parent 6d5b0c3 commit e02f3f5
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 53 deletions.
6 changes: 0 additions & 6 deletions drivers/scsi/scsi_priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ struct Scsi_Host;
#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);
Expand Down
6 changes: 5 additions & 1 deletion drivers/scsi/scsi_proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -200,7 +201,10 @@ static int scsi_add_single_device(uint host, uint channel, uint id, uint lun)
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;
}
Expand Down
16 changes: 2 additions & 14 deletions drivers/scsi/scsi_scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -334,19 +334,6 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
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__);
Expand Down Expand Up @@ -1283,8 +1270,9 @@ struct scsi_device *__scsi_add_device(struct Scsi_Host *shost, uint channel,
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);

Expand Down
5 changes: 4 additions & 1 deletion drivers/scsi/scsi_sysfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ static int scsi_scan(struct Scsi_Host *shost, const char *str)
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;
}

Expand Down
22 changes: 14 additions & 8 deletions drivers/scsi/scsi_transport_fc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1093,17 +1093,23 @@ static int fc_rport_match(struct attribute_container *cont,
/*
* 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 *
Expand Down Expand Up @@ -1142,7 +1148,7 @@ fc_attach_transport(struct fc_function_template *ft)
/* 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.
Expand Down
42 changes: 24 additions & 18 deletions drivers/scsi/scsi_transport_sas.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <linux/slab.h>
#include <linux/string.h>

#include <scsi/scsi.h>
#include <scsi/scsi_device.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_transport.h>
Expand Down Expand Up @@ -62,7 +63,7 @@ struct sas_internal {

struct sas_host_attrs {
struct list_head rphy_list;
spinlock_t lock;
struct mutex lock;
u32 next_target_id;
};
#define to_sas_host_attrs(host) ((struct sas_host_attrs *)(host)->shost_data)
Expand Down Expand Up @@ -165,7 +166,7 @@ static int sas_host_setup(struct transport_container *tc, struct device *dev,
struct sas_host_attrs *sas_host = to_sas_host_attrs(shost);

INIT_LIST_HEAD(&sas_host->rphy_list);
spin_lock_init(&sas_host->lock);
mutex_init(&sas_host->lock);
sas_host->next_target_id = 0;
return 0;
}
Expand Down Expand Up @@ -626,15 +627,15 @@ int sas_rphy_add(struct sas_rphy *rphy)
transport_add_device(&rphy->dev);
transport_configure_device(&rphy->dev);

spin_lock(&sas_host->lock);
mutex_lock(&sas_host->lock);
list_add_tail(&rphy->list, &sas_host->rphy_list);
if (identify->device_type == SAS_END_DEVICE &&
(identify->target_port_protocols &
(SAS_PROTOCOL_SSP|SAS_PROTOCOL_STP|SAS_PROTOCOL_SATA)))
rphy->scsi_target_id = sas_host->next_target_id++;
else
rphy->scsi_target_id = -1;
spin_unlock(&sas_host->lock);
mutex_unlock(&sas_host->lock);

if (rphy->scsi_target_id != -1) {
scsi_scan_target(&rphy->dev, parent->number,
Expand All @@ -661,9 +662,9 @@ void sas_rphy_free(struct sas_rphy *rphy)
struct Scsi_Host *shost = dev_to_shost(rphy->dev.parent->parent);
struct sas_host_attrs *sas_host = to_sas_host_attrs(shost);

spin_lock(&sas_host->lock);
mutex_lock(&sas_host->lock);
list_del(&rphy->list);
spin_unlock(&sas_host->lock);
mutex_unlock(&sas_host->lock);

transport_destroy_device(&rphy->dev);
put_device(rphy->dev.parent);
Expand Down Expand Up @@ -703,9 +704,9 @@ sas_rphy_delete(struct sas_rphy *rphy)
device_del(dev);
transport_destroy_device(dev);

spin_lock(&sas_host->lock);
mutex_lock(&sas_host->lock);
list_del(&rphy->list);
spin_unlock(&sas_host->lock);
mutex_unlock(&sas_host->lock);

parent->rphy = NULL;

Expand All @@ -731,23 +732,28 @@ EXPORT_SYMBOL(scsi_is_sas_rphy);
* SCSI scan helper
*/

static struct device *sas_target_parent(struct Scsi_Host *shost,
int channel, uint id)
static int sas_user_scan(struct Scsi_Host *shost, uint channel,
uint id, uint lun)
{
struct sas_host_attrs *sas_host = to_sas_host_attrs(shost);
struct sas_rphy *rphy;
struct device *dev = NULL;

spin_lock(&sas_host->lock);
mutex_lock(&sas_host->lock);
list_for_each_entry(rphy, &sas_host->rphy_list, list) {
struct sas_phy *parent = dev_to_phy(rphy->dev.parent);
if (parent->number == channel &&
rphy->scsi_target_id == id)
dev = &rphy->dev;

if (rphy->scsi_target_id == -1)
continue;

if ((channel == SCAN_WILD_CARD || channel == parent->number) &&
(id == SCAN_WILD_CARD || id == rphy->scsi_target_id)) {
scsi_scan_target(&rphy->dev, parent->number,
rphy->scsi_target_id, lun, 1);
}
}
spin_unlock(&sas_host->lock);
mutex_unlock(&sas_host->lock);

return dev;
return 0;
}


Expand Down Expand Up @@ -792,7 +798,7 @@ sas_attach_transport(struct sas_function_template *ft)
return NULL;
memset(i, 0, sizeof(struct sas_internal));

i->t.target_parent = sas_target_parent;
i->t.user_scan = sas_user_scan;

i->t.host_attrs.ac.attrs = &i->host_attrs[0];
i->t.host_attrs.ac.class = &sas_host_class.class;
Expand Down
6 changes: 6 additions & 0 deletions include/scsi/scsi.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ extern const unsigned char scsi_command_size[8];
#define MAX_SCSI_DEVICE_CODE 15
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
*/
Expand Down
7 changes: 2 additions & 5 deletions include/scsi/scsi_transport.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,9 @@ struct scsi_transport_template {
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
Expand Down

0 comments on commit e02f3f5

Please sign in to comment.