* [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put()
@ 2022-11-17 18:36 Bart Van Assche
2022-11-17 18:36 ` [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()" Bart Van Assche
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Bart Van Assche @ 2022-11-17 18:36 UTC (permalink / raw)
To: Martin K . Petersen; +Cc: linux-scsi, Bart Van Assche
Hi Martin,
This patch series reworks how the SCSI ALUA device handler calls
scsi_device_put(). Please consider this patch series for the next merge
window.
Thanks,
Bart.
Bart Van Assche (2):
scsi: alua: Revert "Move a scsi_device_put() call out of
alua_check_vpd()"
scsi: alua: Call scsi_device_put() from non-atomic context
drivers/scsi/device_handler/scsi_dh_alua.c | 46 ++++++++++++----------
1 file changed, 25 insertions(+), 21 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()"
2022-11-17 18:36 [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Bart Van Assche
@ 2022-11-17 18:36 ` Bart Van Assche
2022-11-18 6:11 ` Sachin Sant
2022-11-17 18:36 ` [PATCH 2/2] scsi: alua: Call scsi_device_put() from non-atomic context Bart Van Assche
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Bart Van Assche @ 2022-11-17 18:36 UTC (permalink / raw)
To: Martin K . Petersen
Cc: linux-scsi, Bart Van Assche, Hannes Reinecke, Martin Wilck,
Sachin Sant, Benjamin Block
There is a bug in commit 0b25e17e9018 ("scsi: alua: Move a
scsi_device_put() call out of alua_check_vpd()"): that patch may cause
alua_rtpg_queue() callers to call scsi_device_put() even if that function
should not be called. Revert that commit to prepare for a different
solution.
Cc: Hannes Reinecke <hare@suse.de>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Sachin Sant <sachinp@linux.ibm.com>
Cc: Benjamin Block <bblock@linux.ibm.com>
Reported-by: Sachin Sant <sachinp@linux.ibm.com>
Reported-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/device_handler/scsi_dh_alua.c | 23 ++++++++--------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 693cd827e138..bd4ee294f5c7 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -324,7 +324,6 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
struct alua_port_group *pg, *old_pg = NULL;
bool pg_updated = false;
unsigned long flags;
- bool put_sdev;
group_id = scsi_vpd_tpg_id(sdev, &rel_port);
if (group_id < 0) {
@@ -374,14 +373,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);
- 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);
spin_unlock(&h->pg_lock);
- if (put_sdev)
- scsi_device_put(sdev);
-
if (old_pg)
kref_put(&old_pg->kref, release_port_group);
@@ -982,10 +978,9 @@ static void alua_rtpg_work(struct work_struct *work)
* RTPG already has been scheduled.
*
* 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).
+ * That function is responsible for calling @qdata->fn().
*/
-static bool __must_check alua_rtpg_queue(struct alua_port_group *pg,
+static bool alua_rtpg_queue(struct alua_port_group *pg,
struct scsi_device *sdev,
struct alua_queue_data *qdata, bool force)
{
@@ -1024,6 +1019,8 @@ static bool __must_check alua_rtpg_queue(struct alua_port_group *pg,
else
kref_put(&pg->kref, release_port_group);
}
+ if (sdev)
+ scsi_device_put(sdev);
return true;
}
@@ -1130,12 +1127,10 @@ 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, qdata, true))
fn = NULL;
- } else {
+ else
err = SCSI_DH_DEV_OFFLINED;
- }
kref_put(&pg->kref, release_port_group);
out:
if (fn)
@@ -1161,9 +1156,7 @@ static void alua_check(struct scsi_device *sdev, bool force)
return;
}
rcu_read_unlock();
-
- if (alua_rtpg_queue(pg, sdev, NULL, force))
- scsi_device_put(sdev);
+ alua_rtpg_queue(pg, sdev, NULL, force);
kref_put(&pg->kref, release_port_group);
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] scsi: alua: Call scsi_device_put() from non-atomic context
2022-11-17 18:36 [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Bart Van Assche
2022-11-17 18:36 ` [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()" Bart Van Assche
@ 2022-11-17 18:36 ` Bart Van Assche
2022-11-26 0:34 ` [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Martin K. Petersen
2022-12-01 3:45 ` Martin K. Petersen
3 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2022-11-17 18:36 UTC (permalink / raw)
To: Martin K . Petersen
Cc: linux-scsi, Bart Van Assche, Martin Wilck, Hannes Reinecke,
Sachin Sant, Benjamin Block, Dan Carpenter
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.
This patch fixes the following smatch warning:
drivers/scsi/device_handler/scsi_dh_alua.c:1013 alua_rtpg_queue() warn: sleeping in atomic context
alua_check_vpd() <- disables preempt
-> alua_rtpg_queue()
-> scsi_device_put()
Cc: Martin Wilck <mwilck@suse.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sachin Sant <sachinp@linux.ibm.com>
Cc: Benjamin Block <bblock@linux.ibm.com>
Suggested-by: Martin Wilck <mwilck@suse.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/device_handler/scsi_dh_alua.c | 27 +++++++++++++++-------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index bd4ee294f5c7..49cc18a87473 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 (h && 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 */
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()"
2022-11-17 18:36 ` [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()" Bart Van Assche
@ 2022-11-18 6:11 ` Sachin Sant
2022-11-18 15:07 ` Bart Van Assche
0 siblings, 1 reply; 10+ messages in thread
From: Sachin Sant @ 2022-11-18 6:11 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, linux-scsi, Hannes Reinecke, Martin Wilck,
Benjamin Block
> On 18-Nov-2022, at 12:06 AM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> There is a bug in commit 0b25e17e9018 ("scsi: alua: Move a
> scsi_device_put() call out of alua_check_vpd()"): that patch may cause
> alua_rtpg_queue() callers to call scsi_device_put() even if that function
> should not be called. Revert that commit to prepare for a different
> solution.
>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: Sachin Sant <sachinp@linux.ibm.com>
> Cc: Benjamin Block <bblock@linux.ibm.com>
> Reported-by: Sachin Sant <sachinp@linux.ibm.com>
> Reported-by: Benjamin Block <bblock@linux.ibm.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/device_handler/scsi_dh_alua.c | 23 ++++++++--------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
Thanks for the patch.
Tested it on top of next-20221117. The reported warning is not seen.
Tested-by: Sachin Sant <sachinp@linux.ibm.com <mailto:sachinp@linux.ibm.com>>
- Sachin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()"
2022-11-18 6:11 ` Sachin Sant
@ 2022-11-18 15:07 ` Bart Van Assche
2022-11-18 16:03 ` Sachin Sant
0 siblings, 1 reply; 10+ messages in thread
From: Bart Van Assche @ 2022-11-18 15:07 UTC (permalink / raw)
To: Sachin Sant
Cc: Martin K . Petersen, linux-scsi, Hannes Reinecke, Martin Wilck,
Benjamin Block
On 11/17/22 22:11, Sachin Sant wrote:
> Tested it on top of next-20221117. The reported warning is not seen.
>
> Tested-by: Sachin Sant <sachinp@linux.ibm.com <mailto:sachinp@linux.ibm.com>>
Hi Sachin,
Thank you for the help with testing.
Can you also test patch 2/2 from this series
(https://lore.kernel.org/all/20221117183626.2656196-3-bvanassche@acm.org/)?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()"
2022-11-18 15:07 ` Bart Van Assche
@ 2022-11-18 16:03 ` Sachin Sant
2022-11-18 18:54 ` Bart Van Assche
0 siblings, 1 reply; 10+ messages in thread
From: Sachin Sant @ 2022-11-18 16:03 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, linux-scsi, Hannes Reinecke, Martin Wilck,
Benjamin Block
> On 18-Nov-2022, at 8:37 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 11/17/22 22:11, Sachin Sant wrote:
>> Tested it on top of next-20221117. The reported warning is not seen.
>> Tested-by: Sachin Sant <sachinp@linux.ibm.com <mailto:sachinp@linux.ibm.com>>
>
> Hi Sachin,
>
> Thank you for the help with testing.
>
> Can you also test patch 2/2 from this series (https://lore.kernel.org/all/20221117183626.2656196-3-bvanassche@acm.org/)?
I tested with both the patches applied on top of next-20221117.
- Sachin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()"
2022-11-18 16:03 ` Sachin Sant
@ 2022-11-18 18:54 ` Bart Van Assche
2022-11-19 9:46 ` Sachin Sant
0 siblings, 1 reply; 10+ messages in thread
From: Bart Van Assche @ 2022-11-18 18:54 UTC (permalink / raw)
To: Sachin Sant
Cc: Martin K . Petersen, linux-scsi, Hannes Reinecke, Martin Wilck,
Benjamin Block
On 11/18/22 08:03, Sachin Sant wrote:
>> On 18-Nov-2022, at 8:37 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>> Can you also test patch 2/2 from this series (https://lore.kernel.org/all/20221117183626.2656196-3-bvanassche@acm.org/)?
>
> I tested with both the patches applied on top of next-20221117.
Thank you Sachin for having confirmed this. In the future when testing an
entire patch series, consider replying with "Tested-by:" to the cover letter
instead of the first patch. I think that is the conventional way to indicate
that a patch series has been tested in its entirety instead of a single
patch from a series.
Bart.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()"
2022-11-18 18:54 ` Bart Van Assche
@ 2022-11-19 9:46 ` Sachin Sant
0 siblings, 0 replies; 10+ messages in thread
From: Sachin Sant @ 2022-11-19 9:46 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, linux-scsi, Hannes Reinecke, Martin Wilck,
Benjamin Block
> On 19-Nov-2022, at 12:24 AM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 11/18/22 08:03, Sachin Sant wrote:
>>> On 18-Nov-2022, at 8:37 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>>> Can you also test patch 2/2 from this series (https://lore.kernel.org/all/20221117183626.2656196-3-bvanassche@acm.org/)?
>> I tested with both the patches applied on top of next-20221117.
>
> Thank you Sachin for having confirmed this. In the future when testing an
> entire patch series, consider replying with "Tested-by:" to the cover letter
> instead of the first patch. I think that is the conventional way to indicate
> that a patch series has been tested in its entirety instead of a single
> patch from a series.
>
I did not receive the cover letter so replied to the first patch.
I should have explicitly called out that I have tested both the patches.
Sorry about the confusion.
- Sachin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put()
2022-11-17 18:36 [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Bart Van Assche
2022-11-17 18:36 ` [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()" Bart Van Assche
2022-11-17 18:36 ` [PATCH 2/2] scsi: alua: Call scsi_device_put() from non-atomic context Bart Van Assche
@ 2022-11-26 0:34 ` Martin K. Petersen
2022-12-01 3:45 ` Martin K. Petersen
3 siblings, 0 replies; 10+ messages in thread
From: Martin K. Petersen @ 2022-11-26 0:34 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi
Bart,
> This patch series reworks how the SCSI ALUA device handler calls
> scsi_device_put(). Please consider this patch series for the next
> merge window.
Applied to 6.2/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put()
2022-11-17 18:36 [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Bart Van Assche
` (2 preceding siblings ...)
2022-11-26 0:34 ` [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Martin K. Petersen
@ 2022-12-01 3:45 ` Martin K. Petersen
3 siblings, 0 replies; 10+ messages in thread
From: Martin K. Petersen @ 2022-12-01 3:45 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi
On Thu, 17 Nov 2022 10:36:24 -0800, Bart Van Assche wrote:
> This patch series reworks how the SCSI ALUA device handler calls
> scsi_device_put(). Please consider this patch series for the next merge
> window.
>
> Thanks,
>
> Bart.
>
> [...]
Applied to 6.2/scsi-queue, thanks!
[1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()"
https://git.kernel.org/mkp/scsi/c/a500c4cc06cd
[2/2] scsi: alua: Call scsi_device_put() from non-atomic context
https://git.kernel.org/mkp/scsi/c/50759b881e1d
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-12-01 3:45 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 18:36 [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Bart Van Assche
2022-11-17 18:36 ` [PATCH 1/2] scsi: alua: Revert "Move a scsi_device_put() call out of alua_check_vpd()" Bart Van Assche
2022-11-18 6:11 ` Sachin Sant
2022-11-18 15:07 ` Bart Van Assche
2022-11-18 16:03 ` Sachin Sant
2022-11-18 18:54 ` Bart Van Assche
2022-11-19 9:46 ` Sachin Sant
2022-11-17 18:36 ` [PATCH 2/2] scsi: alua: Call scsi_device_put() from non-atomic context Bart Van Assche
2022-11-26 0:34 ` [PATCH 0/2] Rework how the ALUA driver calls scsi_device_put() Martin K. Petersen
2022-12-01 3:45 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).