On Thu, Nov 26, 2020 at 02:29:52PM +0100, Sebastian Andrzej Siewior wrote: > 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(). mpt_diag_reset(), iterates over all reset handler... > All these functions have a `sleepFlag' ...and also uses the same flag. > 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. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> > Cc: Sathya Prakash <sathya.prakash@xxxxxxxxxxxx> > Cc: Sreekanth Reddy <sreekanth.reddy@xxxxxxxxxxxx> > Cc: Suganath Prabu Subramani <suganath-prabu.subramani@xxxxxxxxxxxx> > Cc: MPT-FusionLinux.pdl@xxxxxxxxxxxx Reviewed-by: Daniel Wagner <dwagner@xxxxxxx>
