Skip to content
  • Sebastian Andrzej Siewior's avatar
    817a7c99
    scsi: message: fusion: Remove in_interrupt() usage in mptsas_cleanup_fw_event_q() · 817a7c99
    Sebastian Andrzej Siewior authored
    mptsas_cleanup_fw_event_q() uses in_interrupt() to determine if it is safe
    to cancel a worker item.
    
    Aside of that in_interrupt() is deprecated as it does not provide what the
    name suggests. It covers more than hard/soft interrupt servicing context
    and is semantically ill defined.
    
    Looking closer there are a few problems with the current construct:
    
     - It could be invoked from an interrupt handler / non-blocking context
       because cancel_delayed_work() has no such restriction. Also,
       mptsas_free_fw_event() has no such restriction.
    
     - The list is accessed unlocked. It may dequeue a valid work-item but at
       the time of invoking cancel_delayed_work() the memory may be released or
       reused because the worker has already run.
    
    mptsas_cleanup_fw_event_q() is invoked via mptsas_shutdown() which is
    always invoked from preemtible context on device shutdown.  It is also
    invoked via mptsas_ioc_reset(, MPT_IOC_POST_RESET) which is a
    MptResetHandlers callback. The only caller here are mpt_SoftResetHandler(),
    mpt_HardResetHandler() and mpt_Soft_Hard_ResetHandler(). All these
    functions have a `sleepFlag' argument and each caller uses caller uses
    `CAN_SLEEP' here and according to current documentation: | @sleepFlag:
    Indicates if sleep or schedule must be called
    
    So it is safe to sleep.
    
    Add mptsas_hotplug_event::users member. Initialize it to one by default so
    mptsas_free_fw_event() will free the memory.  mptsas_cleanup_fw_event_q()
    will increment its value for items it dequeues and then it may keep a
    pointer after dropping the lock.  Invoke cancel_delayed_work_sync() to
    cancel the work item and wait if the worker is currently busy. Free the
    memory afterwards since it owns the last reference to it.
    
    Link: https://lore.kernel.org/r/20201126132952.2287996-15-bigeasy@linutronix.de
    
    
    Cc: Sathya Prakash <sathya.prakash@broadcom.com>
    Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
    Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>
    Cc: MPT-FusionLinux.pdl@broadcom.com
    Reviewed-by: default avatarDaniel Wagner <dwagner@suse.de>
    Signed-off-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
    817a7c99
    scsi: message: fusion: Remove in_interrupt() usage in mptsas_cleanup_fw_event_q()
    Sebastian Andrzej Siewior authored
    mptsas_cleanup_fw_event_q() uses in_interrupt() to determine if it is safe
    to cancel a worker item.
    
    Aside of that in_interrupt() is deprecated as it does not provide what the
    name suggests. It covers more than hard/soft interrupt servicing context
    and is semantically ill defined.
    
    Looking closer there are a few problems with the current construct:
    
     - It could be invoked from an interrupt handler / non-blocking context
       because cancel_delayed_work() has no such restriction. Also,
       mptsas_free_fw_event() has no such restriction.
    
     - The list is accessed unlocked. It may dequeue a valid work-item but at
       the time of invoking cancel_delayed_work() the memory may be released or
       reused because the worker has already run.
    
    mptsas_cleanup_fw_event_q() is invoked via mptsas_shutdown() which is
    always invoked from preemtible context on device shutdown.  It is also
    invoked via mptsas_ioc_reset(, MPT_IOC_POST_RESET) which is a
    MptResetHandlers callback. The only caller here are mpt_SoftResetHandler(),
    mpt_HardResetHandler() and mpt_Soft_Hard_ResetHandler(). All these
    functions have a `sleepFlag' argument and each caller uses caller uses
    `CAN_SLEEP' here and according to current documentation: | @sleepFlag:
    Indicates if sleep or schedule must be called
    
    So it is safe to sleep.
    
    Add mptsas_hotplug_event::users member. Initialize it to one by default so
    mptsas_free_fw_event() will free the memory.  mptsas_cleanup_fw_event_q()
    will increment its value for items it dequeues and then it may keep a
    pointer after dropping the lock.  Invoke cancel_delayed_work_sync() to
    cancel the work item and wait if the worker is currently busy. Free the
    memory afterwards since it owns the last reference to it.
    
    Link: https://lore.kernel.org/r/20201126132952.2287996-15-bigeasy@linutronix.de
    
    
    Cc: Sathya Prakash <sathya.prakash@broadcom.com>
    Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
    Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>
    Cc: MPT-FusionLinux.pdl@broadcom.com
    Reviewed-by: default avatarDaniel Wagner <dwagner@suse.de>
    Signed-off-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Loading