| Message ID | 20171009113319.9505-1-jthumshirn@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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; > } >
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
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
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>
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
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!
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; }
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(-)