[PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[Bart Van Assche]

SCSI scanning of a channel:id:lun triplet in Linux works as follows (function
scsi_scan_target() in drivers/scsi/scsi_scan.c):
- If lun == SCAN_WILD_CARD, send a REPORT LUNS command to the target and
  process the result.
- If lun != SCAN_WILD_CARD, send an INQUIRY command to the LUN corresponding
  to the specified channel:id:lun triplet to verify whether the LUN exists.
So a SCSI driver must either take the channel and target id values in account
in its quecommand() function or it should declare that it only supports one
channel and one target id. Currently the ib_srp driver does neither. As a
result scanning the SCSI bus via e.g. rescan-scsi-bus.sh causes many
duplicate SCSI devices to be created. For each 0:0:L device, several
duplicates are created with the same LUN number and with (C:I) != (0:0). Fix
this by declaring that the ib_srp driver only supports one channel and one
target id.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Cc: <stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

---
 drivers/infiniband/ulp/srp/ib_srp.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index ee165fd..7d5109b 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2127,6 +2127,8 @@ static ssize_t srp_create_target(struct device *dev,
 		return -ENOMEM;
 
 	target_host->transportt  = ib_srp_transport_template;
+	target_host->max_channel = 0;
+	target_host->max_id      = 1;
 	target_host->max_lun     = SRP_MAX_LUN;
 	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[Christoph Hellwig]

On Sat, Jun 25, 2011 at 10:10:30AM +0200, Bart Van Assche wrote:
> SCSI scanning of a channel:id:lun triplet in Linux works as follows (function
> scsi_scan_target() in drivers/scsi/scsi_scan.c):
> - If lun == SCAN_WILD_CARD, send a REPORT LUNS command to the target and
>   process the result.
> - If lun != SCAN_WILD_CARD, send an INQUIRY command to the LUN corresponding
>   to the specified channel:id:lun triplet to verify whether the LUN exists.
> So a SCSI driver must either take the channel and target id values in account
> in its quecommand() function or it should declare that it only supports one
> channel and one target id. Currently the ib_srp driver does neither. As a
> result scanning the SCSI bus via e.g. rescan-scsi-bus.sh causes many
> duplicate SCSI devices to be created. For each 0:0:L device, several
> duplicates are created with the same LUN number and with (C:I) != (0:0). Fix
> this by declaring that the ib_srp driver only supports one channel and one
> target id.

Looks good.  We should probably consider chosing sane defaults at least
for max_channel, but for now this is the best fix.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[Bart Van Assche]

On Sat, Jun 25, 2011 at 10:58 AM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>
> On Sat, Jun 25, 2011 at 10:10:30AM +0200, Bart Van Assche wrote:
> > SCSI scanning of a channel:id:lun triplet in Linux works as follows (function
> > scsi_scan_target() in drivers/scsi/scsi_scan.c):
> > - If lun == SCAN_WILD_CARD, send a REPORT LUNS command to the target and
> >   process the result.
> > - If lun != SCAN_WILD_CARD, send an INQUIRY command to the LUN corresponding
> >   to the specified channel:id:lun triplet to verify whether the LUN exists.
> > So a SCSI driver must either take the channel and target id values in account
> > in its quecommand() function or it should declare that it only supports one
> > channel and one target id. Currently the ib_srp driver does neither. As a
> > result scanning the SCSI bus via e.g. rescan-scsi-bus.sh causes many
> > duplicate SCSI devices to be created. For each 0:0:L device, several
> > duplicates are created with the same LUN number and with (C:I) != (0:0). Fix
> > this by declaring that the ib_srp driver only supports one channel and one
> > target id.
>
> Looks good.  We should probably consider chosing sane defaults at least
> for max_channel, but for now this is the best fix.

The default for max_channel is already zero, so strictly speaking it
is not necessary to set max_channel in srp_target_create(). I choose
to set it anyway to make the intent clear that currently ib_srp only
supports channel == 0 and id == 0.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[Christoph Hellwig]

On Sat, Jun 25, 2011 at 11:42:31AM +0200, Bart Van Assche wrote:
> The default for max_channel is already zero, so strictly speaking it
> is not necessary to set max_channel in srp_target_create(). I choose
> to set it anyway to make the intent clear that currently ib_srp only
> supports channel == 0 and id == 0.

You're right.  I'm not sure changing max_id is as easy - most SCSI
drivers are still for old SPI HBAs, so we'd need to update them all.
Eventually we should do it, though as most modern topologies don't
need the traditional sequential target scanning at all.  In fact
we shouldn't be using scsi_scan_host for them to start with.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[David Dillow]

On Sat, 2011-06-25 at 10:10 +0200, Bart Van Assche wrote:
> SCSI scanning of a channel:id:lun triplet in Linux works as follows (function
> scsi_scan_target() in drivers/scsi/scsi_scan.c):
> - If lun == SCAN_WILD_CARD, send a REPORT LUNS command to the target and
>   process the result.
> - If lun != SCAN_WILD_CARD, send an INQUIRY command to the LUN corresponding
>   to the specified channel:id:lun triplet to verify whether the LUN exists.
> So a SCSI driver must either take the channel and target id values in account
> in its quecommand() function or it should declare that it only supports one
> channel and one target id. Currently the ib_srp driver does neither. As a
> result scanning the SCSI bus via e.g. rescan-scsi-bus.sh causes many
> duplicate SCSI devices to be created. For each 0:0:L device, several
> duplicates are created with the same LUN number and with (C:I) != (0:0). Fix
> this by declaring that the ib_srp driver only supports one channel and one
> target id.
> 
> Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
> Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>

Thanks for the additional detail in the commit message.

Acked-by: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[Bart Van Assche]

On Sat, Jun 25, 2011 at 6:14 PM, David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org> wrote:
> On Sat, 2011-06-25 at 10:10 +0200, Bart Van Assche wrote:
> > SCSI scanning of a channel:id:lun triplet in Linux works as follows (function
> > scsi_scan_target() in drivers/scsi/scsi_scan.c):
> > - If lun == SCAN_WILD_CARD, send a REPORT LUNS command to the target and
> >   process the result.
> > - If lun != SCAN_WILD_CARD, send an INQUIRY command to the LUN corresponding
> >   to the specified channel:id:lun triplet to verify whether the LUN exists.
> > So a SCSI driver must either take the channel and target id values in account
> > in its quecommand() function or it should declare that it only supports one
> > channel and one target id. Currently the ib_srp driver does neither. As a
> > result scanning the SCSI bus via e.g. rescan-scsi-bus.sh causes many
> > duplicate SCSI devices to be created. For each 0:0:L device, several
> > duplicates are created with the same LUN number and with (C:I) != (0:0). Fix
> > this by declaring that the ib_srp driver only supports one channel and one
> > target id.
> >
> > Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
> > Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
>
> Thanks for the additional detail in the commit message.
>
> Acked-by: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>

Hi Dave,

Thanks for acking. Through which tree should this patch go in ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[David Dillow]

On Tue, 2011-07-19 at 07:24 -0400, Bart Van Assche wrote:
> On Sat, Jun 25, 2011 at 6:14 PM, David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org> wrote:
> > On Sat, 2011-06-25 at 10:10 +0200, Bart Van Assche wrote:

> > > So a SCSI driver must either take the channel and target id values in account
> > > in its quecommand() function or it should declare that it only supports one
> > > channel and one target id. Currently the ib_srp driver does neither. As a
> > > result scanning the SCSI bus via e.g. rescan-scsi-bus.sh causes many
> > > duplicate SCSI devices to be created. For each 0:0:L device, several
> > > duplicates are created with the same LUN number and with (C:I) != (0:0). Fix
> > > this by declaring that the ib_srp driver only supports one channel and one
> > > target id.
> > >
> > > Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
> > > Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
> >
> > Thanks for the additional detail in the commit message.
> >
> > Acked-by: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
> 
> Hi Dave,
> 
> Thanks for acking. Through which tree should this patch go in ?

I Ack'd thinking Roland would pick it up directly since has done that in
the past for singe patches, but I'm fine with pulling it into my tree
and pushing that way.

Roland?

-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srp: Avoid that LUN scanning creates duplicate devices

[Roland Dreier]

On Tue, Jul 19, 2011 at 9:03 AM, David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org> wrote:
> I Ack'd thinking Roland would pick it up directly since has done that in
> the past for singe patches, but I'm fine with pulling it into my tree
> and pushing that way.
>
> Roland?

I've had it in my for-next branch for a while now... since it wasn't a
regression (AFAICT, we've been that way approximately forever), I
was waiting for 3.1 to ask Linus to take it.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html