| Message ID | 201106191348.24122.bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, 2011-06-19 at 13:48 +0200, Bart Van Assche wrote: > Avoid that SCSI scanning triggers creation of targets with non-zero > channel or non-zero id. Good catch, Bart. How did you find this? I would like to have a little more meat in the description. What real problem are we avoiding -- what happens when we get non-zero id/channel? Based on a quick look at the code -- I don't have ready access to a test system today -- I'm guessing that we create duplicate devices as a result of a manual scan? Signed-off-by: David Dillow <dillowda@ornl.gov> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jun 19, 2011 at 7:27 PM, David Dillow <dillowda@ornl.gov> wrote: > On Sun, 2011-06-19 at 13:48 +0200, Bart Van Assche wrote: >> Avoid that SCSI scanning triggers creation of targets with non-zero >> channel or non-zero id. > > Good catch, Bart. How did you find this? > > I would like to have a little more meat in the description. What real > problem are we avoiding -- what happens when we get non-zero id/channel? > > Based on a quick look at the code -- I don't have ready access to a test > system today -- I'm guessing that we create duplicate devices as a > result of a manual scan? Entirely correct. Some SRP target implementations allow to add and remove LUNs after an SRP initiator has logged in. In order to update the initiator a SCSI scan has to be initiated. Invoking the rescan-scsi-bus.sh script on an initiator learned me that many unwanted duplicate SCSI devices were created on the initiator. Regarding the implementation: a SCSI scan is initiated by writing directly or indirectly to /sys/class/scsi_host/host<number>/scan. If the transport associated with the involved host provides a custom scanning method, that method will be invoked. Otherwise scsi_scan_host_selected() will be invoked. Since the SRP transport does not provide a custom scanning method, for SRP the latter is invoked. It is that functin that creates duplicate SCSI device nodes if not prevented by target_alloc. Since which C:I:L values are valid depends on the LUN addressing method used by the initiator I opted to implement the filtering in the initiator driver instead of the transport. As you might have noticed, the filter proposed for the ibmvscsi driver differs slightly from the filter for the ib_srp driver. That is because the ibmvscsi and the ib_srp drivers use different LUN addressing methods (see also paragraph 4.7, Logical Unit Number (LUN) in the T10 SCSI Architecture Model standard). Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
We really shouldn't require workaround for this in every SCSI driver. scsi_scan_host_selected already makes sure never to scan above shost->max_channel. Could it be that you don't have that one set properly in the srp driver? It not we'll need to debug why it happens instead of adding hacks like this. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index ee165fd..de4fd32 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -110,6 +110,11 @@ static const char *srp_target_info(struct Scsi_Host *host) return host_to_target(host)->target_name; } +static int srp_target_alloc(struct scsi_target *starget) +{ + return starget->channel == 0 && starget->id == 0 ? 0 : -ENODEV; +} + static int srp_target_is_topspin(struct srp_target_port *target) { static const u8 topspin_oui[3] = { 0x00, 0x05, 0xad }; @@ -1836,6 +1841,7 @@ static struct scsi_host_template srp_template = { .name = "InfiniBand SRP initiator", .proc_name = DRV_NAME, .info = srp_target_info, + .target_alloc = srp_target_alloc, .queuecommand = srp_queuecommand, .eh_abort_handler = srp_abort, .eh_device_reset_handler = srp_reset_device,
Avoid that SCSI scanning triggers creation of targets with non-zero channel or non-zero id. Signed-off-by: Bart Van Assche <bvanassche@acm.org> Cc: Roland Dreier <roland@purestorage.com> Cc: David Dillow <dillowda@ornl.gov> Cc: <stable@kernel.org> --- drivers/infiniband/ulp/srp/ib_srp.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)