Re: [PATCH] ib_srp: Remove WARN_ON in srp_terminate_io()

2018-10-17 Thread Christoph Hellwig 
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index e52b9d3c0bd6..c777b36ba62a 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -483,6 +483,8 @@ nvme_fc_signal_discovery_scan(struct nvme_fc_lport *lport,
>   char hostaddr[FCNVME_TRADDR_LENGTH];/* NVMEFC_HOST_TRADDR=...*/
>   char tgtaddr[FCNVME_TRADDR_LENGTH]; /* NVMEFC_TRADDR=...*/
>   char *envp[4] = { "FC_EVENT=nvmediscovery", hostaddr, tgtaddr, NULL };
> + char *aen_envp[5] = { "NVME_EVENT=discovery", "NVME_TRTYPE=fc",
> +   hostaddr, tgtaddr, NULL };

I don't think this belongs into the patch..

[PATCH] ib_srp: Remove WARN_ON in srp_terminate_io()

2018-10-17 Thread Hannes Reinecke 
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 ---
 drivers/nvme/host/fc.c  | 10 ++
 2 files changed, 10 insertions(+), 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];
 
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index e52b9d3c0bd6..c777b36ba62a 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -483,6 +483,8 @@ nvme_fc_signal_discovery_scan(struct nvme_fc_lport *lport,
char hostaddr[FCNVME_TRADDR_LENGTH];/* NVMEFC_HOST_TRADDR=...*/
char tgtaddr[FCNVME_TRADDR_LENGTH]; /* NVMEFC_TRADDR=...*/
char *envp[4] = { "FC_EVENT=nvmediscovery", hostaddr, tgtaddr, NULL };
+   char *aen_envp[5] = { "NVME_EVENT=discovery", "NVME_TRTYPE=fc",
+ hostaddr, tgtaddr, NULL };
 
if (!(rport->remoteport.port_role & FC_PORT_ROLE_NVME_DISCOVERY))
return;
@@ -494,6 +496,14 @@ nvme_fc_signal_discovery_scan(struct nvme_fc_lport *lport,
"NVMEFC_TRADDR=nn-0x%016llx:pn-0x%016llx",
rport->remoteport.node_name, rport->remoteport.port_name);
kobject_uevent_env(_udev_device->kobj, KOBJ_CHANGE, envp);
+   /* Simulate Discovery AENs */
+   snprintf(hostaddr, sizeof(hostaddr),
+"NVME_TRADDR=nn-0x%016llx:pn-0x%016llx",
+   rport->remoteport.node_name, rport->remoteport.port_name);
+   snprintf(tgtaddr, sizeof(tgtaddr),
+"NVME_HOST_TRADDR=nn-0x%016llx:pn-0x%016llx",
+   lport->localport.node_name, lport->localport.port_name);
+   kobject_uevent_env(_udev_device->kobj, KOBJ_CHANGE, envp);
 }
 
 static void
-- 
2.16.4

Re: [PATCH] ib_srp: Remove WARN_ON in srp_terminate_io()

2018-09-21 Thread Bart Van Assche 

On 9/21/18 5:15 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_queuecomment()

Signed-off-by: Hannes Reinecke 
---
  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 444d16520506..4f6e88136da3 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];


Since this function is not in the hot path and since this function is 
called indirectly, I'd like to keep the above code as documentation of 
the requirements for callers of this function.


Please also Cc the linux-rdma mailing list for SRP initiator changes.

Thanks,

Bart.

Re: [PATCH] ib_srp: Remove WARN_ON in srp_terminate_io()

2018-09-21 Thread Johannes Thumshirn 
On Fri, Sep 21, 2018 at 02:15:05PM +0200, 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_queuecomment()


s/srp_queuecomment/srp_queuecommand/

-- 
Johannes Thumshirn  Storage
jthumsh...@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

[PATCH] ib_srp: Remove WARN_ON in srp_terminate_io()

2018-09-21 Thread Hannes Reinecke 
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_queuecomment()

Signed-off-by: Hannes Reinecke 
---
 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 444d16520506..4f6e88136da3 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];
 
-- 
2.16.4