| Message ID | 562FF4D9.2060809@sandisk.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Can you spare a few words on this change in the change log? > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> > Cc: Sagi Grimberg <sagig@mellanox.com> > Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> > --- > drivers/infiniband/ulp/srp/ib_srp.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > index 47c3a72..59d3ff9 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.c > +++ b/drivers/infiniband/ulp/srp/ib_srp.c > @@ -1666,6 +1666,8 @@ map_complete: > > unmap: > srp_unmap_data(scmnd, ch, req, true); > + if (ret == -ENOMEM && req->nmdesc >= target->mr_pool_size) > + ret = -E2BIG; > return ret; > } Why return E2BIG for ENOMEM as well? -- 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 11/03/2015 09:44 AM, Sagi Grimberg wrote: > Can you spare a few words on this change in the change log? > > >> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> >> Cc: Sagi Grimberg <sagig@mellanox.com> >> Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> >> --- >> drivers/infiniband/ulp/srp/ib_srp.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c >> index 47c3a72..59d3ff9 100644 >> --- a/drivers/infiniband/ulp/srp/ib_srp.c >> +++ b/drivers/infiniband/ulp/srp/ib_srp.c >> @@ -1666,6 +1666,8 @@ map_complete: >> >> unmap: >> srp_unmap_data(scmnd, ch, req, true); >> + if (ret == -ENOMEM && req->nmdesc >= target->mr_pool_size) >> + ret = -E2BIG; >> return ret; >> } > > Why return E2BIG for ENOMEM as well? Hello Sagi, The srp_queuecommand() function translates ENOMEM into QUEUE_FULL which causes the SCSI mid-layer to retry the command. All other error codes are translated into DID_ERROR which causes the SCSI command to fail. This is why this patch fixes an infinite loop. I will add this explanation to the patch description when I repost this patch. 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
On 03/11/2015 20:56, Bart Van Assche wrote: > On 11/03/2015 09:44 AM, Sagi Grimberg wrote: >> Can you spare a few words on this change in the change log? >> >> >>> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> >>> Cc: Sagi Grimberg <sagig@mellanox.com> >>> Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> >>> --- >>> drivers/infiniband/ulp/srp/ib_srp.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c >>> b/drivers/infiniband/ulp/srp/ib_srp.c >>> index 47c3a72..59d3ff9 100644 >>> --- a/drivers/infiniband/ulp/srp/ib_srp.c >>> +++ b/drivers/infiniband/ulp/srp/ib_srp.c >>> @@ -1666,6 +1666,8 @@ map_complete: >>> >>> unmap: >>> srp_unmap_data(scmnd, ch, req, true); >>> + if (ret == -ENOMEM && req->nmdesc >= target->mr_pool_size) >>> + ret = -E2BIG; >>> return ret; >>> } >> >> Why return E2BIG for ENOMEM as well? > > Hello Sagi, > > The srp_queuecommand() function translates ENOMEM into QUEUE_FULL which > causes the SCSI mid-layer to retry the command. All other error codes > are translated into DID_ERROR which causes the SCSI command to fail. That's what I meant, ENOMEM is transient by nature. Why would you not want the scsi layer to requeue? I do understand this for the nmdesc condition but for the ENOMEM? -- 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 11/03/2015 10:59 AM, Sagi Grimberg wrote: > On 03/11/2015 20:56, Bart Van Assche wrote: >> On 11/03/2015 09:44 AM, Sagi Grimberg wrote: >>> Can you spare a few words on this change in the change log? >>> >>> >>>> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> >>>> Cc: Sagi Grimberg <sagig@mellanox.com> >>>> Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> >>>> --- >>>> drivers/infiniband/ulp/srp/ib_srp.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c >>>> b/drivers/infiniband/ulp/srp/ib_srp.c >>>> index 47c3a72..59d3ff9 100644 >>>> --- a/drivers/infiniband/ulp/srp/ib_srp.c >>>> +++ b/drivers/infiniband/ulp/srp/ib_srp.c >>>> @@ -1666,6 +1666,8 @@ map_complete: >>>> >>>> unmap: >>>> srp_unmap_data(scmnd, ch, req, true); >>>> + if (ret == -ENOMEM && req->nmdesc >= target->mr_pool_size) >>>> + ret = -E2BIG; >>>> return ret; >>>> } >>> >>> Why return E2BIG for ENOMEM as well? >> >> Hello Sagi, >> >> The srp_queuecommand() function translates ENOMEM into QUEUE_FULL which >> causes the SCSI mid-layer to retry the command. All other error codes >> are translated into DID_ERROR which causes the SCSI command to fail. > > That's what I meant, ENOMEM is transient by nature. Why would you > not want the scsi layer to requeue? I do understand this for the nmdesc > condition but for the ENOMEM? Hello Sagi, Since the ret == -ENOMEM test is redundant in the above code that test can be left out. Would that make this patch more clear to you ? 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
> Hello Sagi, > > Since the ret == -ENOMEM test is redundant in the above code that test > can be left out. Would that make this patch more clear to you ? It will. Thanks, Sagi. -- 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 47c3a72..59d3ff9 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1666,6 +1666,8 @@ map_complete: unmap: srp_unmap_data(scmnd, ch, req, true); + if (ret == -ENOMEM && req->nmdesc >= target->mr_pool_size) + ret = -E2BIG; return ret; }
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> Cc: Sagi Grimberg <sagig@mellanox.com> Cc: Sebastian Parschauer <sebastian.riemer@profitbricks.com> --- drivers/infiniband/ulp/srp/ib_srp.c | 2 ++ 1 file changed, 2 insertions(+)