Skip to content

Commit

Permalink
scsi: libsas: Introduce a _gfp() variant of event notifiers
Browse files Browse the repository at this point in the history
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:

  mvsas/mv_sas.c: mvs_work_queue() [process context]
  spin_lock_irqsave(mvs_info::lock, )
    -> libsas/sas_event.c: sas_notify_phy_event()
      -> sas_alloc_event()
        -> in_interrupt() = false
          -> invalid GFP_KERNEL allocation
    -> libsas/sas_event.c: sas_notify_port_event()
      -> sas_alloc_event()
        -> in_interrupt() = false
          -> invalid GFP_KERNEL allocation

Introduce sas_alloc_event_gfp(), sas_notify_port_event_gfp(), and
sas_notify_phy_event_gfp(), which all behave like the non _gfp() variants
but use a caller-passed GFP mask for allocations.

For bisectability, all callers will be modified first to pass GFP context,
then the non _gfp() libsas API variants will be modified to take a gfp_t by
default.

Link: https://lore.kernel.org/r/[email protected]
Fixes: 1c393b9 ("scsi: libsas: Use dynamic alloced work to avoid sas event lost")
Cc: Jason Yan <[email protected]>
Reviewed-by: John Garry <[email protected]>
Signed-off-by: Ahmed S. Darwish <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
  • Loading branch information
a-darwish authored and martinkpetersen committed Jan 23, 2021
1 parent 121181f commit c2d0f1a
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 17 deletions.
2 changes: 2 additions & 0 deletions Documentation/scsi/libsas.rst
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ The event interface::
/* LLDD calls these to notify the class of an event. */
void sas_notify_port_event(struct sas_phy *, enum port_event);
void sas_notify_phy_event(struct sas_phy *, enum phy_event);
void sas_notify_port_event_gfp(struct sas_phy *, enum port_event, gfp_t);
void sas_notify_phy_event_gfp(struct sas_phy *, enum phy_event, gfp_t);

The port notification::

Expand Down
65 changes: 53 additions & 12 deletions drivers/scsi/libsas/sas_event.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,15 @@ static void sas_phy_event_worker(struct work_struct *work)
sas_free_event(ev);
}

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);
Expand All @@ -151,20 +148,41 @@ int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event)

return ret;
}
EXPORT_SYMBOL_GPL(sas_notify_port_event);

int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
int sas_notify_port_event_gfp(struct asd_sas_phy *phy, enum port_event event,
gfp_t gfp_flags)
{
struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
int ret;

BUG_ON(event >= PHY_NUM_EVENTS);
ev = sas_alloc_event_gfp(phy, gfp_flags);
if (!ev)
return -ENOMEM;

return __sas_notify_port_event(phy, event, ev);
}
EXPORT_SYMBOL_GPL(sas_notify_port_event_gfp);

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);
}
EXPORT_SYMBOL_GPL(sas_notify_port_event);

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);

INIT_SAS_EVENT(ev, sas_phy_event_worker, phy, event);

ret = sas_queue_event(event, &ev->work, ha);
Expand All @@ -173,5 +191,28 @@ int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)

return ret;
}
EXPORT_SYMBOL_GPL(sas_notify_phy_event);

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);
}
EXPORT_SYMBOL_GPL(sas_notify_phy_event_gfp);

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);
}
EXPORT_SYMBOL_GPL(sas_notify_phy_event);
21 changes: 16 additions & 5 deletions drivers/scsi/libsas/sas_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -584,16 +584,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;

Expand All @@ -604,7 +603,8 @@ 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 */
Expand All @@ -618,6 +618,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;
Expand Down
4 changes: 4 additions & 0 deletions drivers/scsi/libsas/sas_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ 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);
Expand Down Expand Up @@ -77,6 +79,8 @@ 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);
Expand Down
4 changes: 4 additions & 0 deletions include/scsi/libsas.h
Original file line number Diff line number Diff line change
Expand Up @@ -704,5 +704,9 @@ int sas_request_addr(struct Scsi_Host *shost, u8 *addr);

int sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event);
int sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event);
int sas_notify_port_event_gfp(struct asd_sas_phy *phy, enum port_event event,
gfp_t gfp_flags);
int sas_notify_phy_event_gfp(struct asd_sas_phy *phy, enum phy_event event,
gfp_t gfp_flags);

#endif /* _SASLIB_H_ */

0 comments on commit c2d0f1a

Please sign in to comment.