public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: alua: Fix alua_rtpg_queue()
@ 2022-11-15 22:49 Bart Van Assche
  2022-11-16 10:32 ` Martin Wilck
  2022-11-17 17:57 ` Benjamin Block
  0 siblings, 2 replies; 6+ messages in thread
From: Bart Van Assche @ 2022-11-15 22:49 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, Bart Van Assche, Sachin Sant, Hannes Reinecke,
	Martin Wilck

Modify alua_rtpg_queue() such that it only requests the caller to drop
the sdev reference if necessary. This patch fixes a recently introduced
regression.

Cc: Sachin Sant <sachinp@linux.ibm.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Martin Wilck <mwilck@suse.com>
Reported-by: Sachin Sant <sachinp@linux.ibm.com>
Fixes: 0b25e17e9018 ("scsi: alua: Move a scsi_device_put() call out of alua_check_vpd()")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 31 ++++++++++++++--------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 693cd827e138..c1bf75673e89 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -102,7 +102,7 @@ struct alua_queue_data {
 
 static void alua_rtpg_work(struct work_struct *work);
 static bool alua_rtpg_queue(struct alua_port_group *pg,
-			    struct scsi_device *sdev,
+			    struct scsi_device *sdev, bool *put_sdev,
 			    struct alua_queue_data *qdata, bool force);
 static void alua_check(struct scsi_device *sdev, bool force);
 
@@ -374,9 +374,9 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
 		list_add_rcu(&h->node, &pg->dh_list);
 	spin_unlock_irqrestore(&pg->lock, flags);
 
-	put_sdev = alua_rtpg_queue(rcu_dereference_protected(h->pg,
+	alua_rtpg_queue(rcu_dereference_protected(h->pg,
 						  lockdep_is_held(&h->pg_lock)),
-			sdev, NULL, true);
+			sdev, &put_sdev, NULL, true);
 	spin_unlock(&h->pg_lock);
 
 	if (put_sdev)
@@ -983,17 +983,23 @@ static void alua_rtpg_work(struct work_struct *work)
  *
  * Returns true if and only if alua_rtpg_work() will be called asynchronously.
  * That function is responsible for calling @qdata->fn(). If this function
- * returns true, the caller is responsible for invoking scsi_device_put(@sdev).
+ * sets *put_sdev to true, the caller is responsible for calling
+ * scsi_device_put(@sdev).
  */
-static bool __must_check alua_rtpg_queue(struct alua_port_group *pg,
-			    struct scsi_device *sdev,
+static bool alua_rtpg_queue(struct alua_port_group *pg,
+			    struct scsi_device *sdev, bool *put_sdev,
 			    struct alua_queue_data *qdata, bool force)
 {
 	int start_queue = 0;
 	unsigned long flags;
+
+	*put_sdev = false;
+
 	if (WARN_ON_ONCE(!pg) || scsi_device_get(sdev))
 		return false;
 
+	*put_sdev = true;
+
 	spin_lock_irqsave(&pg->lock, flags);
 	if (qdata) {
 		list_add_tail(&qdata->entry, &pg->rtpg_list);
@@ -1020,7 +1026,7 @@ static bool __must_check alua_rtpg_queue(struct alua_port_group *pg,
 	if (start_queue) {
 		if (queue_delayed_work(kaluad_wq, &pg->rtpg_work,
 				msecs_to_jiffies(ALUA_RTPG_DELAY_MSECS)))
-			sdev = NULL;
+			*put_sdev = false;
 		else
 			kref_put(&pg->kref, release_port_group);
 	}
@@ -1108,6 +1114,7 @@ static int alua_activate(struct scsi_device *sdev,
 	int err = SCSI_DH_OK;
 	struct alua_queue_data *qdata;
 	struct alua_port_group *pg;
+	bool put_sdev;
 
 	qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
 	if (!qdata) {
@@ -1130,8 +1137,9 @@ static int alua_activate(struct scsi_device *sdev,
 	rcu_read_unlock();
 	mutex_unlock(&h->init_mutex);
 
-	if (alua_rtpg_queue(pg, sdev, qdata, true)) {
-		scsi_device_put(sdev);
+	if (alua_rtpg_queue(pg, sdev, &put_sdev, qdata, true)) {
+		if (put_sdev)
+			scsi_device_put(sdev);
 		fn = NULL;
 	} else {
 		err = SCSI_DH_DEV_OFFLINED;
@@ -1153,6 +1161,7 @@ static void alua_check(struct scsi_device *sdev, bool force)
 {
 	struct alua_dh_data *h = sdev->handler_data;
 	struct alua_port_group *pg;
+	bool put_sdev;
 
 	rcu_read_lock();
 	pg = rcu_dereference(h->pg);
@@ -1161,8 +1170,8 @@ static void alua_check(struct scsi_device *sdev, bool force)
 		return;
 	}
 	rcu_read_unlock();
-
-	if (alua_rtpg_queue(pg, sdev, NULL, force))
+	alua_rtpg_queue(pg, sdev, &put_sdev, NULL, force);
+	if (put_sdev)
 		scsi_device_put(sdev);
 	kref_put(&pg->kref, release_port_group);
 }

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: alua: Fix alua_rtpg_queue()
  2022-11-15 22:49 [PATCH] scsi: alua: Fix alua_rtpg_queue() Bart Van Assche
@ 2022-11-16 10:32 ` Martin Wilck
  2022-11-16 21:22   ` Bart Van Assche
  2022-11-17 17:57 ` Benjamin Block
  1 sibling, 1 reply; 6+ messages in thread
From: Martin Wilck @ 2022-11-16 10:32 UTC (permalink / raw)
  To: Bart Van Assche, Martin K . Petersen
  Cc: linux-scsi, Sachin Sant, Hannes Reinecke

Hello Bart,

On Tue, 2022-11-15 at 14:49 -0800, Bart Van Assche wrote:
> Modify alua_rtpg_queue() such that it only requests the caller to
> drop
> the sdev reference if necessary. This patch fixes a recently
> introduced
> regression.
> 
> Cc: Sachin Sant <sachinp@linux.ibm.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Martin Wilck <mwilck@suse.com>
> Reported-by: Sachin Sant <sachinp@linux.ibm.com>
> Fixes: 0b25e17e9018 ("scsi: alua: Move a scsi_device_put() call out
> of alua_check_vpd()")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Is this complexity really necessary, just for calling
scsi_device_put()? 0b25e17e9018 is supposed to avoid sleeping under 
&h->pg_lock. Have you considered simply not calling alua_rtpg_queue()
with this lock held? alua_rtpg_queue() accesses no data that is
protected by &h->pg_lock (which is just h->pg, AFAIU).

Would it perhaps be sufficient to just make sure we keep a kref for the
pg struct in alua_check_vpd(), like the patch below (on mkp/queue,
meant as an alternative to 0b25e17e9018)?

Regards
Martin

From 04df5933239921e7e7ac00e9ec0558124b050a51 Mon Sep 17 00:00:00 2001
From: Martin Wilck <mwilck@suse.com>
Date: Wed, 16 Nov 2022 11:24:58 +0100
Subject: [PATCH] scsi: alua: alua_check_vpd: drop pg_lock before calling
 alua_rtpg_queue

Since commit f93ed747e2c7 ("scsi: core: Release SCSI devices synchronously"),
scsi_device_put() might sleep. Avoid calling it from alua_rtpg_queue()
with the pg_lock held. The lock only pretects h->pg, anyway. To avoid
the pg being freed under us, because of a race with another thread,
take a temporary reference. In alua_rtpg_queue(), verify that the pg
still belongs to the sdev being passed before actually queueing the RTPG.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 30 +++++++++++++++-------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 610a51538f03..905b49493e01 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -372,12 +372,13 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
 	if (pg_updated)
 		list_add_rcu(&h->node, &pg->dh_list);
 	spin_unlock_irqrestore(&pg->lock, flags);
-
-	alua_rtpg_queue(rcu_dereference_protected(h->pg,
-						  lockdep_is_held(&h->pg_lock)),
-			sdev, NULL, true);
 	spin_unlock(&h->pg_lock);
 
+	if (kref_get_unless_zero(&pg->kref)) {
+		alua_rtpg_queue(pg, sdev, NULL, true);
+		kref_put(&pg->kref, release_port_group);
+	}
+
 	if (old_pg)
 		kref_put(&old_pg->kref, release_port_group);
 
@@ -986,11 +987,22 @@ static bool alua_rtpg_queue(struct alua_port_group *pg,
 		force = true;
 	}
 	if (pg->rtpg_sdev == NULL) {
-		pg->interval = 0;
-		pg->flags |= ALUA_PG_RUN_RTPG;
-		kref_get(&pg->kref);
-		pg->rtpg_sdev = sdev;
-		start_queue = 1;
+		struct alua_dh_data *h = sdev->handler_data;
+		struct alua_port_group *sdev_pg = NULL;
+
+		if (h) {
+			rcu_read_lock();
+			sdev_pg = rcu_dereference(h->pg);
+			rcu_read_unlock();
+		}
+
+		if (pg == sdev_pg) {
+			pg->flags |= ALUA_PG_RUN_RTPG;
+			pg->interval = 0;
+			kref_get(&pg->kref);
+			pg->rtpg_sdev = sdev;
+			start_queue = 1;
+		}
 	} else if (!(pg->flags & ALUA_PG_RUN_RTPG) && force) {
 		pg->flags |= ALUA_PG_RUN_RTPG;
 		/* Do not queue if the worker is already running */
-- 
2.38.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: alua: Fix alua_rtpg_queue()
  2022-11-16 10:32 ` Martin Wilck
@ 2022-11-16 21:22   ` Bart Van Assche
  2022-11-17  7:03     ` Martin Wilck
  0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2022-11-16 21:22 UTC (permalink / raw)
  To: Martin Wilck, Martin K . Petersen
  Cc: linux-scsi, Sachin Sant, Hannes Reinecke

On 11/16/22 02:32, Martin Wilck wrote:
> On Tue, 2022-11-15 at 14:49 -0800, Bart Van Assche wrote:
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 610a51538f03..905b49493e01 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -372,12 +372,13 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
>   	if (pg_updated)
>   		list_add_rcu(&h->node, &pg->dh_list);
>   	spin_unlock_irqrestore(&pg->lock, flags);
> -
> -	alua_rtpg_queue(rcu_dereference_protected(h->pg,
> -						  lockdep_is_held(&h->pg_lock)),
> -			sdev, NULL, true);
>   	spin_unlock(&h->pg_lock);
>   
> +	if (kref_get_unless_zero(&pg->kref)) {
> +		alua_rtpg_queue(pg, sdev, NULL, true);
> +		kref_put(&pg->kref, release_port_group);
> +	}
> +
>   	if (old_pg)
>   		kref_put(&old_pg->kref, release_port_group);
>   
> @@ -986,11 +987,22 @@ static bool alua_rtpg_queue(struct alua_port_group *pg,
>   		force = true;
>   	}
>   	if (pg->rtpg_sdev == NULL) {
> -		pg->interval = 0;
> -		pg->flags |= ALUA_PG_RUN_RTPG;
> -		kref_get(&pg->kref);
> -		pg->rtpg_sdev = sdev;
> -		start_queue = 1;
> +		struct alua_dh_data *h = sdev->handler_data;
> +		struct alua_port_group *sdev_pg = NULL;
> +
> +		if (h) {
> +			rcu_read_lock();
> +			sdev_pg = rcu_dereference(h->pg);
> +			rcu_read_unlock();
> +		}
> +
> +		if (pg == sdev_pg) {
> +			pg->flags |= ALUA_PG_RUN_RTPG;
> +			pg->interval = 0;
> +			kref_get(&pg->kref);
> +			pg->rtpg_sdev = sdev;
> +			start_queue = 1;
> +		}
>   	} else if (!(pg->flags & ALUA_PG_RUN_RTPG) && force) {
>   		pg->flags |= ALUA_PG_RUN_RTPG;
>   		/* Do not queue if the worker is already running */

Although I like the approach of this patch, how about the implementation below?
I think the implementation below is a little cleaner but that might be subjective ...

Thanks,

Bart.


diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index bd4ee294f5c7..d5a7d6ed5c63 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -354,6 +354,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
  			    "%s: port group %x rel port %x\n",
  			    ALUA_DH_NAME, group_id, rel_port);

+	kref_get(&pg->kref);
+
  	/* Check for existing port group references */
  	spin_lock(&h->pg_lock);
  	old_pg = rcu_dereference_protected(h->pg, lockdep_is_held(&h->pg_lock));
@@ -373,11 +375,11 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
  		list_add_rcu(&h->node, &pg->dh_list);
  	spin_unlock_irqrestore(&pg->lock, flags);

-	alua_rtpg_queue(rcu_dereference_protected(h->pg,
-						  lockdep_is_held(&h->pg_lock)),
-			sdev, NULL, true);
  	spin_unlock(&h->pg_lock);

+	alua_rtpg_queue(pg, sdev, NULL, true);
+	kref_put(&pg->kref, release_port_group);
+
  	if (old_pg)
  		kref_put(&old_pg->kref, release_port_group);

@@ -986,6 +988,9 @@ static bool alua_rtpg_queue(struct alua_port_group *pg,
  {
  	int start_queue = 0;
  	unsigned long flags;
+
+	might_sleep();
+
  	if (WARN_ON_ONCE(!pg) || scsi_device_get(sdev))
  		return false;

@@ -996,11 +1001,17 @@ static bool alua_rtpg_queue(struct alua_port_group *pg,
  		force = true;
  	}
  	if (pg->rtpg_sdev == NULL) {
-		pg->interval = 0;
-		pg->flags |= ALUA_PG_RUN_RTPG;
-		kref_get(&pg->kref);
-		pg->rtpg_sdev = sdev;
-		start_queue = 1;
+		struct alua_dh_data *h = sdev->handler_data;
+
+		rcu_read_lock();
+		if (rcu_dereference(h->pg) == pg) {
+			pg->interval = 0;
+			pg->flags |= ALUA_PG_RUN_RTPG;
+			kref_get(&pg->kref);
+			pg->rtpg_sdev = sdev;
+			start_queue = 1;
+		}
+		rcu_read_unlock();
  	} else if (!(pg->flags & ALUA_PG_RUN_RTPG) && force) {
  		pg->flags |= ALUA_PG_RUN_RTPG;
  		/* Do not queue if the worker is already running */
@@ -1246,8 +1257,8 @@ static void alua_bus_detach(struct scsi_device *sdev)
  		spin_unlock_irq(&pg->lock);
  		kref_put(&pg->kref, release_port_group);
  	}
-	sdev->handler_data = NULL;
  	synchronize_rcu();
+	sdev->handler_data = NULL;
  	kfree(h);
  }


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: alua: Fix alua_rtpg_queue()
  2022-11-16 21:22   ` Bart Van Assche
@ 2022-11-17  7:03     ` Martin Wilck
  2022-11-17 17:57       ` Bart Van Assche
  0 siblings, 1 reply; 6+ messages in thread
From: Martin Wilck @ 2022-11-17  7:03 UTC (permalink / raw)
  To: Bart Van Assche, Martin K . Petersen
  Cc: linux-scsi, Sachin Sant, Hannes Reinecke

On Wed, 2022-11-16 at 13:22 -0800, Bart Van Assche wrote:
> On 11/16/22 02:32, Martin Wilck wrote:
> 
> 
> [...]
> 
> Although I like the approach of this patch, how about the
> implementation below?
> I think the implementation below is a little cleaner but that might
> be subjective ...
> 

Fine with me, absolutely, and indeed better than mine. I don't quite
understand the last hook - sdev->handler_data isn't protected by rcu in
any way, is it? But that doesn't matter much.

Regards,
Martin


> Thanks,
> 
> Bart.
> 
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index bd4ee294f5c7..d5a7d6ed5c63 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -354,6 +354,8 @@ static int alua_check_vpd(struct scsi_device
> *sdev, struct alua_dh_data *h,
>                             "%s: port group %x rel port %x\n",
>                             ALUA_DH_NAME, group_id, rel_port);
> 
> +       kref_get(&pg->kref);
> +
>         /* Check for existing port group references */
>         spin_lock(&h->pg_lock);
>         old_pg = rcu_dereference_protected(h->pg, lockdep_is_held(&h-
> >pg_lock));
> @@ -373,11 +375,11 @@ static int alua_check_vpd(struct scsi_device
> *sdev, struct alua_dh_data *h,
>                 list_add_rcu(&h->node, &pg->dh_list);
>         spin_unlock_irqrestore(&pg->lock, flags);
> 
> -       alua_rtpg_queue(rcu_dereference_protected(h->pg,
> -                                                 lockdep_is_held(&h-
> >pg_lock)),
> -                       sdev, NULL, true);
>         spin_unlock(&h->pg_lock);
> 
> +       alua_rtpg_queue(pg, sdev, NULL, true);
> +       kref_put(&pg->kref, release_port_group);
> +
>         if (old_pg)
>                 kref_put(&old_pg->kref, release_port_group);
> 
> @@ -986,6 +988,9 @@ static bool alua_rtpg_queue(struct
> alua_port_group *pg,
>   {
>         int start_queue = 0;
>         unsigned long flags;
> +
> +       might_sleep();
> +
>         if (WARN_ON_ONCE(!pg) || scsi_device_get(sdev))
>                 return false;
> 
> @@ -996,11 +1001,17 @@ static bool alua_rtpg_queue(struct
> alua_port_group *pg,
>                 force = true;
>         }
>         if (pg->rtpg_sdev == NULL) {
> -               pg->interval = 0;
> -               pg->flags |= ALUA_PG_RUN_RTPG;
> -               kref_get(&pg->kref);
> -               pg->rtpg_sdev = sdev;
> -               start_queue = 1;
> +               struct alua_dh_data *h = sdev->handler_data;
> +
> +               rcu_read_lock();
> +               if (rcu_dereference(h->pg) == pg) {
> +                       pg->interval = 0;
> +                       pg->flags |= ALUA_PG_RUN_RTPG;
> +                       kref_get(&pg->kref);
> +                       pg->rtpg_sdev = sdev;
> +                       start_queue = 1;
> +               }
> +               rcu_read_unlock();
>         } else if (!(pg->flags & ALUA_PG_RUN_RTPG) && force) {
>                 pg->flags |= ALUA_PG_RUN_RTPG;
>                 /* Do not queue if the worker is already running */
> @@ -1246,8 +1257,8 @@ static void alua_bus_detach(struct scsi_device
> *sdev)
>                 spin_unlock_irq(&pg->lock);
>                 kref_put(&pg->kref, release_port_group);
>         }
> -       sdev->handler_data = NULL;
>         synchronize_rcu();
> +       sdev->handler_data = NULL;
>         kfree(h);
>   }
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: alua: Fix alua_rtpg_queue()
  2022-11-17  7:03     ` Martin Wilck
@ 2022-11-17 17:57       ` Bart Van Assche
  0 siblings, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2022-11-17 17:57 UTC (permalink / raw)
  To: Martin Wilck, Martin K . Petersen
  Cc: linux-scsi, Sachin Sant, Hannes Reinecke

On 11/16/22 23:03, Martin Wilck wrote:
> On Wed, 2022-11-16 at 13:22 -0800, Bart Van Assche wrote:
>> On 11/16/22 02:32, Martin Wilck wrote:
>>
>>
>> [...]
>>
>> Although I like the approach of this patch, how about the
>> implementation below?
>> I think the implementation below is a little cleaner but that might
>> be subjective ...
>>
> 
> Fine with me, absolutely, and indeed better than mine. I don't quite
> understand the last hook - sdev->handler_data isn't protected by rcu in
> any way, is it? But that doesn't matter much.

Hi Martin,

I plan to leave out the sdev->handler_data assignment change and to follow
the approach from your patch for the sdev->handler_data pointer (check
whether or not it is NULL) since I'm no longer sure that the change I
proposed is safe.

Thanks,

Bart.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: alua: Fix alua_rtpg_queue()
  2022-11-15 22:49 [PATCH] scsi: alua: Fix alua_rtpg_queue() Bart Van Assche
  2022-11-16 10:32 ` Martin Wilck
@ 2022-11-17 17:57 ` Benjamin Block
  1 sibling, 0 replies; 6+ messages in thread
From: Benjamin Block @ 2022-11-17 17:57 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, linux-scsi, Sachin Sant, Hannes Reinecke,
	Martin Wilck

On Tue, Nov 15, 2022 at 02:49:03PM -0800, Bart Van Assche wrote:
> Modify alua_rtpg_queue() such that it only requests the caller to drop
> the sdev reference if necessary. This patch fixes a recently introduced
> regression.
> 
> Cc: Sachin Sant <sachinp@linux.ibm.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Martin Wilck <mwilck@suse.com>
> Reported-by: Sachin Sant <sachinp@linux.ibm.com>
> Fixes: 0b25e17e9018 ("scsi: alua: Move a scsi_device_put() call out of alua_check_vpd()")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 31 ++++++++++++++--------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 

Just FYI, we stumbled over this as well in our CI with zFCP as device
driver and the linux-next cut from yesterday `next-20221116` (it started
a couple of days ago).

When I load the module (and all the SCSI devices get sensed/attached) I
get a seemingly endless stream in Inquiry retries:

[  482.281990] zfcp 0.0.1700: qdio: ZFCP on SC 364 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W
[  482.308284] scsi host0: scsi_eh_0: sleeping
[  482.308355] scsi host0: zfcp
[  482.337627] scsi 0:0:0:16: scsi scan: INQUIRY pass 1 length 36
[  482.337803] scsi 0:0:0:16: scsi scan: INQUIRY successful with code 0x0
[  482.337816] scsi 0:0:0:16: scsi scan: INQUIRY pass 2 length 164
[  482.337987] scsi 0:0:0:16: scsi scan: INQUIRY successful with code 0x0
[  482.337995] scsi 0:0:0:16: Direct-Access     IBM      2107900          2.19 PQ: 0 ANSI: 5
[  482.339397] scsi 0:0:0:16: alua: supports implicit TPGS
[  482.339405] scsi 0:0:0:16: alua: device naa.6005076309ffd430000000000000181a port group 0 rel port 1
[  482.339517] sd 0:0:0:16: sg_alloc: dev=0
[  482.339566] sd 0:0:0:16: Attached scsi generic sg0 type 0
[  482.339907] sd 0:0:0:16: Power-on or device reset occurred
[  482.339923] sd 0:0:0:16: [sda] tag#2560 Done: ADD_TO_MLQUEUE Result: hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[  482.339930] sd 0:0:0:16: [sda] tag#2560 CDB: Test Unit Ready 00 00 00 00 00 00
[  482.339936] sd 0:0:0:16: [sda] tag#2560 Sense Key : Unit Attention [current]
[  482.339942] sd 0:0:0:16: [sda] tag#2560 Add. Sense: Power on, reset, or bus device reset occurred
[  482.388213] sd 0:0:0:16: [sda] 20971520 512-byte logical blocks: (10.7 GB/10.0 GiB)
[  482.388339] sd 0:0:0:16: [sda] Write Protect is off
[  482.388341] sd 0:0:0:16: alua: transition timeout set to 60 seconds
[  482.388342] sd 0:0:0:16: [sda] Mode Sense: ed 00 00 08
[  482.388346] sd 0:0:0:16: alua: port group 00 state A preferred supports tolusnA
[  482.388532] sd 0:0:0:16: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[  482.388631] sd 0:0:0:16: [sda] tag#2565 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=0s
[  482.388636] sd 0:0:0:16: [sda] tag#2565 CDB: Report supported operation codes a3 0c 01 12 00 00 00 00 00 0a 00 00
[  482.388640] sd 0:0:0:16: [sda] tag#2565 Sense Key : Illegal Request [current]
[  482.388644] sd 0:0:0:16: [sda] tag#2565 Add. Sense: Invalid field in cdb
[  482.388696] scsi 0:0:1:1073758284: scsi scan: INQUIRY pass 1 length 36
[  482.388706] scsi 0:0:1:1073758284: tag#576 Done: NEEDS_RETRY Result: hostbyte=DID_IMM_RETRY driverbyte=DRIVER_OK cmd_age=0s
[  482.388710] scsi 0:0:1:1073758284: tag#576 CDB: Inquiry 12 80 00 00 24 00
[  482.392545] sd 0:0:0:16: [sda] Attached SCSI disk
[  482.437995] scsi 0:0:1:1073758284: tag#576 Done: NEEDS_RETRY Result: hostbyte=DID_IMM_RETRY driverbyte=DRIVER_OK cmd_age=0s
[  482.437998] scsi 0:0:1:1073758284: tag#576 CDB: Inquiry 12 80 00 00 24 00
[  482.497958] scsi 0:0:1:1073758284: tag#576 Done: NEEDS_RETRY Result: hostbyte=DID_IMM_RETRY driverbyte=DRIVER_OK cmd_age=0s
[  482.497965] scsi 0:0:1:1073758284: tag#576 CDB: Inquiry 12 80 00 00 24 00
[  482.537967] scsi 0:0:1:1073758284: tag#576 Done: NEEDS_RETRY Result: hostbyte=DID_IMM_RETRY driverbyte=DRIVER_OK cmd_age=0s
[  482.537970] scsi 0:0:1:1073758284: tag#576 CDB: Inquiry 12 80 00 00 24 00
[  482.588004] scsi 0:0:1:1073758284: tag#576 Done: NEEDS_RETRY Result: hostbyte=DID_IMM_RETRY driverbyte=DRIVER_OK cmd_age=0s
[  482.588008] scsi 0:0:1:1073758284: tag#576 CDB: Inquiry 12 80 00 00 24 00
...

This continues until the command eventually times out. This seems to
have the nock-on effect that systems booted from SCSI volumes hang after
boot.

-- 
Best Regards, Benjamin Block  / Linux on IBM Z Kernel Development / IBM Systems
IBM Deutschland Research & Development GmbH    /    https://www.ibm.com/privacy
Vorsitz. AufsR.: Gregor Pillen         /         Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-17 17:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-15 22:49 [PATCH] scsi: alua: Fix alua_rtpg_queue() Bart Van Assche
2022-11-16 10:32 ` Martin Wilck
2022-11-16 21:22   ` Bart Van Assche
2022-11-17  7:03     ` Martin Wilck
2022-11-17 17:57       ` Bart Van Assche
2022-11-17 17:57 ` Benjamin Block

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox