* [PATCH 01/11] Documentation: scsi: libsas: Remove notify_ha_event()
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-21 9:10 ` John Garry
2020-12-18 20:43 ` [PATCH 02/11] scsi: libsas: Introduce a _gfp() variant of event notifiers Ahmed S. Darwish
` (10 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
The ->notify_ha_event() hook has long been removed from the libsas event
interface.
Remove it from documentation.
Fixes: 042ebd293b86 ("scsi: libsas: kill useless ha_event and do some cleanup")
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: stable@vger.kernel.org
Cc: John Garry <john.garry@huawei.com>
Cc: Jason Yan <yanaijie@huawei.com>
---
Documentation/scsi/libsas.rst | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/scsi/libsas.rst b/Documentation/scsi/libsas.rst
index 7216b5d25800..f9b77c7879db 100644
--- a/Documentation/scsi/libsas.rst
+++ b/Documentation/scsi/libsas.rst
@@ -189,7 +189,6 @@ num_phys
The event interface::
/* LLDD calls these to notify the class of an event. */
- void (*notify_ha_event)(struct sas_ha_struct *, enum ha_event);
void (*notify_port_event)(struct sas_phy *, enum port_event);
void (*notify_phy_event)(struct sas_phy *, enum phy_event);
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 01/11] Documentation: scsi: libsas: Remove notify_ha_event()
2020-12-18 20:43 ` [PATCH 01/11] Documentation: scsi: libsas: Remove notify_ha_event() Ahmed S. Darwish
@ 2020-12-21 9:10 ` John Garry
0 siblings, 0 replies; 21+ messages in thread
From: John Garry @ 2020-12-21 9:10 UTC (permalink / raw)
To: Ahmed S. Darwish, James E.J. Bottomley, Martin K. Petersen,
Daniel Wagner, Jason Yan, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior
On 18/12/2020 20:43, Ahmed S. Darwish wrote:
> The ->notify_ha_event() hook has long been removed from the libsas event
> interface.
>
> Remove it from documentation.
>
> Fixes: 042ebd293b86 ("scsi: libsas: kill useless ha_event and do some cleanup")
> Signed-off-by: Ahmed S. Darwish<a.darwish@linutronix.de>
> Cc:stable@vger.kernel.org
> Cc: John Garry<john.garry@huawei.com>
> Cc: Jason Yan<yanaijie@huawei.com>
Reviewed-by: John Garry <john.garry@huawei.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 02/11] scsi: libsas: Introduce a _gfp() variant of event notifiers
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 01/11] Documentation: scsi: libsas: Remove notify_ha_event() Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 03/11] scsi: mvsas: Pass gfp_t flags to libsas " Ahmed S. Darwish
` (9 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
sas_alloc_event() uses in_interrupt() to decide which allocation should
be used.
The usage of in_interrupt() in drivers is phased out and Linus clearly
requested that code which changes behaviour depending on context should
either be separated or the context be conveyed in an argument passed by
the caller, which usually knows the context.
The in_interrupt() check is also only partially correct, because it
fails to choose the correct code path when just preemption or interrupts
are disabled. For example, as in the following call chain:
drivers/scsi/mvsas/mv_sas.c :: mvs_work_queue() [process context]
spin_lock_irqsave()
-> sas_ha->notify_phy_event()
== drivers/scsi/libsas/sas_event.c :: sas_notify_phy_event()
-> sas_alloc_event()
-> in_interrupt() = false
-> invalid GFP_KERNEL allocation
-> sas_ha->notify_port_event()
== drivers/scsi/libsas/sas_event.c :: sas_notify_port_event()
-> sas_alloc_event()
-> in_interrupt() = false
-> invalid GFP_KERNEL allocation
Introduce sas_alloc_event_gfp(), sas_ha_struct::notify_port_event_gfp(),
and sas_ha_struct::notify_phy_event_gfp(), which all behave like the non
_gfp() variants but use a caller passed GFP mask for allocations.
After all callers are converted to pass the GFP context, the non _gfp()
variants will be removed.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: stable@vger.kernel.org
Cc: John Garry <john.garry@huawei.com>
Cc: Jason Yan <yanaijie@huawei.com>
---
Documentation/scsi/libsas.rst | 2 +
drivers/scsi/libsas/sas_event.c | 65 +++++++++++++++++++++++++-----
drivers/scsi/libsas/sas_init.c | 20 ++++++---
drivers/scsi/libsas/sas_internal.h | 2 +
include/scsi/libsas.h | 2 +
5 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/Documentation/scsi/libsas.rst b/Documentation/scsi/libsas.rst
index f9b77c7879db..dc85d0e4c107 100644
--- a/Documentation/scsi/libsas.rst
+++ b/Documentation/scsi/libsas.rst
@@ -191,6 +191,8 @@ The event interface::
/* LLDD calls these to notify the class of an event. */
void (*notify_port_event)(struct sas_phy *, enum port_event);
void (*notify_phy_event)(struct sas_phy *, enum phy_event);
+ void (*notify_port_event_gfp)(struct sas_phy *, enum port_event, gfp_t);
+ void (*notify_phy_event_gfp)(struct sas_phy *, enum phy_event, gfp_t);
When sas_register_ha() returns, those are set and can be
called by the LLDD to notify the SAS layer of such events
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index a1852f6c042b..ed5a8325dca7 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -131,18 +131,14 @@ static void sas_phy_event_worker(struct work_struct *work)
sas_free_event(ev);
}
-static int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event)
+static int __sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event,
+ struct asd_sas_event *ev)
{
- struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
int ret;
BUG_ON(event >= PORT_NUM_EVENTS);
- ev = sas_alloc_event(phy);
- if (!ev)
- return -ENOMEM;
-
INIT_SAS_EVENT(ev, sas_port_event_worker, phy, event);
ret = sas_queue_event(event, &ev->work, ha);
@@ -152,18 +148,39 @@ static int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event)
return ret;
}
-int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
+static int sas_notify_port_event_gfp(struct asd_sas_phy *phy,
+ enum port_event event,
+ gfp_t gfp_flags)
{
struct asd_sas_event *ev;
+
+ ev = sas_alloc_event_gfp(phy, gfp_flags);
+ if (!ev)
+ return -ENOMEM;
+
+ return __sas_notify_port_event(phy, event, ev);
+}
+
+static int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event)
+{
+ struct asd_sas_event *ev;
+
+ ev = sas_alloc_event(phy);
+ if (!ev)
+ return -ENOMEM;
+
+ return __sas_notify_port_event(phy, event, ev);
+}
+
+static inline int __sas_notify_phy_event(struct asd_sas_phy *phy,
+ enum phy_event event,
+ struct asd_sas_event *ev)
+{
struct sas_ha_struct *ha = phy->ha;
int ret;
BUG_ON(event >= PHY_NUM_EVENTS);
- ev = sas_alloc_event(phy);
- if (!ev)
- return -ENOMEM;
-
INIT_SAS_EVENT(ev, sas_phy_event_worker, phy, event);
ret = sas_queue_event(event, &ev->work, ha);
@@ -173,10 +190,36 @@ int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
return ret;
}
+int sas_notify_phy_event_gfp(struct asd_sas_phy *phy, enum phy_event event,
+ gfp_t gfp_flags)
+{
+ struct asd_sas_event *ev;
+
+ ev = sas_alloc_event_gfp(phy, gfp_flags);
+ if (!ev)
+ return -ENOMEM;
+
+ return __sas_notify_phy_event(phy, event, ev);
+}
+
+int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
+{
+ struct asd_sas_event *ev;
+
+ ev = sas_alloc_event(phy);
+ if (!ev)
+ return -ENOMEM;
+
+ return __sas_notify_phy_event(phy, event, ev);
+}
+
int sas_init_events(struct sas_ha_struct *sas_ha)
{
sas_ha->notify_port_event = sas_notify_port_event;
sas_ha->notify_phy_event = sas_notify_phy_event;
+ sas_ha->notify_port_event_gfp = sas_notify_port_event_gfp;
+ sas_ha->notify_phy_event_gfp = sas_notify_phy_event_gfp;
+
return 0;
}
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 21c43b18d5d5..9269d524158f 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -590,16 +590,15 @@ sas_domain_attach_transport(struct sas_domain_function_template *dft)
}
EXPORT_SYMBOL_GPL(sas_domain_attach_transport);
-
-struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
+static struct asd_sas_event * __sas_alloc_event(struct asd_sas_phy *phy,
+ gfp_t gfp_flags)
{
struct asd_sas_event *event;
- gfp_t flags = in_interrupt() ? GFP_ATOMIC : GFP_KERNEL;
struct sas_ha_struct *sas_ha = phy->ha;
struct sas_internal *i =
to_sas_internal(sas_ha->core.shost->transportt);
- event = kmem_cache_zalloc(sas_event_cache, flags);
+ event = kmem_cache_zalloc(sas_event_cache, gfp_flags);
if (!event)
return NULL;
@@ -610,7 +609,7 @@ struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
if (cmpxchg(&phy->in_shutdown, 0, 1) == 0) {
pr_notice("The phy%d bursting events, shut it down.\n",
phy->id);
- sas_notify_phy_event(phy, PHYE_SHUTDOWN);
+ sas_notify_phy_event_gfp(phy, PHYE_SHUTDOWN, gfp_flags);
}
} else {
/* Do not support PHY control, stop allocating events */
@@ -624,6 +623,17 @@ struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
return event;
}
+struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
+{
+ return __sas_alloc_event(phy, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
+}
+
+struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy,
+ gfp_t gfp_flags)
+{
+ return __sas_alloc_event(phy, gfp_flags);
+}
+
void sas_free_event(struct asd_sas_event *event)
{
struct asd_sas_phy *phy = event->phy;
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 1f1d01901978..437a697b6f73 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -49,6 +49,7 @@ int sas_register_phys(struct sas_ha_struct *sas_ha);
void sas_unregister_phys(struct sas_ha_struct *sas_ha);
struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy);
+struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy, gfp_t gfp_flags);
void sas_free_event(struct asd_sas_event *event);
int sas_register_ports(struct sas_ha_struct *sas_ha);
@@ -78,6 +79,7 @@ int sas_smp_phy_control(struct domain_device *dev, int phy_id,
int sas_smp_get_phy_events(struct sas_phy *phy);
int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event);
+int sas_notify_phy_event_gfp(struct asd_sas_phy *phy, enum phy_event event, gfp_t flags);
void sas_device_set_phy(struct domain_device *dev, struct sas_port *port);
struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy);
struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 4e2d61e8fb1e..f7c2530bbd9d 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -394,6 +394,8 @@ struct sas_ha_struct {
/* LLDD calls these to notify the class of an event. */
int (*notify_port_event)(struct asd_sas_phy *, enum port_event);
int (*notify_phy_event)(struct asd_sas_phy *, enum phy_event);
+ int (*notify_port_event_gfp)(struct asd_sas_phy *, enum port_event, gfp_t);
+ int (*notify_phy_event_gfp)(struct asd_sas_phy *, enum phy_event, gfp_t);
void *lldd_ha; /* not touched by sas class code */
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 03/11] scsi: mvsas: Pass gfp_t flags to libsas event notifiers
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 01/11] Documentation: scsi: libsas: Remove notify_ha_event() Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 02/11] scsi: libsas: Introduce a _gfp() variant of event notifiers Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 04/11] scsi: isci: port: link down: Pass gfp_t flags Ahmed S. Darwish
` (8 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
mvsas calls the non _gfp version of the libsas event notifiers API,
leading to the buggy call chains below:
mvsas/mv_sas.c :: mvs_work_queue() [process context]
spin_lock_irqsave(mvs_info::lock, )
-> sas_ha->notify_phy_event()
== libsas/sas_event.c :: sas_notify_phy_event()
-> sas_alloc_event()
-> in_interrupt() = false
-> invalid GFP_KERNEL allocation
-> sas_ha->notify_port_event()
== libsas/sas_event.c :: sas_notify_port_event()
-> sas_alloc_event()
-> in_interrupt() = false
-> invalid GFP_KERNEL allocation
Use the new event notifiers API instead, which requires callers to
explicitly pass the gfp_t memory allocation flags.
Below are context analysis for the modified functions:
=> mvs_bytes_dmaed():
Since it is invoked from both process and atomic contexts, let its
callers pass the gfp_t flags. Call chains:
scsi_scan.c: do_scsi_scan_host() [has msleep()]
-> shost->hostt->scan_start()
-> [mvsas/mv_init.c: Scsi_Host::scsi_host_template .scan_start = mvs_scan_start()]
-> mvsas/mv_sas.c: mvs_scan_start()
-> mvs_bytes_dmaed(..., GFP_KERNEL)
mvsas/mv_sas.c: mvs_work_queue()
spin_lock_irqsave(mvs_info::lock,)
-> mvs_bytes_dmaed(..., GFP_ATOMIC)
mvsas/mv_64xx.c: mvs_64xx_isr() || mvsas/mv_94xx.c: mvs_94xx_isr()
-> mvsas/mv_chips.h: mvs_int_full()
-> mvsas/mv_sas.c: mvs_int_port()
-> mvs_bytes_dmaed(..., GFP_ATOMIC);
=> mvs_work_queue():
Invoked from process context, but it calls all the libsas event notifier
APIs under a spin_lock_irqsave(). Pass GFP_ATOMIC.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: stable@vger.kernel.org
Cc: John Garry <john.garry@huawei.com>
Cc: Jason Yan <yanaijie@huawei.com>
---
drivers/scsi/mvsas/mv_sas.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a920eced92ec..30a34c5bdf6b 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -216,7 +216,7 @@ void mvs_set_sas_addr(struct mvs_info *mvi, int port_id, u32 off_lo,
MVS_CHIP_DISP->write_port_cfg_data(mvi, port_id, hi);
}
-static void mvs_bytes_dmaed(struct mvs_info *mvi, int i)
+static void mvs_bytes_dmaed(struct mvs_info *mvi, int i, gfp_t gfp_flags)
{
struct mvs_phy *phy = &mvi->phy[i];
struct asd_sas_phy *sas_phy = &phy->sas_phy;
@@ -230,7 +230,7 @@ static void mvs_bytes_dmaed(struct mvs_info *mvi, int i)
}
sas_ha = mvi->sas;
- sas_ha->notify_phy_event(sas_phy, PHYE_OOB_DONE);
+ sas_ha->notify_phy_event_gfp(sas_phy, PHYE_OOB_DONE, gfp_flags);
if (sas_phy->phy) {
struct sas_phy *sphy = sas_phy->phy;
@@ -262,8 +262,8 @@ static void mvs_bytes_dmaed(struct mvs_info *mvi, int i)
sas_phy->frame_rcvd_size = phy->frame_rcvd_size;
- mvi->sas->notify_port_event(sas_phy,
- PORTE_BYTES_DMAED);
+ mvi->sas->notify_port_event_gfp(sas_phy,
+ PORTE_BYTES_DMAED, gfp_flags);
}
void mvs_scan_start(struct Scsi_Host *shost)
@@ -279,7 +279,7 @@ void mvs_scan_start(struct Scsi_Host *shost)
for (j = 0; j < core_nr; j++) {
mvi = ((struct mvs_prv_info *)sha->lldd_ha)->mvi[j];
for (i = 0; i < mvi->chip->n_phy; ++i)
- mvs_bytes_dmaed(mvi, i);
+ mvs_bytes_dmaed(mvi, i, GFP_KERNEL);
}
mvs_prv->scan_finished = 1;
}
@@ -1895,21 +1895,21 @@ static void mvs_work_queue(struct work_struct *work)
if (!(tmp & PHY_READY_MASK)) {
sas_phy_disconnected(sas_phy);
mvs_phy_disconnected(phy);
- sas_ha->notify_phy_event(sas_phy,
- PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(sas_phy,
+ PHYE_LOSS_OF_SIGNAL, GFP_ATOMIC);
mv_dprintk("phy%d Removed Device\n", phy_no);
} else {
MVS_CHIP_DISP->detect_porttype(mvi, phy_no);
mvs_update_phyinfo(mvi, phy_no, 1);
- mvs_bytes_dmaed(mvi, phy_no);
+ mvs_bytes_dmaed(mvi, phy_no, GFP_ATOMIC);
mvs_port_notify_formed(sas_phy, 0);
mv_dprintk("phy%d Attached Device\n", phy_no);
}
}
} else if (mwq->handler & EXP_BRCT_CHG) {
phy->phy_event &= ~EXP_BRCT_CHG;
- sas_ha->notify_port_event(sas_phy,
- PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy,
+ PORTE_BROADCAST_RCVD, GFP_ATOMIC);
mv_dprintk("phy%d Got Broadcast Change\n", phy_no);
}
list_del(&mwq->entry);
@@ -2026,7 +2026,7 @@ void mvs_int_port(struct mvs_info *mvi, int phy_no, u32 events)
mdelay(10);
}
- mvs_bytes_dmaed(mvi, phy_no);
+ mvs_bytes_dmaed(mvi, phy_no, GFP_ATOMIC);
/* whether driver is going to handle hot plug */
if (phy->phy_event & PHY_PLUG_OUT) {
mvs_port_notify_formed(&phy->sas_phy, 0);
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 04/11] scsi: isci: port: link down: Pass gfp_t flags
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (2 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 03/11] scsi: mvsas: Pass gfp_t flags to libsas " Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 05/11] scsi: isci: port: link up: " Ahmed S. Darwish
` (7 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
Use the new libsas event notifiers API, which requires callers to
explicitly pass the gfp_t memory allocation flags.
The libsas ->notify_phy_event() hook is exclusively called from
isci_port_link_down(). Below is the context analysis for all of its
call chains:
port.c: port_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> port_state_machine_change(..., SCI_PORT_FAILED)
-> enter SCI port state: *SCI_PORT_FAILED*
-> sci_port_failed_state_enter()
-> isci_port_hard_reset_complete()
-> isci_port_link_down()
port.c: isci_port_perform_hard_reset()
spin_lock_irqsave(isci_host::scic_lock, )
-> port.c: sci_port_hard_reset(), atomic (*)
-> phy.c: sci_phy_reset()
-> sci_change_state(SCI_PHY_RESETTING)
-> enter SCI PHY state: *SCI_PHY_RESETTING*
-> sci_phy_resetting_state_enter()
-> port.c: sci_port_deactivate_phy()
-> isci_port_link_down()
port.c: enter SCI port state: *SCI_PORT_READY* # Cont. from [1]
-> sci_port_ready_state_enter()
-> isci_port_hard_reset_complete()
-> isci_port_link_down()
phy.c: enter SCI state: *SCI_PHY_STOPPED* # Cont. from [2]
-> sci_phy_stopped_state_enter()
-> host.c: sci_controller_link_down()
-> ->link_down_handler()
== port_config.c: sci_apc_agent_link_down()
-> port.c: sci_port_remove_phy()
-> sci_port_deactivate_phy()
-> isci_port_link_down()
== port_config.c: sci_mpc_agent_link_down()
-> port.c: sci_port_link_down()
-> sci_port_deactivate_phy()
-> isci_port_link_down()
phy.c: enter SCI state: *SCI_PHY_STARTING* # Cont. from [3]
-> sci_phy_starting_state_enter()
-> host.c: sci_controller_link_down()
-> ->link_down_handler()
== port_config.c: sci_apc_agent_link_down()
-> port.c: sci_port_remove_phy()
-> isci_port_link_down()
== port_config.c: sci_mpc_agent_link_down()
-> port.c: sci_port_link_down()
-> sci_port_deactivate_phy()
-> isci_port_link_down()
[1] Call chains for 'enter SCI port state: *SCI_PORT_READY*'
------------------------------------------------------------
host.c: isci_host_init() (@)
spin_lock_irq(isci_host::scic_lock)
-> sci_controller_initialize(), atomic (*)
-> port_config.c: sci_port_configuration_agent_initialize()
-> sci_mpc_agent_validate_phy_configuration()
-> port.c: sci_port_add_phy()
-> sci_port_general_link_up_handler()
-> port_state_machine_change(, SCI_PORT_READY)
-> enter port state *SCI_PORT_READY*
host.c: isci_host_start() (@)
spin_lock_irq(isci_host::scic_lock)
-> host.c: sci_controller_start(), atomic (*)
-> host.c: sci_port_start()
-> port.c: port_state_machine_change(, SCI_PORT_READY)
-> enter port state *SCI_PORT_READY*
port_config.c: apc_agent_timeout(), atomic, timer callback (*)
-> sci_apc_agent_configure_ports()
-> port.c: sci_port_add_phy()
-> sci_port_general_link_up_handler()
-> port_state_machine_change(, SCI_PORT_READY)
-> enter port state *SCI_PORT_READY*
port_config.c: mpc_agent_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> ->link_up_handler()
== port.c: sci_apc_agent_link_up()
-> sci_port_general_link_up_handler()
-> port_state_machine_change(, SCI_PORT_READY)
-> enter port state *SCI_PORT_READY*
== port.c: sci_mpc_agent_link_up()
-> port.c: sci_port_link_up()
-> sci_port_general_link_up_handler()
-> port_state_machine_change(, SCI_PORT_READY)
-> enter port state *SCI_PORT_READY*
phy.c: enter SCI state: SCI_PHY_SUB_FINAL # Cont. from [1A]
-> sci_phy_starting_final_substate_enter()
-> sci_change_state(SCI_PHY_READY)
-> enter SCI state: *SCI_PHY_READY*
-> sci_phy_ready_state_enter()
-> host.c: sci_controller_link_up()
-> port_agent.link_up_handler()
-> port_config.c: [->link_up_handler = sci_apc_agent_link_up]
sci_apc_agent_link_up()
-> port.c: sci_port_link_up()
-> sci_port_general_link_up_handler()
-> port_state_machine_change(, SCI_PORT_READY)
-> enter port state *SCI_PORT_READY*
-> port_config.c: [->link_up_handler = sci_mpc_agent_link_up]
sci_mpc_agent_link_up()
-> port.c: sci_port_link_up()
-> sci_port_general_link_up_handler()
-> port_state_machine_change(, SCI_PORT_READY)
-> enter port state *SCI_PORT_READY*
[1A] Call chains for entering SCI state: *SCI_PHY_SUB_FINAL*
------------------------------------------------------------
host.c: power_control_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> phy.c: sci_phy_consume_power_handler()
-> phy.c: sci_change_state(SCI_PHY_SUB_FINAL)
host.c: sci_controller_error_handler(): atomic, irq handler (*)
OR host.c: sci_controller_completion_handler(), atomic, tasklet (*)
-> sci_controller_process_completions()
-> sci_controller_unsolicited_frame()
-> phy.c: sci_phy_frame_handler()
-> sci_change_state(SCI_PHY_SUB_AWAIT_SAS_POWER)
-> sci_phy_starting_await_sas_power_substate_enter()
-> host.c: sci_controller_power_control_queue_insert()
-> phy.c: sci_phy_consume_power_handler()
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_controller_event_completion()
-> phy.c: sci_phy_event_handler()
-> sci_phy_start_sata_link_training()
-> sci_change_state(SCI_PHY_SUB_AWAIT_SATA_POWER)
-> sci_phy_starting_await_sata_power_substate_enter
-> host.c: sci_controller_power_control_queue_insert()
-> phy.c: sci_phy_consume_power_handler()
-> sci_change_state(SCI_PHY_SUB_FINAL)
[2] Call chains for entering state: *SCI_PHY_STOPPED*
-----------------------------------------------------
host.c: isci_host_init() (@)
spin_lock_irq(isci_host::scic_lock)
-> sci_controller_initialize(), atomic (*)
-> phy.c: sci_phy_initialize()
-> phy.c: sci_phy_link_layer_initialization()
-> phy.c: sci_change_state(SCI_PHY_STOPPED)
init.c: PCI ->remove() || PM_OPS ->suspend, process context (+)
-> host.c: isci_host_deinit()
-> sci_controller_stop_phys()
-> phy.c: sci_phy_stop()
-> sci_change_state(SCI_PHY_STOPPED)
phy.c: isci_phy_control()
spin_lock_irqsave(isci_host::scic_lock, )
-> sci_phy_stop(), atomic (*)
-> sci_change_state(SCI_PHY_STOPPED)
[3] Call chains for entering state: *SCI_PHY_STARTING*
------------------------------------------------------
phy.c: phy_sata_timeout(), atimer, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> sci_change_state(SCI_PHY_STARTING)
host.c: phy_startup_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> sci_controller_start_next_phy()
-> sci_phy_start()
-> sci_change_state(SCI_PHY_STARTING)
host.c: isci_host_start() (@)
spin_lock_irq(isci_host::scic_lock)
-> sci_controller_start(), atomic (*)
-> sci_controller_start_next_phy()
-> sci_phy_start()
-> sci_change_state(SCI_PHY_STARTING)
phy.c: Enter SCI state *SCI_PHY_SUB_FINAL*, atomic, check above (*)
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_phy_starting_final_substate_enter()
-> sci_change_state(SCI_PHY_READY)
-> Enter SCI state: *SCI_PHY_READY*
-> sci_phy_ready_state_enter()
-> host.c: sci_controller_link_up()
-> sci_controller_start_next_phy()
-> sci_phy_start()
-> sci_change_state(SCI_PHY_STARTING)
phy.c: sci_phy_event_handler(), atomic, discussed earlier (*)
-> sci_change_state(SCI_PHY_STARTING), 11 instances
phy.c: enter SCI state: *SCI_PHY_RESETTING*, atomic, discussed (*)
-> sci_phy_resetting_state_enter()
-> sci_change_state(SCI_PHY_STARTING)
As can be seen from the "(*)" markers above, almost all the call-chains
are atomic. The only exception, marked with "(+)", is a PCI ->remove()
and PM_OPS ->suspend() cold path. Thus, pass GFP_ATOMIC to the libsas
phy event notifier.
Note, The now-replaced libsas APIs used in_interrupt() to implicitly
decide which memory allocation type to use. This was only partially
correct, as it fails to choose the correct GFP flags when just
preemption or interrupts are disabled. Such buggy code paths are marked
with "(@)" in the call chains above.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: stable@vger.kernel.org
Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
drivers/scsi/isci/port.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index 1df45f028ea7..c3a8c84b19a2 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -270,8 +270,9 @@ static void isci_port_link_down(struct isci_host *isci_host,
* isci_port_deformed and isci_dev_gone functions.
*/
sas_phy_disconnected(&isci_phy->sas_phy);
- isci_host->sas_ha.notify_phy_event(&isci_phy->sas_phy,
- PHYE_LOSS_OF_SIGNAL);
+ isci_host->sas_ha.notify_phy_event_gfp(&isci_phy->sas_phy,
+ PHYE_LOSS_OF_SIGNAL,
+ GFP_ATOMIC);
dev_dbg(&isci_host->pdev->dev,
"%s: isci_port = %p - Done\n", __func__, isci_port);
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/11] scsi: isci: port: link up: Pass gfp_t flags
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (3 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 04/11] scsi: isci: port: link down: Pass gfp_t flags Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 06/11] scsi: isci: port: broadcast change: " Ahmed S. Darwish
` (6 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
Use the new libsas event notifiers API, which requires callers to
explicitly pass the gfp_t memory allocation flags.
The libsas ->notify_port_event() hook is called from
isci_port_link_up(). Below is the context analysis for all of its call
chains:
host.c: isci_host_init() (@)
spin_lock_irq(isci_host::scic_lock)
-> sci_controller_initialize(), atomic (*)
-> port_config.c: sci_port_configuration_agent_initialize()
-> sci_mpc_agent_validate_phy_configuration()
-> port.c: sci_port_add_phy()
-> sci_port_general_link_up_handler()
-> sci_port_activate_phy()
-> isci_port_link_up()
port_config.c: apc_agent_timeout(), atomic, timer callback (*)
-> sci_apc_agent_configure_ports()
-> port.c: sci_port_add_phy()
-> sci_port_general_link_up_handler()
-> sci_port_activate_phy()
-> isci_port_link_up()
phy.c: enter SCI state: *SCI_PHY_SUB_FINAL* # Cont. from [1]
-> phy.c: sci_phy_starting_final_substate_enter()
-> phy.c: sci_change_state(SCI_PHY_READY)
-> enter SCI state: *SCI_PHY_READY*
-> phy.c: sci_phy_ready_state_enter()
-> host.c: sci_controller_link_up()
-> .link_up_handler()
== port_config.c: sci_apc_agent_link_up()
-> port.c: sci_port_link_up()
-> (continue at [A])
== port_config.c: sci_mpc_agent_link_up()
-> port.c: sci_port_link_up()
-> (continue at [A])
port_config.c: mpc_agent_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> ->link_up_handler()
== port_config.c: sci_apc_agent_link_up()
-> port.c: sci_port_link_up()
-> (continue at [A])
== port_config.c: sci_mpc_agent_link_up()
-> port.c: sci_port_link_up()
-> (continue at [A])
[A] port.c: sci_port_link_up()
-> sci_port_activate_phy()
-> isci_port_link_up()
-> sci_port_general_link_up_handler()
-> sci_port_activate_phy()
-> isci_port_link_up()
[1] Call chains for entering SCI state: *SCI_PHY_SUB_FINAL*
-----------------------------------------------------------
host.c: power_control_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> phy.c: sci_phy_consume_power_handler()
-> phy.c: sci_change_state(SCI_PHY_SUB_FINAL)
host.c: sci_controller_error_handler(): atomic, irq handler (*)
OR host.c: sci_controller_completion_handler(), atomic, tasklet (*)
-> sci_controller_process_completions()
-> sci_controller_unsolicited_frame()
-> phy.c: sci_phy_frame_handler()
-> sci_change_state(SCI_PHY_SUB_AWAIT_SAS_POWER)
-> sci_phy_starting_await_sas_power_substate_enter()
-> host.c: sci_controller_power_control_queue_insert()
-> phy.c: sci_phy_consume_power_handler()
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_controller_event_completion()
-> phy.c: sci_phy_event_handler()
-> sci_phy_start_sata_link_training()
-> sci_change_state(SCI_PHY_SUB_AWAIT_SATA_POWER)
-> sci_phy_starting_await_sata_power_substate_enter
-> host.c: sci_controller_power_control_queue_insert()
-> phy.c: sci_phy_consume_power_handler()
-> sci_change_state(SCI_PHY_SUB_FINAL)
As can be seen from the "(*)" markers above, all the call-chains are
atomic. Pass GFP_ATOMIC to libsas port event notifier.
Note, the now-replaced libsas APIs used in_interrupt() to implicitly
decide which memory allocation type to use. This was only partially
correct, as it fails to choose the correct GFP flags when just
preemption or interrupts are disabled. Such buggy code paths are marked
with "(@)" in the call chains above.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: stable@vger.kernel.org
Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
drivers/scsi/isci/port.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index c3a8c84b19a2..69684c80c407 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -223,8 +223,9 @@ static void isci_port_link_up(struct isci_host *isci_host,
/* Notify libsas that we have an address frame, if indeed
* we've found an SSP, SMP, or STP target */
if (success)
- isci_host->sas_ha.notify_port_event(&iphy->sas_phy,
- PORTE_BYTES_DMAED);
+ isci_host->sas_ha.notify_port_event_gfp(&iphy->sas_phy,
+ PORTE_BYTES_DMAED,
+ GFP_ATOMIC);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 06/11] scsi: isci: port: broadcast change: Pass gfp_t flags
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (4 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 05/11] scsi: isci: port: link up: " Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 07/11] scsi: libsas: Pass gfp_t flags to event notifiers Ahmed S. Darwish
` (5 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
Use the new libsas event notifiers API, which requires callers to
explicitly pass the gfp_t memory allocation flags.
The libsas ->notify_port_event() hook is called from
isci_port_bc_change_received(). Below is the context analysis for all of
its call chains:
host.c: sci_controller_error_handler(): atomic, irq handler (*)
OR host.c: sci_controller_completion_handler(), atomic, tasklet (*)
-> sci_controller_process_completions()
-> sci_controller_event_completion()
-> phy.c: sci_phy_event_handler()
-> port.c: sci_port_broadcast_change_received()
-> isci_port_bc_change_received()
host.c: isci_host_init() (@)
spin_lock_irq(isci_host::scic_lock)
-> sci_controller_initialize(), atomic (*)
-> port_config.c: sci_port_configuration_agent_initialize()
-> sci_mpc_agent_validate_phy_configuration()
-> port.c: sci_port_add_phy()
-> sci_port_set_phy()
-> phy.c: sci_phy_set_port()
-> port.c: sci_port_broadcast_change_received()
-> isci_port_bc_change_received()
port_config.c: apc_agent_timeout(), atomic, timer callback (*)
-> sci_apc_agent_configure_ports()
-> port.c: sci_port_add_phy()
-> sci_port_set_phy()
-> phy.c: sci_phy_set_port()
-> port.c: sci_port_broadcast_change_received()
-> isci_port_bc_change_received()
phy.c: enter SCI state: *SCI_PHY_STOPPED* # Cont. from [1]
-> sci_phy_stopped_state_enter()
-> host.c: sci_controller_link_down()
-> ->link_down_handler()
== port_config.c: sci_apc_agent_link_down()
-> port.c: sci_port_remove_phy()
-> sci_port_clear_phy()
-> phy.c: sci_phy_set_port()
-> port.c: sci_port_broadcast_change_received()
-> isci_port_bc_change_received()
phy.c: enter SCI state: *SCI_PHY_STARTING* # Cont. from [2]
-> sci_phy_starting_state_enter()
-> host.c: sci_controller_link_down()
-> ->link_down_handler()
== port_config.c: sci_apc_agent_link_down()
-> port.c: sci_port_remove_phy()
-> sci_port_clear_phy()
-> phy.c: sci_phy_set_port()
-> port.c: sci_port_broadcast_change_received()
-> isci_port_bc_change_received()
[1] Call chains for entering state: *SCI_PHY_STOPPED*
-----------------------------------------------------
host.c: isci_host_init() (@)
spin_lock_irq(isci_host::scic_lock)
-> sci_controller_initialize(), atomic (*)
-> phy.c: sci_phy_initialize()
-> phy.c: sci_phy_link_layer_initialization()
-> phy.c: sci_change_state(SCI_PHY_STOPPED)
init.c: PCI ->remove() || PM_OPS ->suspend, process context (+)
-> host.c: isci_host_deinit()
-> sci_controller_stop_phys()
-> phy.c: sci_phy_stop()
-> sci_change_state(SCI_PHY_STOPPED)
phy.c: isci_phy_control()
spin_lock_irqsave(isci_host::scic_lock, )
-> sci_phy_stop(), atomic (*)
-> sci_change_state(SCI_PHY_STOPPED)
[2] Call chains for entering state: *SCI_PHY_STARTING*
------------------------------------------------------
phy.c: phy_sata_timeout(), atimer, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> sci_change_state(SCI_PHY_STARTING)
host.c: phy_startup_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> sci_controller_start_next_phy()
-> sci_phy_start()
-> sci_change_state(SCI_PHY_STARTING)
host.c: isci_host_start() (@)
spin_lock_irq(isci_host::scic_lock)
-> sci_controller_start(), atomic (*)
-> sci_controller_start_next_phy()
-> sci_phy_start()
-> sci_change_state(SCI_PHY_STARTING)
phy.c: Enter SCI state *SCI_PHY_SUB_FINAL* # Cont. from [2A]
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_phy_starting_final_substate_enter()
-> sci_change_state(SCI_PHY_READY)
-> Enter SCI state: *SCI_PHY_READY*
-> sci_phy_ready_state_enter()
-> host.c: sci_controller_link_up()
-> sci_controller_start_next_phy()
-> sci_phy_start()
-> sci_change_state(SCI_PHY_STARTING)
phy.c: sci_phy_event_handler(), atomic, discussed earlier (*)
-> sci_change_state(SCI_PHY_STARTING), 11 instances
port.c: isci_port_perform_hard_reset()
spin_lock_irqsave(isci_host::scic_lock, )
-> port.c: sci_port_hard_reset(), atomic (*)
-> phy.c: sci_phy_reset()
-> sci_change_state(SCI_PHY_RESETTING)
-> enter SCI PHY state: *SCI_PHY_RESETTING*
-> sci_phy_resetting_state_enter()
-> sci_change_state(SCI_PHY_STARTING)
[2A] Call chains for entering SCI state: *SCI_PHY_SUB_FINAL*
------------------------------------------------------------
host.c: power_control_timeout(), atomic, timer callback (*)
spin_lock_irqsave(isci_host::scic_lock, )
-> phy.c: sci_phy_consume_power_handler()
-> phy.c: sci_change_state(SCI_PHY_SUB_FINAL)
host.c: sci_controller_error_handler(): atomic, irq handler (*)
OR host.c: sci_controller_completion_handler(), atomic, tasklet (*)
-> sci_controller_process_completions()
-> sci_controller_unsolicited_frame()
-> phy.c: sci_phy_frame_handler()
-> sci_change_state(SCI_PHY_SUB_AWAIT_SAS_POWER)
-> sci_phy_starting_await_sas_power_substate_enter()
-> host.c: sci_controller_power_control_queue_insert()
-> phy.c: sci_phy_consume_power_handler()
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_change_state(SCI_PHY_SUB_FINAL)
-> sci_controller_event_completion()
-> phy.c: sci_phy_event_handler()
-> sci_phy_start_sata_link_training()
-> sci_change_state(SCI_PHY_SUB_AWAIT_SATA_POWER)
-> sci_phy_starting_await_sata_power_substate_enter
-> host.c: sci_controller_power_control_queue_insert()
-> phy.c: sci_phy_consume_power_handler()
-> sci_change_state(SCI_PHY_SUB_FINAL)
As can be seen from the "(*)" markers above, almost all the call-chains
are atomic. The only exception, marked with "(+)", is a PCI ->remove()
and PM_OPS ->suspend() cold path. Thus, pass GFP_ATOMIC to the libsas
port event notifier.
Note, the now-replaced libsas APIs used in_interrupt() to implicitly
decide which memory allocation type to use. This was only partially
correct, as it fails to choose the correct GFP flags when just
preemption or interrupts are disabled. Such buggy code paths are marked
with "(@)" in the call chains above.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: stable@vger.kernel.org
Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
---
drivers/scsi/isci/port.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index 69684c80c407..6244981a3ebf 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -164,7 +164,9 @@ static void isci_port_bc_change_received(struct isci_host *ihost,
"%s: isci_phy = %p, sas_phy = %p\n",
__func__, iphy, &iphy->sas_phy);
- ihost->sas_ha.notify_port_event(&iphy->sas_phy, PORTE_BROADCAST_RCVD);
+ ihost->sas_ha.notify_port_event_gfp(&iphy->sas_phy,
+ PORTE_BROADCAST_RCVD,
+ GFP_ATOMIC);
sci_port_bcn_enable(iport);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/11] scsi: libsas: Pass gfp_t flags to event notifiers
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (5 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 06/11] scsi: isci: port: broadcast change: " Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 08/11] scsi: pm80xx: Pass gfp_t flags to libsas " Ahmed S. Darwish
` (4 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
Use the new libsas event notifiers API, which requires callers to
explicitly pass the gfp_t memory allocation flags.
Context analysis:
- sas_enable_revalidation(): process, acquires mutex
- sas_resume_ha(): process, calls wait_event_timeout()
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: John Garry <john.garry@huawei.com>
Cc: Jason Yan <yanaijie@huawei.com>
---
drivers/scsi/libsas/sas_event.c | 2 +-
drivers/scsi/libsas/sas_init.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index ed5a8325dca7..31b733eeabf6 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -109,7 +109,7 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
sas_phy = container_of(port->phy_list.next, struct asd_sas_phy,
port_phy_el);
- ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_KERNEL);
}
mutex_unlock(&ha->disco_mutex);
}
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 9269d524158f..2d2116f827c6 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -410,7 +410,7 @@ void sas_resume_ha(struct sas_ha_struct *ha)
if (phy->suspended) {
dev_warn(&phy->phy->dev, "resume timeout\n");
- sas_notify_phy_event(phy, PHYE_RESUME_TIMEOUT);
+ sas_notify_phy_event_gfp(phy, PHYE_RESUME_TIMEOUT, GFP_KERNEL);
}
}
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/11] scsi: pm80xx: Pass gfp_t flags to libsas event notifiers
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (6 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 07/11] scsi: libsas: Pass gfp_t flags to event notifiers Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 09/11] scsi: aic94xx: " Ahmed S. Darwish
` (3 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
Use the new libsas event notifiers API, which requires callers to
explicitly pass the gfp_t memory allocation flags.
Call chain analysis, pm8001_hwi.c:
pm8001_interrupt_handler_msix() || pm8001_interrupt_handler_intx() || pm8001_tasklet()
-> PM8001_CHIP_DISP->isr() = pm80xx_chip_isr()
-> process_oq [spin_lock_irqsave(&pm8001_ha->lock, ...)]
-> process_one_iomb()
-> mpi_hw_event()
-> hw_event_sas_phy_up()
-> pm8001_bytes_dmaed()
-> hw_event_sata_phy_up
-> pm8001_bytes_dmaed()
All functions are invoked by process_one_iomb(), which is invoked by the
interrupt service routine and the tasklet handler. A similar call chain
is also found at pm80xx_hwi.c. Pass GFP_ATOMIC.
For pm8001_sas.c, pm8001_phy_control() runs in task context as it calls
wait_for_completion() and msleep(). Pass GFP_KERNEL.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/scsi/pm8001/pm8001_hwi.c | 38 ++++++++++++++++----------------
drivers/scsi/pm8001/pm8001_sas.c | 8 +++----
drivers/scsi/pm8001/pm80xx_hwi.c | 30 ++++++++++++-------------
3 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 2b7b2954ec31..36eb5cc6f2fa 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -3306,7 +3306,7 @@ void pm8001_bytes_dmaed(struct pm8001_hba_info *pm8001_ha, int i)
PM8001_MSG_DBG(pm8001_ha, pm8001_printk("phy %d byte dmaded.\n", i));
sas_phy->frame_rcvd_size = phy->frame_rcvd_size;
- pm8001_ha->sas->notify_port_event(sas_phy, PORTE_BYTES_DMAED);
+ pm8001_ha->sas->notify_port_event_gfp(sas_phy, PORTE_BYTES_DMAED, GFP_ATOMIC);
}
/* Get the link rate speed */
@@ -3467,7 +3467,7 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
else if (phy->identify.device_type != SAS_PHY_UNUSED)
phy->identify.target_port_protocols = SAS_PROTOCOL_SMP;
phy->sas_phy.oob_mode = SAS_OOB_MODE;
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_DONE, GFP_ATOMIC);
spin_lock_irqsave(&phy->sas_phy.frame_rcvd_lock, flags);
memcpy(phy->frame_rcvd, &pPayload->sas_identify,
sizeof(struct sas_identify_frame)-4);
@@ -3512,7 +3512,7 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
phy->phy_type |= PORT_TYPE_SATA;
phy->phy_attached = 1;
phy->sas_phy.oob_mode = SATA_OOB_MODE;
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_DONE, GFP_ATOMIC);
spin_lock_irqsave(&phy->sas_phy.frame_rcvd_lock, flags);
memcpy(phy->frame_rcvd, ((u8 *)&pPayload->sata_fis - 4),
sizeof(struct dev_to_host_fis));
@@ -3879,12 +3879,12 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
case HW_EVENT_SATA_SPINUP_HOLD:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD\n"));
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_SPINUP_HOLD, GFP_ATOMIC);
break;
case HW_EVENT_PHY_DOWN:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_PHY_DOWN\n"));
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_LOSS_OF_SIGNAL, GFP_ATOMIC);
phy->phy_attached = 0;
phy->phy_state = 0;
hw_event_phy_down(pm8001_ha, piomb);
@@ -3894,7 +3894,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
pm8001_printk("HW_EVENT_PORT_INVALID\n"));
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
/* the broadcast change primitive received, tell the LIBSAS this event
to revalidate the sas domain*/
@@ -3906,14 +3906,14 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
spin_lock_irqsave(&sas_phy->sas_prim_lock, flags);
sas_phy->sas_prim = HW_EVENT_BROADCAST_CHANGE;
spin_unlock_irqrestore(&sas_phy->sas_prim_lock, flags);
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
break;
case HW_EVENT_PHY_ERROR:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_PHY_ERROR\n"));
sas_phy_disconnected(&phy->sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_ERROR);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_ERROR, GFP_ATOMIC);
break;
case HW_EVENT_BROADCAST_EXP:
PM8001_MSG_DBG(pm8001_ha,
@@ -3921,7 +3921,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
spin_lock_irqsave(&sas_phy->sas_prim_lock, flags);
sas_phy->sas_prim = HW_EVENT_BROADCAST_EXP;
spin_unlock_irqrestore(&sas_phy->sas_prim_lock, flags);
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_INVALID_DWORD:
PM8001_MSG_DBG(pm8001_ha,
@@ -3930,7 +3930,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
HW_EVENT_LINK_ERR_INVALID_DWORD, port_id, phy_id, 0, 0);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_DISPARITY_ERROR:
PM8001_MSG_DBG(pm8001_ha,
@@ -3940,7 +3940,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
port_id, phy_id, 0, 0);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_CODE_VIOLATION:
PM8001_MSG_DBG(pm8001_ha,
@@ -3950,7 +3950,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
port_id, phy_id, 0, 0);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH:
PM8001_MSG_DBG(pm8001_ha,
@@ -3960,7 +3960,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
port_id, phy_id, 0, 0);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_MALFUNCTION:
PM8001_MSG_DBG(pm8001_ha,
@@ -3972,7 +3972,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
spin_lock_irqsave(&sas_phy->sas_prim_lock, flags);
sas_phy->sas_prim = HW_EVENT_BROADCAST_SES;
spin_unlock_irqrestore(&sas_phy->sas_prim_lock, flags);
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
break;
case HW_EVENT_INBOUND_CRC_ERROR:
PM8001_MSG_DBG(pm8001_ha,
@@ -3984,14 +3984,14 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
case HW_EVENT_HARD_RESET_RECEIVED:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_HARD_RESET_RECEIVED\n"));
- sas_ha->notify_port_event(sas_phy, PORTE_HARD_RESET);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_HARD_RESET, GFP_ATOMIC);
break;
case HW_EVENT_ID_FRAME_TIMEOUT:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_ID_FRAME_TIMEOUT\n"));
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
PM8001_MSG_DBG(pm8001_ha,
@@ -4001,21 +4001,21 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void* piomb)
port_id, phy_id, 0, 0);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_PORT_RESET_TIMER_TMO:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_PORT_RESET_TIMER_TMO\n"));
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_PORT_RECOVERY_TIMER_TMO\n"));
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_PORT_RECOVER:
PM8001_MSG_DBG(pm8001_ha,
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 9889bab7d31c..8850e62550a3 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -209,8 +209,8 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
PHY_STATE_LINK_UP_SPCV) {
sas_ha = pm8001_ha->sas;
sas_phy_disconnected(&phy->sas_phy);
- sas_ha->notify_phy_event(&phy->sas_phy,
- PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy,
+ PHYE_LOSS_OF_SIGNAL, GFP_KERNEL);
phy->phy_attached = 0;
}
} else {
@@ -218,8 +218,8 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
PHY_STATE_LINK_UP_SPC) {
sas_ha = pm8001_ha->sas;
sas_phy_disconnected(&phy->sas_phy);
- sas_ha->notify_phy_event(&phy->sas_phy,
- PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy,
+ PHYE_LOSS_OF_SIGNAL, GFP_KERNEL);
phy->phy_attached = 0;
}
}
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 7593f248afb2..a6fd08ae4402 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -3355,7 +3355,7 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
else if (phy->identify.device_type != SAS_PHY_UNUSED)
phy->identify.target_port_protocols = SAS_PROTOCOL_SMP;
phy->sas_phy.oob_mode = SAS_OOB_MODE;
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_DONE, GFP_ATOMIC);
spin_lock_irqsave(&phy->sas_phy.frame_rcvd_lock, flags);
memcpy(phy->frame_rcvd, &pPayload->sas_identify,
sizeof(struct sas_identify_frame)-4);
@@ -3403,7 +3403,7 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
phy->phy_type |= PORT_TYPE_SATA;
phy->phy_attached = 1;
phy->sas_phy.oob_mode = SATA_OOB_MODE;
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_DONE, GFP_ATOMIC);
spin_lock_irqsave(&phy->sas_phy.frame_rcvd_lock, flags);
memcpy(phy->frame_rcvd, ((u8 *)&pPayload->sata_fis - 4),
sizeof(struct dev_to_host_fis));
@@ -3489,7 +3489,7 @@ hw_event_phy_down(struct pm8001_hba_info *pm8001_ha, void *piomb)
if (port_sata && (portstate != PORT_IN_RESET)) {
struct sas_ha_struct *sas_ha = pm8001_ha->sas;
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_LOSS_OF_SIGNAL, GFP_ATOMIC);
}
}
@@ -3591,7 +3591,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
case HW_EVENT_SATA_SPINUP_HOLD:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD\n"));
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_SPINUP_HOLD, GFP_ATOMIC);
break;
case HW_EVENT_PHY_DOWN:
PM8001_MSG_DBG(pm8001_ha,
@@ -3610,7 +3610,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
pm8001_printk("HW_EVENT_PORT_INVALID\n"));
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
/* the broadcast change primitive received, tell the LIBSAS this event
to revalidate the sas domain*/
@@ -3622,14 +3622,14 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
spin_lock_irqsave(&sas_phy->sas_prim_lock, flags);
sas_phy->sas_prim = HW_EVENT_BROADCAST_CHANGE;
spin_unlock_irqrestore(&sas_phy->sas_prim_lock, flags);
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
break;
case HW_EVENT_PHY_ERROR:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_PHY_ERROR\n"));
sas_phy_disconnected(&phy->sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_ERROR);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_ERROR, GFP_ATOMIC);
break;
case HW_EVENT_BROADCAST_EXP:
PM8001_MSG_DBG(pm8001_ha,
@@ -3637,7 +3637,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
spin_lock_irqsave(&sas_phy->sas_prim_lock, flags);
sas_phy->sas_prim = HW_EVENT_BROADCAST_EXP;
spin_unlock_irqrestore(&sas_phy->sas_prim_lock, flags);
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_INVALID_DWORD:
PM8001_MSG_DBG(pm8001_ha,
@@ -3676,7 +3676,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
spin_lock_irqsave(&sas_phy->sas_prim_lock, flags);
sas_phy->sas_prim = HW_EVENT_BROADCAST_SES;
spin_unlock_irqrestore(&sas_phy->sas_prim_lock, flags);
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
break;
case HW_EVENT_INBOUND_CRC_ERROR:
PM8001_MSG_DBG(pm8001_ha,
@@ -3688,14 +3688,14 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
case HW_EVENT_HARD_RESET_RECEIVED:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_HARD_RESET_RECEIVED\n"));
- sas_ha->notify_port_event(sas_phy, PORTE_HARD_RESET);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_HARD_RESET, GFP_ATOMIC);
break;
case HW_EVENT_ID_FRAME_TIMEOUT:
PM8001_MSG_DBG(pm8001_ha,
pm8001_printk("HW_EVENT_ID_FRAME_TIMEOUT\n"));
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
PM8001_MSG_DBG(pm8001_ha,
@@ -3705,7 +3705,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
port_id, phy_id, 0, 0);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
break;
case HW_EVENT_PORT_RESET_TIMER_TMO:
PM8001_MSG_DBG(pm8001_ha,
@@ -3714,7 +3714,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
port_id, phy_id, 0, 0);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
if (pm8001_ha->phy[phy_id].reset_completion) {
pm8001_ha->phy[phy_id].port_reset_status =
PORT_RESET_TMO;
@@ -3731,8 +3731,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
for (i = 0; i < pm8001_ha->chip->n_phy; i++) {
if (port->wide_port_phymap & (1 << i)) {
phy = &pm8001_ha->phy[i];
- sas_ha->notify_phy_event(&phy->sas_phy,
- PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy,
+ PHYE_LOSS_OF_SIGNAL, GFP_ATOMIC);
port->wide_port_phymap &= ~(1 << i);
}
}
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/11] scsi: aic94xx: Pass gfp_t flags to libsas event notifiers
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (7 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 08/11] scsi: pm80xx: Pass gfp_t flags to libsas " Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 10/11] scsi: hisi_sas: " Ahmed S. Darwish
` (2 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
Use the new libsas event notifiers API, which requires callers to
explicitly pass the gfp_t memory allocation flags.
Context analysis:
aic94xx_hwi.c: asd_dl_tasklet_handler()
-> asd_ascb::tasklet_complete()
== escb_tasklet_complete()
-> aic94xx_scb.c: asd_phy_event_tasklet()
-> aic94xx_scb.c: asd_bytes_dmaed_tasklet()
-> aic94xx_scb.c: asd_link_reset_err_tasklet()
-> aic94xx_scb.c: asd_primitive_rcvd_tasklet()
All functions are invoked by escb_tasklet_complete(), which is invoked
by the tasklet handler. Pass GFP_ATOMIC.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
---
drivers/scsi/aic94xx/aic94xx_scb.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_scb.c b/drivers/scsi/aic94xx/aic94xx_scb.c
index e2d880a5f391..9ca60da59865 100644
--- a/drivers/scsi/aic94xx/aic94xx_scb.c
+++ b/drivers/scsi/aic94xx/aic94xx_scb.c
@@ -81,7 +81,7 @@ static void asd_phy_event_tasklet(struct asd_ascb *ascb,
ASD_DPRINTK("phy%d: device unplugged\n", phy_id);
asd_turn_led(asd_ha, phy_id, 0);
sas_phy_disconnected(&phy->sas_phy);
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_LOSS_OF_SIGNAL, GFP_ATOMIC);
break;
case CURRENT_OOB_DONE:
/* hot plugged device */
@@ -89,12 +89,12 @@ static void asd_phy_event_tasklet(struct asd_ascb *ascb,
get_lrate_mode(phy, oob_mode);
ASD_DPRINTK("phy%d device plugged: lrate:0x%x, proto:0x%x\n",
phy_id, phy->sas_phy.linkrate, phy->sas_phy.iproto);
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_DONE, GFP_ATOMIC);
break;
case CURRENT_SPINUP_HOLD:
/* hot plug SATA, no COMWAKE sent */
asd_turn_led(asd_ha, phy_id, 1);
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_SPINUP_HOLD, GFP_ATOMIC);
break;
case CURRENT_GTO_TIMEOUT:
case CURRENT_OOB_ERROR:
@@ -102,7 +102,7 @@ static void asd_phy_event_tasklet(struct asd_ascb *ascb,
dl->status_block[1]);
asd_turn_led(asd_ha, phy_id, 0);
sas_phy_disconnected(&phy->sas_phy);
- sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_ERROR);
+ sas_ha->notify_phy_event_gfp(&phy->sas_phy, PHYE_OOB_ERROR, GFP_ATOMIC);
break;
}
}
@@ -234,7 +234,7 @@ static void asd_bytes_dmaed_tasklet(struct asd_ascb *ascb,
spin_unlock_irqrestore(&phy->sas_phy.frame_rcvd_lock, flags);
asd_dump_frame_rcvd(phy, dl);
asd_form_port(ascb->ha, phy);
- sas_ha->notify_port_event(&phy->sas_phy, PORTE_BYTES_DMAED);
+ sas_ha->notify_port_event_gfp(&phy->sas_phy, PORTE_BYTES_DMAED, GFP_ATOMIC);
}
static void asd_link_reset_err_tasklet(struct asd_ascb *ascb,
@@ -270,7 +270,7 @@ static void asd_link_reset_err_tasklet(struct asd_ascb *ascb,
asd_turn_led(asd_ha, phy_id, 0);
sas_phy_disconnected(sas_phy);
asd_deform_port(asd_ha, phy);
- sas_ha->notify_port_event(sas_phy, PORTE_LINK_RESET_ERR);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_LINK_RESET_ERR, GFP_ATOMIC);
if (retries_left == 0) {
int num = 1;
@@ -315,7 +315,7 @@ static void asd_primitive_rcvd_tasklet(struct asd_ascb *ascb,
spin_lock_irqsave(&sas_phy->sas_prim_lock, flags);
sas_phy->sas_prim = ffs(cont);
spin_unlock_irqrestore(&sas_phy->sas_prim_lock, flags);
- sas_ha->notify_port_event(sas_phy,PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy,PORTE_BROADCAST_RCVD, GFP_ATOMIC);
break;
case LmUNKNOWNP:
@@ -336,7 +336,7 @@ static void asd_primitive_rcvd_tasklet(struct asd_ascb *ascb,
/* The sequencer disables all phys on that port.
* We have to re-enable the phys ourselves. */
asd_deform_port(asd_ha, phy);
- sas_ha->notify_port_event(sas_phy, PORTE_HARD_RESET);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_HARD_RESET, GFP_ATOMIC);
break;
default:
@@ -567,7 +567,7 @@ static void escb_tasklet_complete(struct asd_ascb *ascb,
/* the device is gone */
sas_phy_disconnected(sas_phy);
asd_deform_port(asd_ha, phy);
- sas_ha->notify_port_event(sas_phy, PORTE_TIMER_EVENT);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_TIMER_EVENT, GFP_ATOMIC);
break;
default:
ASD_DPRINTK("%s: phy%d: unknown event:0x%x\n", __func__,
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/11] scsi: hisi_sas: Pass gfp_t flags to libsas event notifiers
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (8 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 09/11] scsi: aic94xx: " Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-18 20:43 ` [PATCH 11/11] scsi: libsas: event notifiers: Remove non _gfp() variants Ahmed S. Darwish
2020-12-21 10:13 ` [PATCH 00/11] scsi: libsas: Remove in_interrupt() check John Garry
11 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
Use the new libsas event notifiers API, which requires callers to
explicitly pass the gfp_t memory allocation flags.
Below are the context analysis for modified functions:
=> hisi_sas_bytes_dmaed():
Since it is invoked from both process and atomic contexts, let its
callers pass the gfp_t flags:
* hisi_sas_main.c:
------------------
hisi_sas_phyup_work(): workqueue context
-> hisi_sas_bytes_dmaed(..., GFP_KERNEL)
hisi_sas_controller_reset_done(): has an msleep()
-> hisi_sas_rescan_topology()
-> hisi_sas_phy_down()
-> hisi_sas_bytes_dmaed(..., GFP_KERNEL)
hisi_sas_debug_I_T_nexus_reset(): calls wait_for_completion_timeout()
-> hisi_sas_phy_down()
-> hisi_sas_bytes_dmaed(..., GFP_KERNEL)
* hisi_sas_v1_hw.c:
-------------------
int_abnormal_v1_hw(): irq handler
-> hisi_sas_phy_down()
-> hisi_sas_bytes_dmaed(..., GFP_ATOMIC)
* hisi_sas_v[23]_hw.c:
----------------------
int_phy_updown_v[23]_hw(): irq handler
-> phy_down_v[23]_hw()
-> hisi_sas_phy_down()
-> hisi_sas_bytes_dmaed(..., GFP_ATOMIC)
=> int_bcast_v1_hw() and phy_bcast_v3_hw():
Both are invoked exclusively from irq handlers. Pass GFP_ATOMIC.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: John Garry <john.garry@huawei.com>
---
drivers/scsi/hisi_sas/hisi_sas.h | 3 ++-
drivers/scsi/hisi_sas/hisi_sas_main.c | 26 +++++++++++++++-----------
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 5 +++--
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 5 +++--
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 5 +++--
5 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index a25cfc11c96d..e08c71bf607d 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -658,7 +658,8 @@ extern void hisi_sas_scan_start(struct Scsi_Host *shost);
extern int hisi_sas_host_reset(struct Scsi_Host *shost, int reset_type);
extern void hisi_sas_phy_enable(struct hisi_hba *hisi_hba, int phy_no,
int enable);
-extern void hisi_sas_phy_down(struct hisi_hba *hisi_hba, int phy_no, int rdy);
+extern void hisi_sas_phy_down(struct hisi_hba *hisi_hba, int phy_no, int rdy,
+ gfp_t gfp_flags);
extern void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba,
struct sas_task *task,
struct hisi_sas_slot *slot);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 274ccf18ce2d..f9332f62739b 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -618,7 +618,8 @@ static int hisi_sas_task_exec(struct sas_task *task, gfp_t gfp_flags,
return rc;
}
-static void hisi_sas_bytes_dmaed(struct hisi_hba *hisi_hba, int phy_no)
+static void hisi_sas_bytes_dmaed(struct hisi_hba *hisi_hba, int phy_no,
+ gfp_t gfp_flags)
{
struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
struct asd_sas_phy *sas_phy = &phy->sas_phy;
@@ -634,7 +635,7 @@ static void hisi_sas_bytes_dmaed(struct hisi_hba *hisi_hba, int phy_no)
}
sas_ha = &hisi_hba->sha;
- sas_ha->notify_phy_event(sas_phy, PHYE_OOB_DONE);
+ sas_ha->notify_phy_event_gfp(sas_phy, PHYE_OOB_DONE, gfp_flags);
if (sas_phy->phy) {
struct sas_phy *sphy = sas_phy->phy;
@@ -662,7 +663,7 @@ static void hisi_sas_bytes_dmaed(struct hisi_hba *hisi_hba, int phy_no)
}
sas_phy->frame_rcvd_size = phy->frame_rcvd_size;
- sas_ha->notify_port_event(sas_phy, PORTE_BYTES_DMAED);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BYTES_DMAED, gfp_flags);
}
static struct hisi_sas_device *hisi_sas_alloc_dev(struct domain_device *device)
@@ -868,7 +869,7 @@ static void hisi_sas_phyup_work(struct work_struct *work)
if (phy->identify.target_port_protocols == SAS_PROTOCOL_SSP)
hisi_hba->hw->sl_notify_ssp(hisi_hba, phy_no);
- hisi_sas_bytes_dmaed(hisi_hba, phy_no);
+ hisi_sas_bytes_dmaed(hisi_hba, phy_no, GFP_KERNEL);
}
static void hisi_sas_linkreset_work(struct work_struct *work)
@@ -1438,11 +1439,12 @@ static void hisi_sas_rescan_topology(struct hisi_hba *hisi_hba, u32 state)
_sas_port = sas_port;
if (dev_is_expander(dev->dev_type))
- sas_ha->notify_port_event(sas_phy,
- PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy,
+ PORTE_BROADCAST_RCVD,
+ GFP_KERNEL);
}
} else {
- hisi_sas_phy_down(hisi_hba, phy_no, 0);
+ hisi_sas_phy_down(hisi_hba, phy_no, 0, GFP_KERNEL);
}
}
}
@@ -1796,7 +1798,7 @@ static int hisi_sas_debug_I_T_nexus_reset(struct domain_device *device)
/* report PHY down if timed out */
if (!ret)
- hisi_sas_phy_down(hisi_hba, sas_phy->id, 0);
+ hisi_sas_phy_down(hisi_hba, sas_phy->id, 0, GFP_KERNEL);
} else if (sas_dev->dev_status != HISI_SAS_DEV_INIT) {
/*
* If in init state, we rely on caller to wait for link to be
@@ -2196,7 +2198,8 @@ static void hisi_sas_phy_disconnected(struct hisi_sas_phy *phy)
spin_unlock_irqrestore(&phy->lock, flags);
}
-void hisi_sas_phy_down(struct hisi_hba *hisi_hba, int phy_no, int rdy)
+void hisi_sas_phy_down(struct hisi_hba *hisi_hba, int phy_no, int rdy,
+ gfp_t gfp_flags)
{
struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
struct asd_sas_phy *sas_phy = &phy->sas_phy;
@@ -2205,7 +2208,7 @@ void hisi_sas_phy_down(struct hisi_hba *hisi_hba, int phy_no, int rdy)
if (rdy) {
/* Phy down but ready */
- hisi_sas_bytes_dmaed(hisi_hba, phy_no);
+ hisi_sas_bytes_dmaed(hisi_hba, phy_no, gfp_flags);
hisi_sas_port_notify_formed(sas_phy);
} else {
struct hisi_sas_port *port = phy->port;
@@ -2216,7 +2219,8 @@ void hisi_sas_phy_down(struct hisi_hba *hisi_hba, int phy_no, int rdy)
return;
}
/* Phy down and not ready */
- sas_ha->notify_phy_event(sas_phy, PHYE_LOSS_OF_SIGNAL);
+ sas_ha->notify_phy_event_gfp(sas_phy, PHYE_LOSS_OF_SIGNAL,
+ gfp_flags);
sas_phy_disconnected(sas_phy);
if (port) {
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 45e866cb9164..56b2ca5544e1 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1424,7 +1424,7 @@ static irqreturn_t int_bcast_v1_hw(int irq, void *p)
}
if (!test_bit(HISI_SAS_RESET_BIT, &hisi_hba->flags))
- sha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
end:
hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2,
@@ -1453,7 +1453,8 @@ static irqreturn_t int_abnormal_v1_hw(int irq, void *p)
u32 phy_state = hisi_sas_read32(hisi_hba, PHY_STATE);
hisi_sas_phy_down(hisi_hba, phy_no,
- (phy_state & 1 << phy_no) ? 1 : 0);
+ (phy_state & 1 << phy_no) ? 1 : 0,
+ GFP_ATOMIC);
}
if (irq_value & CHL_INT0_ID_TIMEOUT_MSK)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index b57177b52fac..a8cc89abcc6b 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -2734,7 +2734,8 @@ static int phy_down_v2_hw(int phy_no, struct hisi_hba *hisi_hba)
phy_state = hisi_sas_read32(hisi_hba, PHY_STATE);
dev_info(dev, "phydown: phy%d phy_state=0x%x\n", phy_no, phy_state);
- hisi_sas_phy_down(hisi_hba, phy_no, (phy_state & 1 << phy_no) ? 1 : 0);
+ hisi_sas_phy_down(hisi_hba, phy_no, (phy_state & 1 << phy_no) ? 1 : 0,
+ GFP_ATOMIC);
sl_ctrl = hisi_sas_phy_read32(hisi_hba, phy_no, SL_CONTROL);
hisi_sas_phy_write32(hisi_hba, phy_no, SL_CONTROL,
@@ -2825,7 +2826,7 @@ static void phy_bcast_v2_hw(int phy_no, struct hisi_hba *hisi_hba)
bcast_status = hisi_sas_phy_read32(hisi_hba, phy_no, RX_PRIMS_STATUS);
if ((bcast_status & RX_BCAST_CHG_MSK) &&
!test_bit(HISI_SAS_RESET_BIT, &hisi_hba->flags))
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT0,
CHL_INT0_SL_RX_BCST_ACK_MSK);
hisi_sas_phy_write32(hisi_hba, phy_no, SL_RX_BCAST_CHK_MSK, 0);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 960de375ce69..efba79252ddc 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -1578,7 +1578,8 @@ static irqreturn_t phy_down_v3_hw(int phy_no, struct hisi_hba *hisi_hba)
phy_state = hisi_sas_read32(hisi_hba, PHY_STATE);
dev_info(dev, "phydown: phy%d phy_state=0x%x\n", phy_no, phy_state);
- hisi_sas_phy_down(hisi_hba, phy_no, (phy_state & 1 << phy_no) ? 1 : 0);
+ hisi_sas_phy_down(hisi_hba, phy_no, (phy_state & 1 << phy_no) ? 1 : 0,
+ GFP_ATOMIC);
sl_ctrl = hisi_sas_phy_read32(hisi_hba, phy_no, SL_CONTROL);
hisi_sas_phy_write32(hisi_hba, phy_no, SL_CONTROL,
@@ -1605,7 +1606,7 @@ static irqreturn_t phy_bcast_v3_hw(int phy_no, struct hisi_hba *hisi_hba)
bcast_status = hisi_sas_phy_read32(hisi_hba, phy_no, RX_PRIMS_STATUS);
if ((bcast_status & RX_BCAST_CHG_MSK) &&
!test_bit(HISI_SAS_RESET_BIT, &hisi_hba->flags))
- sas_ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+ sas_ha->notify_port_event_gfp(sas_phy, PORTE_BROADCAST_RCVD, GFP_ATOMIC);
hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT0,
CHL_INT0_SL_RX_BCST_ACK_MSK);
hisi_sas_phy_write32(hisi_hba, phy_no, SL_RX_BCAST_CHK_MSK, 0);
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 11/11] scsi: libsas: event notifiers: Remove non _gfp() variants
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (9 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 10/11] scsi: hisi_sas: " Ahmed S. Darwish
@ 2020-12-18 20:43 ` Ahmed S. Darwish
2020-12-21 17:17 ` John Garry
2020-12-21 10:13 ` [PATCH 00/11] scsi: libsas: Remove in_interrupt() check John Garry
11 siblings, 1 reply; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-18 20:43 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, John Garry, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior,
Ahmed S. Darwish
All call-sites of below libsas APIs:
- sas_alloc_event()
- sas_ha_struct::notify_port_event()
- sas_ha_struct::notify_phy_event()
have been converted to use the new _gfp()-suffixed version.
Remove the old APIs from libsas code, headers, and documentation.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: John Garry <john.garry@huawei.com>
Cc: Jason Yan <yanaijie@huawei.com>
---
Documentation/scsi/libsas.rst | 2 -
drivers/scsi/libsas/sas_event.c | 72 +++++++-----------------------
drivers/scsi/libsas/sas_init.c | 15 +------
drivers/scsi/libsas/sas_internal.h | 2 -
include/scsi/libsas.h | 2 -
5 files changed, 18 insertions(+), 75 deletions(-)
diff --git a/Documentation/scsi/libsas.rst b/Documentation/scsi/libsas.rst
index dc85d0e4c107..7e1bf710760b 100644
--- a/Documentation/scsi/libsas.rst
+++ b/Documentation/scsi/libsas.rst
@@ -189,8 +189,6 @@ num_phys
The event interface::
/* LLDD calls these to notify the class of an event. */
- void (*notify_port_event)(struct sas_phy *, enum port_event);
- void (*notify_phy_event)(struct sas_phy *, enum phy_event);
void (*notify_port_event_gfp)(struct sas_phy *, enum port_event, gfp_t);
void (*notify_phy_event_gfp)(struct sas_phy *, enum phy_event, gfp_t);
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index 31b733eeabf6..23aeb67f6381 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -131,57 +131,21 @@ static void sas_phy_event_worker(struct work_struct *work)
sas_free_event(ev);
}
-static int __sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event,
- struct asd_sas_event *ev)
-{
- struct sas_ha_struct *ha = phy->ha;
- int ret;
-
- BUG_ON(event >= PORT_NUM_EVENTS);
-
- INIT_SAS_EVENT(ev, sas_port_event_worker, phy, event);
-
- ret = sas_queue_event(event, &ev->work, ha);
- if (ret != 1)
- sas_free_event(ev);
-
- return ret;
-}
-
static int sas_notify_port_event_gfp(struct asd_sas_phy *phy,
enum port_event event,
gfp_t gfp_flags)
-{
- struct asd_sas_event *ev;
-
- ev = sas_alloc_event_gfp(phy, gfp_flags);
- if (!ev)
- return -ENOMEM;
-
- return __sas_notify_port_event(phy, event, ev);
-}
-
-static int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event)
-{
- struct asd_sas_event *ev;
-
- ev = sas_alloc_event(phy);
- if (!ev)
- return -ENOMEM;
-
- return __sas_notify_port_event(phy, event, ev);
-}
-
-static inline int __sas_notify_phy_event(struct asd_sas_phy *phy,
- enum phy_event event,
- struct asd_sas_event *ev)
{
struct sas_ha_struct *ha = phy->ha;
+ struct asd_sas_event *ev;
int ret;
- BUG_ON(event >= PHY_NUM_EVENTS);
+ BUG_ON(event >= PORT_NUM_EVENTS);
- INIT_SAS_EVENT(ev, sas_phy_event_worker, phy, event);
+ ev = sas_alloc_event_gfp(phy, gfp_flags);
+ if (!ev)
+ return -ENOMEM;
+
+ INIT_SAS_EVENT(ev, sas_port_event_worker, phy, event);
ret = sas_queue_event(event, &ev->work, ha);
if (ret != 1)
@@ -193,31 +157,27 @@ static inline int __sas_notify_phy_event(struct asd_sas_phy *phy,
int sas_notify_phy_event_gfp(struct asd_sas_phy *phy, enum phy_event event,
gfp_t gfp_flags)
{
+ struct sas_ha_struct *ha = phy->ha;
struct asd_sas_event *ev;
+ int ret;
+
+ BUG_ON(event >= PHY_NUM_EVENTS);
ev = sas_alloc_event_gfp(phy, gfp_flags);
if (!ev)
return -ENOMEM;
- return __sas_notify_phy_event(phy, event, ev);
-}
+ INIT_SAS_EVENT(ev, sas_phy_event_worker, phy, event);
-int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
-{
- struct asd_sas_event *ev;
+ ret = sas_queue_event(event, &ev->work, ha);
+ if (ret != 1)
+ sas_free_event(ev);
- ev = sas_alloc_event(phy);
- if (!ev)
- return -ENOMEM;
-
- return __sas_notify_phy_event(phy, event, ev);
+ return ret;
}
int sas_init_events(struct sas_ha_struct *sas_ha)
{
- sas_ha->notify_port_event = sas_notify_port_event;
- sas_ha->notify_phy_event = sas_notify_phy_event;
-
sas_ha->notify_port_event_gfp = sas_notify_port_event_gfp;
sas_ha->notify_phy_event_gfp = sas_notify_phy_event_gfp;
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 2d2116f827c6..e08351f909fb 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -590,8 +590,8 @@ sas_domain_attach_transport(struct sas_domain_function_template *dft)
}
EXPORT_SYMBOL_GPL(sas_domain_attach_transport);
-static struct asd_sas_event * __sas_alloc_event(struct asd_sas_phy *phy,
- gfp_t gfp_flags)
+struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy,
+ gfp_t gfp_flags)
{
struct asd_sas_event *event;
struct sas_ha_struct *sas_ha = phy->ha;
@@ -623,17 +623,6 @@ static struct asd_sas_event * __sas_alloc_event(struct asd_sas_phy *phy,
return event;
}
-struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
-{
- return __sas_alloc_event(phy, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
-}
-
-struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy,
- gfp_t gfp_flags)
-{
- return __sas_alloc_event(phy, gfp_flags);
-}
-
void sas_free_event(struct asd_sas_event *event)
{
struct asd_sas_phy *phy = event->phy;
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 437a697b6f73..b0422d47675b 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -48,7 +48,6 @@ int sas_show_oob_mode(enum sas_oob_mode oob_mode, char *buf);
int sas_register_phys(struct sas_ha_struct *sas_ha);
void sas_unregister_phys(struct sas_ha_struct *sas_ha);
-struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy);
struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy, gfp_t gfp_flags);
void sas_free_event(struct asd_sas_event *event);
@@ -78,7 +77,6 @@ int sas_smp_phy_control(struct domain_device *dev, int phy_id,
enum phy_func phy_func, struct sas_phy_linkrates *);
int sas_smp_get_phy_events(struct sas_phy *phy);
-int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event);
int sas_notify_phy_event_gfp(struct asd_sas_phy *phy, enum phy_event event, gfp_t flags);
void sas_device_set_phy(struct domain_device *dev, struct sas_port *port);
struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index f7c2530bbd9d..fdd338fa65c9 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -392,8 +392,6 @@ struct sas_ha_struct {
* their siblings when forming wide ports */
/* LLDD calls these to notify the class of an event. */
- int (*notify_port_event)(struct asd_sas_phy *, enum port_event);
- int (*notify_phy_event)(struct asd_sas_phy *, enum phy_event);
int (*notify_port_event_gfp)(struct asd_sas_phy *, enum port_event, gfp_t);
int (*notify_phy_event_gfp)(struct asd_sas_phy *, enum phy_event, gfp_t);
--
2.29.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 11/11] scsi: libsas: event notifiers: Remove non _gfp() variants
2020-12-18 20:43 ` [PATCH 11/11] scsi: libsas: event notifiers: Remove non _gfp() variants Ahmed S. Darwish
@ 2020-12-21 17:17 ` John Garry
2020-12-21 17:38 ` Ahmed S. Darwish
0 siblings, 1 reply; 21+ messages in thread
From: John Garry @ 2020-12-21 17:17 UTC (permalink / raw)
To: Ahmed S. Darwish, James E.J. Bottomley, Martin K. Petersen,
Daniel Wagner, Jason Yan, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior
On 18/12/2020 20:43, Ahmed S. Darwish wrote:
> All call-sites of below libsas APIs:
>
> - sas_alloc_event()
> - sas_ha_struct::notify_port_event()
> - sas_ha_struct::notify_phy_event()
>
> have been converted to use the new _gfp()-suffixed version.
>
nit: Is it possible to have non- _gfp()-suffixed symbols at the end,
i.e. have same as original?
Thanks,
John
> Remove the old APIs from libsas code, headers, and documentation.
>
> Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
> Cc: John Garry <john.garry@huawei.com>
> Cc: Jason Yan <yanaijie@huawei.com>
> ---
> Documentation/scsi/libsas.rst | 2 -
> drivers/scsi/libsas/sas_event.c | 72 +++++++-----------------------
> drivers/scsi/libsas/sas_init.c | 15 +------
> drivers/scsi/libsas/sas_internal.h | 2 -
> include/scsi/libsas.h | 2 -
> 5 files changed, 18 insertions(+), 75 deletions(-)
>
> diff --git a/Documentation/scsi/libsas.rst b/Documentation/scsi/libsas.rst
> index dc85d0e4c107..7e1bf710760b 100644
> --- a/Documentation/scsi/libsas.rst
> +++ b/Documentation/scsi/libsas.rst
> @@ -189,8 +189,6 @@ num_phys
> The event interface::
>
> /* LLDD calls these to notify the class of an event. */
> - void (*notify_port_event)(struct sas_phy *, enum port_event);
> - void (*notify_phy_event)(struct sas_phy *, enum phy_event);
> void (*notify_port_event_gfp)(struct sas_phy *, enum port_event, gfp_t);
> void (*notify_phy_event_gfp)(struct sas_phy *, enum phy_event, gfp_t);
>
> diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
> index 31b733eeabf6..23aeb67f6381 100644
> --- a/drivers/scsi/libsas/sas_event.c
> +++ b/drivers/scsi/libsas/sas_event.c
> @@ -131,57 +131,21 @@ static void sas_phy_event_worker(struct work_struct *work)
> sas_free_event(ev);
> }
>
> -static int __sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event,
> - struct asd_sas_event *ev)
> -{
> - struct sas_ha_struct *ha = phy->ha;
> - int ret;
> -
> - BUG_ON(event >= PORT_NUM_EVENTS);
> -
> - INIT_SAS_EVENT(ev, sas_port_event_worker, phy, event);
> -
> - ret = sas_queue_event(event, &ev->work, ha);
> - if (ret != 1)
> - sas_free_event(ev);
> -
> - return ret;
> -}
> -
> static int sas_notify_port_event_gfp(struct asd_sas_phy *phy,
> enum port_event event,
> gfp_t gfp_flags)
> -{
> - struct asd_sas_event *ev;
> -
> - ev = sas_alloc_event_gfp(phy, gfp_flags);
> - if (!ev)
> - return -ENOMEM;
> -
> - return __sas_notify_port_event(phy, event, ev);
> -}
> -
> -static int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event)
> -{
> - struct asd_sas_event *ev;
> -
> - ev = sas_alloc_event(phy);
> - if (!ev)
> - return -ENOMEM;
> -
> - return __sas_notify_port_event(phy, event, ev);
> -}
> -
> -static inline int __sas_notify_phy_event(struct asd_sas_phy *phy,
> - enum phy_event event,
> - struct asd_sas_event *ev)
> {
> struct sas_ha_struct *ha = phy->ha;
> + struct asd_sas_event *ev;
> int ret;
>
> - BUG_ON(event >= PHY_NUM_EVENTS);
> + BUG_ON(event >= PORT_NUM_EVENTS);
>
> - INIT_SAS_EVENT(ev, sas_phy_event_worker, phy, event);
> + ev = sas_alloc_event_gfp(phy, gfp_flags);
> + if (!ev)
> + return -ENOMEM;
> +
> + INIT_SAS_EVENT(ev, sas_port_event_worker, phy, event);
>
> ret = sas_queue_event(event, &ev->work, ha);
> if (ret != 1)
> @@ -193,31 +157,27 @@ static inline int __sas_notify_phy_event(struct asd_sas_phy *phy,
> int sas_notify_phy_event_gfp(struct asd_sas_phy *phy, enum phy_event event,
> gfp_t gfp_flags)
> {
> + struct sas_ha_struct *ha = phy->ha;
> struct asd_sas_event *ev;
> + int ret;
> +
> + BUG_ON(event >= PHY_NUM_EVENTS);
>
> ev = sas_alloc_event_gfp(phy, gfp_flags);
> if (!ev)
> return -ENOMEM;
>
> - return __sas_notify_phy_event(phy, event, ev);
> -}
> + INIT_SAS_EVENT(ev, sas_phy_event_worker, phy, event);
>
> -int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
> -{
> - struct asd_sas_event *ev;
> + ret = sas_queue_event(event, &ev->work, ha);
> + if (ret != 1)
> + sas_free_event(ev);
>
> - ev = sas_alloc_event(phy);
> - if (!ev)
> - return -ENOMEM;
> -
> - return __sas_notify_phy_event(phy, event, ev);
> + return ret;
> }
>
> int sas_init_events(struct sas_ha_struct *sas_ha)
> {
> - sas_ha->notify_port_event = sas_notify_port_event;
> - sas_ha->notify_phy_event = sas_notify_phy_event;
> -
> sas_ha->notify_port_event_gfp = sas_notify_port_event_gfp;
> sas_ha->notify_phy_event_gfp = sas_notify_phy_event_gfp;
>
> diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
> index 2d2116f827c6..e08351f909fb 100644
> --- a/drivers/scsi/libsas/sas_init.c
> +++ b/drivers/scsi/libsas/sas_init.c
> @@ -590,8 +590,8 @@ sas_domain_attach_transport(struct sas_domain_function_template *dft)
> }
> EXPORT_SYMBOL_GPL(sas_domain_attach_transport);
>
> -static struct asd_sas_event * __sas_alloc_event(struct asd_sas_phy *phy,
> - gfp_t gfp_flags)
> +struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy,
> + gfp_t gfp_flags)
> {
> struct asd_sas_event *event;
> struct sas_ha_struct *sas_ha = phy->ha;
> @@ -623,17 +623,6 @@ static struct asd_sas_event * __sas_alloc_event(struct asd_sas_phy *phy,
> return event;
> }
>
> -struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
> -{
> - return __sas_alloc_event(phy, in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
> -}
> -
> -struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy,
> - gfp_t gfp_flags)
> -{
> - return __sas_alloc_event(phy, gfp_flags);
> -}
> -
> void sas_free_event(struct asd_sas_event *event)
> {
> struct asd_sas_phy *phy = event->phy;
> diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
> index 437a697b6f73..b0422d47675b 100644
> --- a/drivers/scsi/libsas/sas_internal.h
> +++ b/drivers/scsi/libsas/sas_internal.h
> @@ -48,7 +48,6 @@ int sas_show_oob_mode(enum sas_oob_mode oob_mode, char *buf);
> int sas_register_phys(struct sas_ha_struct *sas_ha);
> void sas_unregister_phys(struct sas_ha_struct *sas_ha);
>
> -struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy);
> struct asd_sas_event *sas_alloc_event_gfp(struct asd_sas_phy *phy, gfp_t gfp_flags);
> void sas_free_event(struct asd_sas_event *event);
>
> @@ -78,7 +77,6 @@ int sas_smp_phy_control(struct domain_device *dev, int phy_id,
> enum phy_func phy_func, struct sas_phy_linkrates *);
> int sas_smp_get_phy_events(struct sas_phy *phy);
>
> -int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event);
> int sas_notify_phy_event_gfp(struct asd_sas_phy *phy, enum phy_event event, gfp_t flags);
> void sas_device_set_phy(struct domain_device *dev, struct sas_port *port);
> struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy);
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index f7c2530bbd9d..fdd338fa65c9 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -392,8 +392,6 @@ struct sas_ha_struct {
> * their siblings when forming wide ports */
>
> /* LLDD calls these to notify the class of an event. */
> - int (*notify_port_event)(struct asd_sas_phy *, enum port_event);
> - int (*notify_phy_event)(struct asd_sas_phy *, enum phy_event);
> int (*notify_port_event_gfp)(struct asd_sas_phy *, enum port_event, gfp_t);
> int (*notify_phy_event_gfp)(struct asd_sas_phy *, enum phy_event, gfp_t);
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 11/11] scsi: libsas: event notifiers: Remove non _gfp() variants
2020-12-21 17:17 ` John Garry
@ 2020-12-21 17:38 ` Ahmed S. Darwish
0 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2020-12-21 17:38 UTC (permalink / raw)
To: John Garry
Cc: James E.J. Bottomley, Martin K. Petersen, Daniel Wagner,
Jason Yan, Artur Paszkiewicz, Jack Wang, linux-scsi, LKML,
Thomas Gleixner, Sebastian A. Siewior
On Mon, Dec 21, 2020 at 05:17:13PM +0000, John Garry wrote:
> On 18/12/2020 20:43, Ahmed S. Darwish wrote:
> > All call-sites of below libsas APIs:
> >
> > - sas_alloc_event()
> > - sas_ha_struct::notify_port_event()
> > - sas_ha_struct::notify_phy_event()
> >
> > have been converted to use the new _gfp()-suffixed version.
> >
>
> nit: Is it possible to have non- _gfp()-suffixed symbols at the end, i.e.
> have same as original?
>
Yes, of course. I just did not want to double-fold the patch series size
from first submission ;-)
If the overall outlook of this series is OK, in v2 I'll append patches
#12 => #20 restoring call sites to the original names without _gfp(),
then keep only the original libsas names.
Thanks,
--
Ahmed S. Darwish
Linutronix GmbH
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/11] scsi: libsas: Remove in_interrupt() check
2020-12-18 20:43 [PATCH 00/11] scsi: libsas: Remove in_interrupt() check Ahmed S. Darwish
` (10 preceding siblings ...)
2020-12-18 20:43 ` [PATCH 11/11] scsi: libsas: event notifiers: Remove non _gfp() variants Ahmed S. Darwish
@ 2020-12-21 10:13 ` John Garry
2020-12-22 12:30 ` Jason Yan
11 siblings, 1 reply; 21+ messages in thread
From: John Garry @ 2020-12-21 10:13 UTC (permalink / raw)
To: Ahmed S. Darwish, James E.J. Bottomley, Martin K. Petersen,
Daniel Wagner, Jason Yan, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior, Hannes Reinecke
On 18/12/2020 20:43, Ahmed S. Darwish wrote:
> Folks,
>
> In the discussion about preempt count consistency across kernel
> configurations:
>
> https://lkml.kernel.org/r/20200914204209.256266093@linutronix.de
>
> it was concluded that the usage of in_interrupt() and related context
> checks should be removed from non-core code.
>
> This includes memory allocation mode decisions (GFP_*). In the long run,
> usage of in_interrupt() and its siblings should be banned from driver
> code completely.
>
> This series addresses SCSI libsas. Basically, the function:
>
> => drivers/scsi/libsas/sas_init.c:
> struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
> {
> ...
> gfp_t flags = in_interrupt() ? GFP_ATOMIC : GFP_KERNEL;
> event = kmem_cache_zalloc(sas_event_cache, flags);
Hi Ahmed,
Firstly I would say that it would be nice to just remove all the atomic
context calls. But that may require significant LLDD rework and
participation from driver stakeholders.
However, considering function sas_alloc_event() again:
gfp_t flags = in_interrupt() ? GFP_ATOMIC : GFP_KERNEL;
...
event = kmem_cache_zalloc(sas_event_cache, flags);
if (!event)
return NULL;
atomic_inc(&phy->event_nr);
if (atomic_read(&phy->event_nr) > phy->ha->event_thres) {
/* Code to shutdown the phy */
}
return event;
So default for phy->ha->event_thres is 32, and I can't imagine that
anyone has ever reconfigured this via sysfs or even required a value
that large. Maybe Jason (cc'ed) knows better. It's an arbitrary value to
say that the PHY is malfunctioning. I do note that there is the circular
path sas_alloc_event() -> sas_notify_phy_event() -> sas_alloc_event()
there also.
Anyway, if the 32x event memories were per-allocated, maybe there is a
clean method to manage this memory, which even works in atomic context,
so we could avoid this rework (ignoring the context bugs you reported
for a moment). I do also note that the sas_event_cache size is not huge.
Anyway, I'll look at the rest of the series.
Thanks,
John
> ...
> }
>
> is transformed so that callers explicitly pass the gfp_t memory
> allocation flags. Affected libsas clients are modified accordingly.
>
> The first six patches have "Fixes: " tags and address bugs the were
> noticed during the context analysis.
>
> Thanks!
>
> 8<--------------
>
> Ahmed S. Darwish (11):
> Documentation: scsi: libsas: Remove notify_ha_event()
> scsi: libsas: Introduce a _gfp() variant of event notifiers
> scsi: mvsas: Pass gfp_t flags to libsas event notifiers
> scsi: isci: port: link down: Pass gfp_t flags
> scsi: isci: port: link up: Pass gfp_t flags
> scsi: isci: port: broadcast change: Pass gfp_t flags
> scsi: libsas: Pass gfp_t flags to event notifiers
> scsi: pm80xx: Pass gfp_t flags to libsas event notifiers
> scsi: aic94xx: Pass gfp_t flags to libsas event notifiers
> scsi: hisi_sas: Pass gfp_t flags to libsas event notifiers
> scsi: libsas: event notifiers: Remove non _gfp() variants
>
> Documentation/scsi/libsas.rst | 5 ++--
> drivers/scsi/aic94xx/aic94xx_scb.c | 18 ++++++------
> drivers/scsi/hisi_sas/hisi_sas.h | 3 +-
> drivers/scsi/hisi_sas/hisi_sas_main.c | 26 ++++++++++--------
> drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 5 ++--
> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 5 ++--
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 5 ++--
> drivers/scsi/isci/port.c | 14 ++++++----
> drivers/scsi/libsas/sas_event.c | 21 ++++++++------
> drivers/scsi/libsas/sas_init.c | 11 ++++----
> drivers/scsi/libsas/sas_internal.h | 4 +--
> drivers/scsi/mvsas/mv_sas.c | 22 +++++++--------
> drivers/scsi/pm8001/pm8001_hwi.c | 38 +++++++++++++-------------
> drivers/scsi/pm8001/pm8001_sas.c | 8 +++---
> drivers/scsi/pm8001/pm80xx_hwi.c | 30 ++++++++++----------
> include/scsi/libsas.h | 4 +--
> 16 files changed, 116 insertions(+), 103 deletions(-)
>
> base-commit: 2c85ebc57b3e1817b6ce1a6b703928e113a90442
> --
> 2.29.2
> .
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 00/11] scsi: libsas: Remove in_interrupt() check
2020-12-21 10:13 ` [PATCH 00/11] scsi: libsas: Remove in_interrupt() check John Garry
@ 2020-12-22 12:30 ` Jason Yan
2020-12-22 12:54 ` John Garry
0 siblings, 1 reply; 21+ messages in thread
From: Jason Yan @ 2020-12-22 12:30 UTC (permalink / raw)
To: John Garry, Ahmed S. Darwish, James E.J. Bottomley,
Martin K. Petersen, Daniel Wagner, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior, Hannes Reinecke
在 2020/12/21 18:13, John Garry 写道:
> On 18/12/2020 20:43, Ahmed S. Darwish wrote:
>> Folks,
>>
>> In the discussion about preempt count consistency across kernel
>> configurations:
>>
>> https://lkml.kernel.org/r/20200914204209.256266093@linutronix.de
>>
>> it was concluded that the usage of in_interrupt() and related context
>> checks should be removed from non-core code.
>>
>> This includes memory allocation mode decisions (GFP_*). In the long run,
>> usage of in_interrupt() and its siblings should be banned from driver
>> code completely.
>>
>> This series addresses SCSI libsas. Basically, the function:
>>
>> => drivers/scsi/libsas/sas_init.c:
>> struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy)
>> {
>> ...
>> gfp_t flags = in_interrupt() ? GFP_ATOMIC : GFP_KERNEL;
>> event = kmem_cache_zalloc(sas_event_cache, flags);
>
> Hi Ahmed,
>
> Firstly I would say that it would be nice to just remove all the atomic
> context calls. But that may require significant LLDD rework and
> participation from driver stakeholders.
>
> However, considering function sas_alloc_event() again:
>
> gfp_t flags = in_interrupt() ? GFP_ATOMIC : GFP_KERNEL;
>
> ...
>
> event = kmem_cache_zalloc(sas_event_cache, flags);
> if (!event)
> return NULL;
>
> atomic_inc(&phy->event_nr);
>
> if (atomic_read(&phy->event_nr) > phy->ha->event_thres) {
> /* Code to shutdown the phy */
> }
>
> return event;
>
>
> So default for phy->ha->event_thres is 32, and I can't imagine that
The default value is 1024.
> anyone has ever reconfigured this via sysfs or even required a value
> that large. Maybe Jason (cc'ed) knows better. It's an arbitrary value to
> say that the PHY is malfunctioning. I do note that there is the circular
> path sas_alloc_event() -> sas_notify_phy_event() -> sas_alloc_event()
> there also.
>
> Anyway, if the 32x event memories were per-allocated, maybe there is a
> clean method to manage this memory, which even works in atomic context,
> so we could avoid this rework (ignoring the context bugs you reported
> for a moment). I do also note that the sas_event_cache size is not huge.
>
Pre-allocated memory is an option.(Which we have tried at the very
beginnig by Wang Yijing.)
Or directly use GFP_ATOMIC is maybe better than passing flags from lldds.
Thanks,
Jason
> Anyway, I'll look at the rest of the series.
>
> Thanks,
> John
>
>> ...
>> }
>>
>> is transformed so that callers explicitly pass the gfp_t memory
>> allocation flags. Affected libsas clients are modified accordingly.
>>
>> The first six patches have "Fixes: " tags and address bugs the were
>> noticed during the context analysis.
>>
>> Thanks!
>>
>> 8<--------------
>>
>> Ahmed S. Darwish (11):
>> Documentation: scsi: libsas: Remove notify_ha_event()
>> scsi: libsas: Introduce a _gfp() variant of event notifiers
>> scsi: mvsas: Pass gfp_t flags to libsas event notifiers
>> scsi: isci: port: link down: Pass gfp_t flags
>> scsi: isci: port: link up: Pass gfp_t flags
>> scsi: isci: port: broadcast change: Pass gfp_t flags
>> scsi: libsas: Pass gfp_t flags to event notifiers
>> scsi: pm80xx: Pass gfp_t flags to libsas event notifiers
>> scsi: aic94xx: Pass gfp_t flags to libsas event notifiers
>> scsi: hisi_sas: Pass gfp_t flags to libsas event notifiers
>> scsi: libsas: event notifiers: Remove non _gfp() variants
>>
>> Documentation/scsi/libsas.rst | 5 ++--
>> drivers/scsi/aic94xx/aic94xx_scb.c | 18 ++++++------
>> drivers/scsi/hisi_sas/hisi_sas.h | 3 +-
>> drivers/scsi/hisi_sas/hisi_sas_main.c | 26 ++++++++++--------
>> drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 5 ++--
>> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 5 ++--
>> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 5 ++--
>> drivers/scsi/isci/port.c | 14 ++++++----
>> drivers/scsi/libsas/sas_event.c | 21 ++++++++------
>> drivers/scsi/libsas/sas_init.c | 11 ++++----
>> drivers/scsi/libsas/sas_internal.h | 4 +--
>> drivers/scsi/mvsas/mv_sas.c | 22 +++++++--------
>> drivers/scsi/pm8001/pm8001_hwi.c | 38 +++++++++++++-------------
>> drivers/scsi/pm8001/pm8001_sas.c | 8 +++---
>> drivers/scsi/pm8001/pm80xx_hwi.c | 30 ++++++++++----------
>> include/scsi/libsas.h | 4 +--
>> 16 files changed, 116 insertions(+), 103 deletions(-)
>>
>> base-commit: 2c85ebc57b3e1817b6ce1a6b703928e113a90442
>> --
>> 2.29.2
>> .
>>
>
> .
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 00/11] scsi: libsas: Remove in_interrupt() check
2020-12-22 12:30 ` Jason Yan
@ 2020-12-22 12:54 ` John Garry
2021-01-11 13:43 ` Ahmed S. Darwish
0 siblings, 1 reply; 21+ messages in thread
From: John Garry @ 2020-12-22 12:54 UTC (permalink / raw)
To: Jason Yan, Ahmed S. Darwish, James E.J. Bottomley,
Martin K. Petersen, Daniel Wagner, Artur Paszkiewicz, Jack Wang
Cc: linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior, Hannes Reinecke
On 22/12/2020 12:30, Jason Yan wrote:
>> return event;
>>
>>
>> So default for phy->ha->event_thres is 32, and I can't imagine that
>
> The default value is 1024.
Ah, 32 is the minimum allowed set via sysfs.
>
>> anyone has ever reconfigured this via sysfs or even required a value
>> that large. Maybe Jason (cc'ed) knows better. It's an arbitrary value
>> to say that the PHY is malfunctioning. I do note that there is the
>> circular path sas_alloc_event() -> sas_notify_phy_event() ->
>> sas_alloc_event() there also.
>>
>> Anyway, if the 32x event memories were per-allocated, maybe there is a
>> clean method to manage this memory, which even works in atomic
>> context, so we could avoid this rework (ignoring the context bugs you
>> reported for a moment). I do also note that the sas_event_cache size
>> is not huge.
>>
>
> Pre-allocated memory is an option.(Which we have tried at the very
> beginnig by Wang Yijing.)
Right, I remember this, but I think the concern was having a proper
method to manage this pre-allocated memory then. And same problem now.
>
> Or directly use GFP_ATOMIC is maybe better than passing flags from lldds.
>
I think that if we don't really need this, then should not use it.
Thanks,
John
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/11] scsi: libsas: Remove in_interrupt() check
2020-12-22 12:54 ` John Garry
@ 2021-01-11 13:43 ` Ahmed S. Darwish
2021-01-11 13:59 ` John Garry
0 siblings, 1 reply; 21+ messages in thread
From: Ahmed S. Darwish @ 2021-01-11 13:43 UTC (permalink / raw)
To: John Garry, Jason Yan
Cc: Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen,
Daniel Wagner, Artur Paszkiewicz, Jack Wang, linux-scsi, LKML,
Thomas Gleixner, Sebastian A. Siewior
Hi John, Jason,
On Tue, Dec 22, 2020 at 12:54:58PM +0000, John Garry wrote:
> On 22/12/2020 12:30, Jason Yan wrote:
> > > return event;
> > >
> > >
> > > So default for phy->ha->event_thres is 32, and I can't imagine that
> >
> > The default value is 1024.
>
> Ah, 32 is the minimum allowed set via sysfs.
>
> >
> > > anyone has ever reconfigured this via sysfs or even required a value
> > > that large. Maybe Jason (cc'ed) knows better. It's an arbitrary
> > > value to say that the PHY is malfunctioning. I do note that there is
> > > the circular path sas_alloc_event() -> sas_notify_phy_event() ->
> > > sas_alloc_event() there also.
> > >
> > > Anyway, if the 32x event memories were per-allocated, maybe there is
> > > a clean method to manage this memory, which even works in atomic
> > > context, so we could avoid this rework (ignoring the context bugs
> > > you reported for a moment). I do also note that the sas_event_cache
> > > size is not huge.
> > >
> >
> > Pre-allocated memory is an option.(Which we have tried at the very
> > beginnig by Wang Yijing.)
>
> Right, I remember this, but I think the concern was having a proper method
> to manage this pre-allocated memory then. And same problem now.
>
> >
> > Or directly use GFP_ATOMIC is maybe better than passing flags from lldds.
> >
>
> I think that if we don't really need this, then should not use it.
>
Kind reminder. Do we have any consensus here?
Thanks,
--
Ahmed S. Darwish
Linutronix GmbH
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/11] scsi: libsas: Remove in_interrupt() check
2021-01-11 13:43 ` Ahmed S. Darwish
@ 2021-01-11 13:59 ` John Garry
2021-01-11 14:28 ` Ahmed S. Darwish
0 siblings, 1 reply; 21+ messages in thread
From: John Garry @ 2021-01-11 13:59 UTC (permalink / raw)
To: Ahmed S. Darwish, Jason Yan
Cc: Hannes Reinecke, James E.J. Bottomley, Martin K. Petersen,
Daniel Wagner, Artur Paszkiewicz, Jack Wang, linux-scsi, LKML,
Thomas Gleixner, Sebastian A. Siewior
On 11/01/2021 13:43, Ahmed S. Darwish wrote:
> Hi John, Jason,
>
> On Tue, Dec 22, 2020 at 12:54:58PM +0000, John Garry wrote:
>> On 22/12/2020 12:30, Jason Yan wrote:
>>>> return event;
>>>>
>>>>
>>>> So default for phy->ha->event_thres is 32, and I can't imagine that
>>> The default value is 1024.
>> Ah, 32 is the minimum allowed set via sysfs.
>>
>>>> anyone has ever reconfigured this via sysfs or even required a value
>>>> that large. Maybe Jason (cc'ed) knows better. It's an arbitrary
>>>> value to say that the PHY is malfunctioning. I do note that there is
>>>> the circular path sas_alloc_event() -> sas_notify_phy_event() ->
>>>> sas_alloc_event() there also.
>>>>
>>>> Anyway, if the 32x event memories were per-allocated, maybe there is
>>>> a clean method to manage this memory, which even works in atomic
>>>> context, so we could avoid this rework (ignoring the context bugs
>>>> you reported for a moment). I do also note that the sas_event_cache
>>>> size is not huge.
>>>>
>>> Pre-allocated memory is an option.(Which we have tried at the very
>>> beginnig by Wang Yijing.)
>> Right, I remember this, but I think the concern was having a proper method
>> to manage this pre-allocated memory then. And same problem now.
>>
>>> Or directly use GFP_ATOMIC is maybe better than passing flags from lldds.
>>>
>> I think that if we don't really need this, then should not use it.
>>
> Kind reminder. Do we have any consensus here?
>
Hi Ahmed,
To me, what you're doing seems fine.
I was looking for some API to manage small memory pools and which is
atomic safe to avoid passing the context flag, but I didn't find such a
thing.
Just one other thing to mention:
I have a patch to remove the indirection in libsas notifiers:
https://github.com/hisilicon/kernel-dev/commit/87fcd7e113dc05b7933260e7fa4588dc3730cc2a
I was going to send it today. Hopefully, if community has no problem
with it, you can make your changes with that in mind.
Thanks,
John
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/11] scsi: libsas: Remove in_interrupt() check
2021-01-11 13:59 ` John Garry
@ 2021-01-11 14:28 ` Ahmed S. Darwish
0 siblings, 0 replies; 21+ messages in thread
From: Ahmed S. Darwish @ 2021-01-11 14:28 UTC (permalink / raw)
To: John Garry
Cc: Jason Yan, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Wagner, Artur Paszkiewicz, Jack Wang,
linux-scsi, LKML, Thomas Gleixner, Sebastian A. Siewior
On Mon, Jan 11, 2021 at 01:59:25PM +0000, John Garry wrote:
...
> To me, what you're doing seems fine.
>
...
>
> Just one other thing to mention:
> I have a patch to remove the indirection in libsas notifiers:
> https://github.com/hisilicon/kernel-dev/commit/87fcd7e113dc05b7933260e7fa4588dc3730cc2a
>
> I was going to send it today. Hopefully, if community has no problem with
> it, you can make your changes with that in mind.
>
Perfect. I'll rebase on top of it if everything is OK there.
I'll also append some patches to the series, removing the _gfp() suffix,
per your request earlier:
https://lkml.kernel.org/r/68957d37-c789-0f0e-f5d1-85fef7f39f4f@huawei.com
Thanks!
--
Ahmed S. Darwish
Linutronix GmbH
^ permalink raw reply [flat|nested] 21+ messages in thread