在 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@xxxxxxxxxxxxx
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
.
.
