Re: [PATCHv2] ib_srp: Remove WARN_ON in srp_terminate_io()
On Wed, 2018-10-17 at 11:30 -0400, Doug Ledford wrote:
> On Wed, 2018-10-17 at 07:43 -0700, Bart Van Assche wrote:
> > On 10/17/18 12:20 AM, Hannes Reinecke wrote:
> > > The WARN_ON() is pointless as the rport is placed in
> > > SDEV_TRANSPORT_OFFLINE
> > > at that time, so no new commands can be submitted via srp_queuecommand()
> > >
> > > Signed-off-by: Hannes Reinecke
> > > Reviewed-by: Jens Axboe
> > > Reviewed-by: Johannes Thumshirn
> > > ---
> > > drivers/infiniband/ulp/srp/ib_srp.c | 7 ---
> > > 1 file changed, 7 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
> > > b/drivers/infiniband/ulp/srp/ib_srp.c
> > > index 0b34e909505f..5a79444c2f3c 100644
> > > --- a/drivers/infiniband/ulp/srp/ib_srp.c
> > > +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> > > @@ -1334,13 +1334,6 @@ static void srp_terminate_io(struct srp_rport
> > > *rport)
> > > struct scsi_device *sdev;
> > > int i, j;
> > >
> > > - /*
> > > - * Invoking srp_terminate_io() while srp_queuecommand() is running
> > > - * is not safe. Hence the warning statement below.
> > > - */
> > > - shost_for_each_device(sdev, shost)
> > > - WARN_ON_ONCE(sdev->request_queue->request_fn_active);
> > > -
> > > for (i = 0; i < target->ch_count; i++) {
> > > ch = >ch[i];
> >
> > Although I had explained before why I think that warning is not
> > pointless, I agree with this change because the legacy block layer is
> > going away. Anyway:
> >
> > Acked-by: Bart Van Assche
>
> Thanks, applied to for-next.
>
FWIW, this introduced a build warning (shost and sdev are now unused
variables). I edited the patch to remove the newly unneeded variables.
--
Doug Ledford
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
signature.asc
Description: This is a digitally signed message part
Re: [PATCHv2] ib_srp: Remove WARN_ON in srp_terminate_io()
On Wed, 2018-10-17 at 07:43 -0700, Bart Van Assche wrote:
> On 10/17/18 12:20 AM, Hannes Reinecke wrote:
> > The WARN_ON() is pointless as the rport is placed in SDEV_TRANSPORT_OFFLINE
> > at that time, so no new commands can be submitted via srp_queuecommand()
> >
> > Signed-off-by: Hannes Reinecke
> > Reviewed-by: Jens Axboe
> > Reviewed-by: Johannes Thumshirn
> > ---
> > drivers/infiniband/ulp/srp/ib_srp.c | 7 ---
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
> > b/drivers/infiniband/ulp/srp/ib_srp.c
> > index 0b34e909505f..5a79444c2f3c 100644
> > --- a/drivers/infiniband/ulp/srp/ib_srp.c
> > +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> > @@ -1334,13 +1334,6 @@ static void srp_terminate_io(struct srp_rport *rport)
> > struct scsi_device *sdev;
> > int i, j;
> >
> > - /*
> > -* Invoking srp_terminate_io() while srp_queuecommand() is running
> > -* is not safe. Hence the warning statement below.
> > -*/
> > - shost_for_each_device(sdev, shost)
> > - WARN_ON_ONCE(sdev->request_queue->request_fn_active);
> > -
> > for (i = 0; i < target->ch_count; i++) {
> > ch = >ch[i];
>
> Although I had explained before why I think that warning is not
> pointless, I agree with this change because the legacy block layer is
> going away. Anyway:
>
> Acked-by: Bart Van Assche
Thanks, applied to for-next.
--
Doug Ledford
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
signature.asc
Description: This is a digitally signed message part
Re: [PATCHv2] ib_srp: Remove WARN_ON in srp_terminate_io()
On 10/17/18 12:20 AM, Hannes Reinecke wrote:
The WARN_ON() is pointless as the rport is placed in SDEV_TRANSPORT_OFFLINE
at that time, so no new commands can be submitted via srp_queuecommand()
Signed-off-by: Hannes Reinecke
Reviewed-by: Jens Axboe
Reviewed-by: Johannes Thumshirn
---
drivers/infiniband/ulp/srp/ib_srp.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
b/drivers/infiniband/ulp/srp/ib_srp.c
index 0b34e909505f..5a79444c2f3c 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1334,13 +1334,6 @@ static void srp_terminate_io(struct srp_rport *rport)
struct scsi_device *sdev;
int i, j;
- /*
-* Invoking srp_terminate_io() while srp_queuecommand() is running
-* is not safe. Hence the warning statement below.
-*/
- shost_for_each_device(sdev, shost)
- WARN_ON_ONCE(sdev->request_queue->request_fn_active);
-
for (i = 0; i < target->ch_count; i++) {
ch = >ch[i];
Although I had explained before why I think that warning is not
pointless, I agree with this change because the legacy block layer is
going away. Anyway:
Acked-by: Bart Van Assche
