* Re: [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte
2017-10-09 11:33 [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte Johannes Thumshirn
@ 2017-10-09 12:35 ` Steffen Maier
2017-10-09 12:46 ` Johannes Thumshirn
2017-10-09 14:33 ` Johannes Thumshirn
2017-10-09 21:26 ` Lee Duncan
` (2 subsequent siblings)
3 siblings, 2 replies; 7+ messages in thread
From: Steffen Maier @ 2017-10-09 12:35 UTC (permalink / raw)
To: Johannes Thumshirn, Martin K . Petersen
Cc: Linux Kernel Mailinglist, Linux SCSI Mailinglist, Lee Duncan,
Hannes Reinecke, Bart Van Assche, Chris Leech
Use wrapper functions to advertize their use in an attempt to avoid
wrong shifting in the future?
On 10/09/2017 01:33 PM, Johannes Thumshirn wrote:
> The SCSI host byte should be shifted left by 16 in order to have
> scsi_decide_disposition() do the right thing (.i.e. requeue the command).
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Fixes: 661134ad3765 ("[SCSI] libiscsi, bnx2i: make bound ep check common")
> Cc: Lee Duncan <lduncan@suse.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Bart Van Assche <Bart.VanAssche@sandisk.com>
> Cc: Chris Leech <cleech@redhat.com>
> ---
> drivers/scsi/libiscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index bd4605a34f54..9cba4913b43c 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1728,7 +1728,7 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc)
>
> if (test_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx)) {
> reason = FAILURE_SESSION_IN_RECOVERY;
> - sc->result = DID_REQUEUE;
> + sc->result = DID_REQUEUE << 16;
not sure if this really wants to reset the other parts of result, but if
so (and they are not 0 already anyway), preceed the set_host_byte() by:
sc->result = 0;
set_host_byte(sc, DID_REQUEUE);
> goto fault;
> }
>
--
Mit freundlichen Grüßen / Kind regards
Steffen Maier
Linux on z Systems Development
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte
2017-10-09 12:35 ` Steffen Maier
@ 2017-10-09 12:46 ` Johannes Thumshirn
2017-10-09 14:33 ` Johannes Thumshirn
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Thumshirn @ 2017-10-09 12:46 UTC (permalink / raw)
To: Steffen Maier
Cc: Martin K . Petersen, Linux Kernel Mailinglist,
Linux SCSI Mailinglist, Lee Duncan, Hannes Reinecke,
Bart Van Assche, Chris Leech
On Mon, Oct 09, 2017 at 02:35:06PM +0200, Steffen Maier wrote:
> Use wrapper functions to advertize their use in an attempt to avoid wrong
> shifting in the future?
Not sure, converting all the users would be a lot of churn for relatively low
benefit:
linux (master)$ git grep "result = DID_" drivers/scsi/ | wc -l
419
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte
2017-10-09 12:35 ` Steffen Maier
2017-10-09 12:46 ` Johannes Thumshirn
@ 2017-10-09 14:33 ` Johannes Thumshirn
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Thumshirn @ 2017-10-09 14:33 UTC (permalink / raw)
To: Steffen Maier
Cc: Martin K . Petersen, Linux Kernel Mailinglist,
Linux SCSI Mailinglist, Lee Duncan, Hannes Reinecke,
Bart Van Assche, Chris Leech
On Mon, Oct 09, 2017 at 02:35:06PM +0200, Steffen Maier wrote:
> Use wrapper functions to advertize their use in an attempt to avoid wrong
> shifting in the future?
After a second thought and a bit of coccinelle magic I converted all drivers
under drivers/scsi to use set_host_byte(), msg and driver byte will follow as
well.
But all this is on top of this patch, which IMHO is stable material (we had an
actual customer bug with this).
Byte,
Johannes
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte
2017-10-09 11:33 [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte Johannes Thumshirn
2017-10-09 12:35 ` Steffen Maier
@ 2017-10-09 21:26 ` Lee Duncan
2017-10-10 15:20 ` Hannes Reinecke
2017-10-11 17:34 ` Martin K. Petersen
3 siblings, 0 replies; 7+ messages in thread
From: Lee Duncan @ 2017-10-09 21:26 UTC (permalink / raw)
To: Johannes Thumshirn, Martin K . Petersen
Cc: Linux Kernel Mailinglist, Linux SCSI Mailinglist,
Hannes Reinecke, Bart Van Assche, Chris Leech
On 10/09/2017 04:33 AM, Johannes Thumshirn wrote:
> The SCSI host byte should be shifted left by 16 in order to have
> scsi_decide_disposition() do the right thing (.i.e. requeue the command).
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Fixes: 661134ad3765 ("[SCSI] libiscsi, bnx2i: make bound ep check common")
> Cc: Lee Duncan <lduncan@suse.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Bart Van Assche <Bart.VanAssche@sandisk.com>
> Cc: Chris Leech <cleech@redhat.com>
> ---
> drivers/scsi/libiscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index bd4605a34f54..9cba4913b43c 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1728,7 +1728,7 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc)
>
> if (test_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx)) {
> reason = FAILURE_SESSION_IN_RECOVERY;
> - sc->result = DID_REQUEUE;
> + sc->result = DID_REQUEUE << 16;
> goto fault;
> }
>
>
Good catch.
I know you're working on fixing all drivers to use the correct macros
rather than rolling there own.
Acked-by: Lee Duncan <lduncan@suse.com>
--
Lee Duncan
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte
2017-10-09 11:33 [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte Johannes Thumshirn
2017-10-09 12:35 ` Steffen Maier
2017-10-09 21:26 ` Lee Duncan
@ 2017-10-10 15:20 ` Hannes Reinecke
2017-10-11 17:34 ` Martin K. Petersen
3 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2017-10-10 15:20 UTC (permalink / raw)
To: Johannes Thumshirn, Martin K . Petersen
Cc: Linux Kernel Mailinglist, Linux SCSI Mailinglist, Lee Duncan,
Bart Van Assche, Chris Leech
On 10/09/2017 01:33 PM, Johannes Thumshirn wrote:
> The SCSI host byte should be shifted left by 16 in order to have
> scsi_decide_disposition() do the right thing (.i.e. requeue the command).
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Fixes: 661134ad3765 ("[SCSI] libiscsi, bnx2i: make bound ep check common")
> Cc: Lee Duncan <lduncan@suse.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Bart Van Assche <Bart.VanAssche@sandisk.com>
> Cc: Chris Leech <cleech@redhat.com>
> ---
> drivers/scsi/libiscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index bd4605a34f54..9cba4913b43c 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1728,7 +1728,7 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc)
>
> if (test_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx)) {
> reason = FAILURE_SESSION_IN_RECOVERY;
> - sc->result = DID_REQUEUE;
> + sc->result = DID_REQUEUE << 16;
> goto fault;
> }
>
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte
2017-10-09 11:33 [PATCH] scsi: libiscsi: fix shifting of DID_REQUEUE host byte Johannes Thumshirn
` (2 preceding siblings ...)
2017-10-10 15:20 ` Hannes Reinecke
@ 2017-10-11 17:34 ` Martin K. Petersen
3 siblings, 0 replies; 7+ messages in thread
From: Martin K. Petersen @ 2017-10-11 17:34 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Martin K . Petersen, Linux Kernel Mailinglist,
Linux SCSI Mailinglist, Lee Duncan, Hannes Reinecke,
Bart Van Assche, Chris Leech
Johannes,
> The SCSI host byte should be shifted left by 16 in order to have
> scsi_decide_disposition() do the right thing (.i.e. requeue the
> command).
Applied to 4.14/scsi-fixes. Thank you!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 7+ messages in thread