Re: [PATCH 00/11] scsi: libsas: Remove in_interrupt() check

From: Jason Yan
Date: Tue Dec 22 2020 - 07:32:05 EST



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


.