* [PATCH] scsi: scsi_dh_alua: set handler_data before alua_initialize()
@ 2026-11-09 20:11 Laurence Oberman
2026-11-11 11:14 ` Martin Wilck
2026-11-15 18:04 ` John Garry
0 siblings, 2 replies; 4+ messages in thread
From: Laurence Oberman @ 2026-11-09 20:11 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Martin Wilck,
Hannes Reinecke, John Garry, linux-kernel, stable
Since commit 50759b881e1d ("scsi: device_handler: alua: Call
scsi_device_put() from non-atomic context"), alua_rtpg_queue() only
queues an RTPG when sdev->handler_data points at an alua_dh_data whose
port group matches:
struct alua_dh_data *h = sdev->handler_data;
...
if (h && rcu_dereference(h->pg) == pg) {
However alua_bus_attach() only assigns sdev->handler_data after
alua_initialize() returns, and alua_initialize() -> alua_check_vpd()
is where the initial RTPG is queued. At that point handler_data is
still NULL, so the initial RTPG is silently dropped: no RTPG is sent,
nothing is logged, and sdev->access_state keeps its zeroed default of
active/optimized for every path.
With dm-multipath this goes unnoticed, as pg_init calls
scsi_dh_activate() once the map is loaded, which queues an RTPG after
handler_data is set, and multipathd's alua prioritizer issues its own
RTPG from userspace. Anything relying on sdev->access_state without a
subsequent activate (sysfs access_state/preferred_path, native SCSI
multipath path selection) sees every path as active/optimized until a
Unit Attention or rescan triggers an RTPG.
Seen on an HP P2000 G3 FC array (implicit ALUA, two port groups, four
paths per LUN) with dm-multipath not in use: sg_rtpg reports port group
0 active/non-optimized and port group 1 active/optimized, but
/sys/class/scsi_device/H:C:T:L/device/access_state reads
active/optimized on all four paths after boot, and no
"alua: port group XX state ..." message is logged. Writing "activate"
to dh_state sends the RTPG and corrects access_state.
Kernels without commit 50759b881e1d queue the attach-time RTPG
unconditionally and are not affected.
Fix by assigning sdev->handler_data before calling alua_initialize(),
and clearing it again on failure. sdev->handler is only set once
scsi_dh_handler_attach() sees ->attach() succeed, so no handler
callback can observe the partially initialised alua_dh_data. On the
failure paths of alua_initialize(), h has not been added to a port
group and no RTPG has been queued, so freeing it is unchanged.
Fixes: 50759b881e1d ("scsi: device_handler: alua: Call scsi_device_put() from non-atomic context")
Cc: stable@vger.kernel.org
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
Notes:
Tested on an HP P2000 G3 FC array (30 LUNs, 4 paths each): with this
patch the "alua: port group 00 state N ..." messages are logged during
boot and access_state shows two active/optimized and two
active/non-optimized paths per LUN without any manual activate.
Without it, all paths read active/optimized until an RTPG is triggered.
I found this while testing John Garry's native SCSI multipath series
(v9) on an HP P2000 G3 FC array (30 LUNs, 4 paths each, dm-multipath
not in use). Native multipath uses sdev->access_state for path
selection, and every path showed active/optimized, although sg_rtpg
and dm-multipath (alua prio 50/10) both report port group 0 as
active/non-optimized. No "alua: port group XX state ..." message was
logged at boot. Writing "activate" to dh_state corrected access_state,
which pointed at the attach-time RTPG never being queued.
With this patch, the port group state messages are logged during boot
and access_state shows two active/optimized and two
active/non-optimized paths per LUN without any manual activate.
This fix is independent of the native multipath series. It affects any
user of access_state/preferred_path, and ALUA prep_fn fast-fail, on
kernels with 50759b881e1d where nothing calls scsi_dh_activate()
after attach.
v9 series: https://lore.kernel.org/linux-scsi/20261108204733.557995-1-
john.garry@linux.dev/
drivers/scsi/device_handler/scsi_dh_alua.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 87ac0a6d63e5..84ff3aed2c46 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -1233,13 +1233,20 @@ static int alua_bus_attach(struct scsi_device *sdev)
INIT_LIST_HEAD(&h->node);
mutex_init(&h->init_mutex);
+ /*
+ * alua_rtpg_queue() only queues an RTPG when sdev->handler_data
+ * points at this alua_dh_data, so set it before alua_initialize()
+ * otherwise the initial RTPG issued from alua_check_vpd() is
+ * silently dropped and sdev->access_state is never populated.
+ */
+ sdev->handler_data = h;
err = alua_initialize(sdev, h);
if (err != SCSI_DH_OK && err != SCSI_DH_DEV_OFFLINED)
goto failed;
- sdev->handler_data = h;
return SCSI_DH_OK;
failed:
+ sdev->handler_data = NULL;
kfree(h);
return err;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] scsi: scsi_dh_alua: set handler_data before alua_initialize() 2026-11-09 20:11 [PATCH] scsi: scsi_dh_alua: set handler_data before alua_initialize() Laurence Oberman @ 2026-11-11 11:14 ` Martin Wilck 2026-11-15 18:04 ` John Garry 1 sibling, 0 replies; 4+ messages in thread From: Martin Wilck @ 2026-11-11 11:14 UTC (permalink / raw) To: Laurence Oberman, Martin K . Petersen, linux-scsi Cc: James E . J . Bottomley, Bart Van Assche, Hannes Reinecke, John Garry, linux-kernel, stable On Mon, 2026-11-09 at 15:11 -0500, Laurence Oberman wrote: > Since commit 50759b881e1d ("scsi: device_handler: alua: Call > scsi_device_put() from non-atomic context"), alua_rtpg_queue() only > queues an RTPG when sdev->handler_data points at an alua_dh_data > whose > port group matches: > > struct alua_dh_data *h = sdev->handler_data; > ... > if (h && rcu_dereference(h->pg) == pg) { > > However alua_bus_attach() only assigns sdev->handler_data after > alua_initialize() returns, and alua_initialize() -> alua_check_vpd() > is where the initial RTPG is queued. At that point handler_data is > still NULL, so the initial RTPG is silently dropped: no RTPG is sent, > nothing is logged, and sdev->access_state keeps its zeroed default of > active/optimized for every path. > > With dm-multipath this goes unnoticed, as pg_init calls > scsi_dh_activate() once the map is loaded, which queues an RTPG after > handler_data is set, and multipathd's alua prioritizer issues its own > RTPG from userspace. Anything relying on sdev->access_state without a > subsequent activate (sysfs access_state/preferred_path, native SCSI > multipath path selection) sees every path as active/optimized until a > Unit Attention or rescan triggers an RTPG. > > Seen on an HP P2000 G3 FC array (implicit ALUA, two port groups, four > paths per LUN) with dm-multipath not in use: sg_rtpg reports port > group > 0 active/non-optimized and port group 1 active/optimized, but > /sys/class/scsi_device/H:C:T:L/device/access_state reads > active/optimized on all four paths after boot, and no > "alua: port group XX state ..." message is logged. Writing "activate" > to dh_state sends the RTPG and corrects access_state. > > Kernels without commit 50759b881e1d queue the attach-time RTPG > unconditionally and are not affected. > > Fix by assigning sdev->handler_data before calling alua_initialize(), > and clearing it again on failure. sdev->handler is only set once > scsi_dh_handler_attach() sees ->attach() succeed, so no handler > callback can observe the partially initialised alua_dh_data. On the > failure paths of alua_initialize(), h has not been added to a port > group and no RTPG has been queued, so freeing it is unchanged. > > Fixes: 50759b881e1d ("scsi: device_handler: alua: Call > scsi_device_put() from non-atomic context") > Cc: stable@vger.kernel.org > Signed-off-by: Laurence Oberman <loberman@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com> -- Dr. Martin Wilck <mwilck@suse.com> SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg, Germany Geschäftsführer: Stefan Gaiser, Jochen Jaser, Abhinav Puri (HRB 36809,AG Nürnberg) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: scsi_dh_alua: set handler_data before alua_initialize() 2026-11-09 20:11 [PATCH] scsi: scsi_dh_alua: set handler_data before alua_initialize() Laurence Oberman 2026-11-11 11:14 ` Martin Wilck @ 2026-11-15 18:04 ` John Garry 2026-11-15 19:37 ` Laurence Oberman 1 sibling, 1 reply; 4+ messages in thread From: John Garry @ 2026-11-15 18:04 UTC (permalink / raw) To: Laurence Oberman, Martin K . Petersen, linux-scsi Cc: James E . J . Bottomley, Bart Van Assche, Martin Wilck, Hannes Reinecke, linux-kernel, stable On 11/9/26 20:11, Laurence Oberman wrote: > Since commit 50759b881e1d ("scsi: device_handler: alua: Call > scsi_device_put() from non-atomic context"), alua_rtpg_queue() only > queues an RTPG when sdev->handler_data points at an alua_dh_data whose > port group matches: > > struct alua_dh_data *h = sdev->handler_data; > ... > if (h && rcu_dereference(h->pg) == pg) { Do we still require this check (for h being non-NULL) after the change in this patch? I assume that we flush any calls to this function prior to unsetting sdev->handler_data. > > However alua_bus_attach() only assigns sdev->handler_data after > alua_initialize() returns, and alua_initialize() -> alua_check_vpd() > is where the initial RTPG is queued. At that point handler_data is > still NULL, so the initial RTPG is silently dropped: no RTPG is sent, > nothing is logged, and sdev->access_state keeps its zeroed default of > active/optimized for every path. > > With dm-multipath this goes unnoticed, as pg_init calls > scsi_dh_activate() once the map is loaded, which queues an RTPG after > handler_data is set, and multipathd's alua prioritizer issues its own > RTPG from userspace. Anything relying on sdev->access_state without a > subsequent activate (sysfs access_state/preferred_path, native SCSI > multipath path selection) sees every path as active/optimized until a > Unit Attention or rescan triggers an RTPG. > > Seen on an HP P2000 G3 FC array (implicit ALUA, two port groups, four > paths per LUN) with dm-multipath not in use: sg_rtpg reports port group > 0 active/non-optimized and port group 1 active/optimized, but > /sys/class/scsi_device/H:C:T:L/device/access_state reads > active/optimized on all four paths after boot, and no > "alua: port group XX state ..." message is logged. Writing "activate" > to dh_state sends the RTPG and corrects access_state. > > Kernels without commit 50759b881e1d queue the attach-time RTPG > unconditionally and are not affected. > > Fix by assigning sdev->handler_data before calling alua_initialize(), > and clearing it again on failure. sdev->handler is only set once > scsi_dh_handler_attach() sees ->attach() succeed, so no handler > callback can observe the partially initialised alua_dh_data. On the > failure paths of alua_initialize(), h has not been added to a port > group and no RTPG has been queued, so freeing it is unchanged. > > Fixes: 50759b881e1d ("scsi: device_handler: alua: Call scsi_device_put() from non-atomic context") > Cc: stable@vger.kernel.org > Signed-off-by: Laurence Oberman <loberman@redhat.com> > --- > > Notes: > Tested on an HP P2000 G3 FC array (30 LUNs, 4 paths each): with this > patch the "alua: port group 00 state N ..." messages are logged during > boot and access_state shows two active/optimized and two > active/non-optimized paths per LUN without any manual activate. > Without it, all paths read active/optimized until an RTPG is triggered. > > I found this while testing John Garry's native SCSI multipath series > (v9) on an HP P2000 G3 FC array (30 LUNs, 4 paths each, dm-multipath > not in use). Native multipath uses sdev->access_state for path > selection, and every path showed active/optimized, although sg_rtpg > and dm-multipath (alua prio 50/10) both report port group 0 as > active/non-optimized. No "alua: port group XX state ..." message was > logged at boot. Writing "activate" to dh_state corrected access_state, > which pointed at the attach-time RTPG never being queued. > > With this patch, the port group state messages are logged during boot > and access_state shows two active/optimized and two > active/non-optimized paths per LUN without any manual activate. > > This fix is independent of the native multipath series. It affects any > user of access_state/preferred_path, and ALUA prep_fn fast-fail, on > kernels with 50759b881e1d where nothing calls scsi_dh_activate() > after attach. > > v9 series: https://lore.kernel.org/linux-scsi/20261108204733.557995-1- > john.garry@linux.dev/ > > drivers/scsi/device_handler/scsi_dh_alua.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c > index 87ac0a6d63e5..84ff3aed2c46 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -1233,13 +1233,20 @@ static int alua_bus_attach(struct scsi_device *sdev) > INIT_LIST_HEAD(&h->node); > > mutex_init(&h->init_mutex); > + /* > + * alua_rtpg_queue() only queues an RTPG when sdev->handler_data > + * points at this alua_dh_data, so set it before alua_initialize() > + * otherwise the initial RTPG issued from alua_check_vpd() is > + * silently dropped and sdev->access_state is never populated. > + */ > + sdev->handler_data = h; Note: I was checking which members of alua_dh_data are not initialized prior to publishing sdev->handler_data, and alua_dh_data.group_id is never accessed at all AFAICS. Thanks, John ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: scsi_dh_alua: set handler_data before alua_initialize() 2026-11-15 18:04 ` John Garry @ 2026-11-15 19:37 ` Laurence Oberman 0 siblings, 0 replies; 4+ messages in thread From: Laurence Oberman @ 2026-11-15 19:37 UTC (permalink / raw) To: John Garry, Martin K . Petersen, linux-scsi Cc: James E . J . Bottomley, Bart Van Assche, Martin Wilck, Hannes Reinecke, linux-kernel, stable On Sun, 2026-11-15 at 18:04 +0000, John Garry wrote: > On 11/9/26 20:11, Laurence Oberman wrote: > > Since commit 50759b881e1d ("scsi: device_handler: alua: Call > > scsi_device_put() from non-atomic context"), alua_rtpg_queue() only > > queues an RTPG when sdev->handler_data points at an alua_dh_data > > whose > > port group matches: > > > > struct alua_dh_data *h = sdev->handler_data; > > ... > > if (h && rcu_dereference(h->pg) == pg) { > > Do we still require this check (for h being non-NULL) after the > change > in this patch? I assume that we flush any calls to this function > prior > to unsetting sdev->handler_data. > > > > > However alua_bus_attach() only assigns sdev->handler_data after > > alua_initialize() returns, and alua_initialize() -> > > alua_check_vpd() > > is where the initial RTPG is queued. At that point handler_data is > > still NULL, so the initial RTPG is silently dropped: no RTPG is > > sent, > > nothing is logged, and sdev->access_state keeps its zeroed default > > of > > active/optimized for every path. > > > > With dm-multipath this goes unnoticed, as pg_init calls > > scsi_dh_activate() once the map is loaded, which queues an RTPG > > after > > handler_data is set, and multipathd's alua prioritizer issues its > > own > > RTPG from userspace. Anything relying on sdev->access_state without > > a > > subsequent activate (sysfs access_state/preferred_path, native SCSI > > multipath path selection) sees every path as active/optimized until > > a > > Unit Attention or rescan triggers an RTPG. > > > > Seen on an HP P2000 G3 FC array (implicit ALUA, two port groups, > > four > > paths per LUN) with dm-multipath not in use: sg_rtpg reports port > > group > > 0 active/non-optimized and port group 1 active/optimized, but > > /sys/class/scsi_device/H:C:T:L/device/access_state reads > > active/optimized on all four paths after boot, and no > > "alua: port group XX state ..." message is logged. Writing > > "activate" > > to dh_state sends the RTPG and corrects access_state. > > > > Kernels without commit 50759b881e1d queue the attach-time RTPG > > unconditionally and are not affected. > > > > Fix by assigning sdev->handler_data before calling > > alua_initialize(), > > and clearing it again on failure. sdev->handler is only set once > > scsi_dh_handler_attach() sees ->attach() succeed, so no handler > > callback can observe the partially initialised alua_dh_data. On the > > failure paths of alua_initialize(), h has not been added to a port > > group and no RTPG has been queued, so freeing it is unchanged. > > > > Fixes: 50759b881e1d ("scsi: device_handler: alua: Call > > scsi_device_put() from non-atomic context") > > Cc: stable@vger.kernel.org > > Signed-off-by: Laurence Oberman <loberman@redhat.com> > > --- > > > > Notes: > > Tested on an HP P2000 G3 FC array (30 LUNs, 4 paths each): > > with this > > patch the "alua: port group 00 state N ..." messages are > > logged during > > boot and access_state shows two active/optimized and two > > active/non-optimized paths per LUN without any manual > > activate. > > Without it, all paths read active/optimized until an RTPG is > > triggered. > > > > I found this while testing John Garry's native SCSI multipath > > series > > (v9) on an HP P2000 G3 FC array (30 LUNs, 4 paths each, dm- > > multipath > > not in use). Native multipath uses sdev->access_state for path > > selection, and every path showed active/optimized, although > > sg_rtpg > > and dm-multipath (alua prio 50/10) both report port group 0 as > > active/non-optimized. No "alua: port group XX state ..." > > message was > > logged at boot. Writing "activate" to dh_state corrected > > access_state, > > which pointed at the attach-time RTPG never being queued. > > > > With this patch, the port group state messages are logged > > during boot > > and access_state shows two active/optimized and two > > active/non-optimized paths per LUN without any manual > > activate. > > > > This fix is independent of the native multipath series. It > > affects any > > user of access_state/preferred_path, and ALUA prep_fn fast- > > fail, on > > kernels with 50759b881e1d where nothing calls > > scsi_dh_activate() > > after attach. > > > > v9 series: > > https://lore.kernel.org/linux-scsi/20261108204733.557995-1- > > john.garry@linux.dev/ > > > > drivers/scsi/device_handler/scsi_dh_alua.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c > > b/drivers/scsi/device_handler/scsi_dh_alua.c > > index 87ac0a6d63e5..84ff3aed2c46 100644 > > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > > @@ -1233,13 +1233,20 @@ static int alua_bus_attach(struct > > scsi_device *sdev) > > INIT_LIST_HEAD(&h->node); > > > > mutex_init(&h->init_mutex); > > + /* > > + * alua_rtpg_queue() only queues an RTPG when sdev- > > >handler_data > > + * points at this alua_dh_data, so set it before > > alua_initialize() > > + * otherwise the initial RTPG issued from alua_check_vpd() > > is > > + * silently dropped and sdev->access_state is never > > populated. > > + */ > > + sdev->handler_data = h; > > Note: I was checking which members of alua_dh_data are not > initialized > prior to publishing sdev->handler_data, and alua_dh_data.group_id is > never accessed at all AFAICS. > > Thanks, > John On 11/15/26, John Garry wrote: > Do we still require this check (for h being non-NULL) after the change > in this patch? I assume that we flush any calls to this function prior > to unsetting sdev->handler_data. Hi John, Thanks for looking at this. You're right, with this patch the NULL check is redundant. All three callers already have a valid h: - alua_check_vpd() is called from attach/rescan with h passed in, and sdev->handler_data now points at it before the initial RTPG is queued - alua_activate() and alua_check() both dereference sdev->handler_data (h->init_mutex, h->pg) before calling alua_rtpg_queue() There is no explicit flush, but there doesn't need to be: handler_data is only cleared in alua_bus_detach(), which runs from scsi_dh_release_device() in scsi_device_dev_release(), i.e. after the last sdev reference is dropped. Every caller holds a reference, alua_rtpg_queue() takes its own scsi_device_get() before reading handler_data, and queued RTPG work holds one through pg->rtpg_sdev. The rcu_dereference(h->pg) == pg part is still needed, as a rescan can move h to a different port group. > Note: I was checking which members of alua_dh_data are not initialized > prior to publishing sdev->handler_data, and alua_dh_data.group_id is > never accessed at all AFAICS. Agreed, group_id is never read or written. All other members are initialized before handler_data is set (h is kzalloc'd, and disabled is set under init_mutex in alua_initialize()). I would prefer to keep this patch minimal since it is tagged for stable, and send a follow-up cleanup that drops the now-redundant h check in alua_rtpg_queue() and removes the unused group_id member. Does that work for you, or would you rather I fold both into a v2? Thanks, Laurence ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-15 19:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-09 20:11 [PATCH] scsi: scsi_dh_alua: set handler_data before alua_initialize() Laurence Oberman 2026-11-11 11:14 ` Martin Wilck 2026-11-15 18:04 ` John Garry 2026-11-15 19:37 ` Laurence Oberman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox