On Fri, 2016-04-01 at 13:56 -0400, Joe Lawrence wrote:
> The driver's fw events are queued up using the the fw_event_work's
> struct work, not its delayed_work member. The latter appears to be
> unused and may provoke CONFIG_DEBUG_OBJECTS_TIMERS "assert_init not
> available" false warnings in _scsih_fw_event_cleanup_queue. Remove it
> and update _scsih_fw_event_cleanup_queue accordingly.
>
> Signed-off-by: Joe Lawrence <joe.lawrence@xxxxxxxxxxx>
> ---
>
> I think this goes all the way back to the introduction of the mpt3sas
> driver. The previous generation mpt2sas driver uses delayed_work, so
> perhaps it was simply copied and pasted into the mpt3sas but never
> updated.
>
> drivers/scsi/mpt3sas/mpt3sas_scsih.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index e0e4920d0fa6..67643602efbc 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -189,7 +189,6 @@ struct fw_event_work {
> struct list_head list;
> struct work_struct work;
> u8 cancel_pending_work;
> - struct delayed_work delayed_work;
>
> struct MPT3SAS_ADAPTER *ioc;
> u16 device_handle;
> @@ -2804,12 +2803,12 @@ _scsih_fw_event_cleanup_queue(struct MPT3SAS_ADAPTER *ioc)
> /*
> * Wait on the fw_event to complete. If this returns 1, then
> * the event was never executed, and we need a put for the
> - * reference the delayed_work had on the fw_event.
> + * reference the work had on the fw_event.
> *
> * If it did execute, we wait for it to finish, and the put will
> * happen from _firmware_event_work()
> */
> - if (cancel_delayed_work_sync(&fw_event->delayed_work))
> + if (cancel_work_sync(&fw_event->work))
> fw_event_work_put(fw_event);
>
> fw_event_work_put(fw_event);
Hmm... Fixes: 146b16c8 (mpt3sas: Refcount fw_events and fix unsafe list usage)
Since mpt3sas uses ->work instead of _delayed_work this would seem to be
correct, however the deprecated mpt2sas driver had a commit that changed
the firmware event work mechanism to use ->delayed_work instead of ->work:
commit f1c35e6aea579d5bdb6dc02dfa99c67c7c3b3f67
Author: Kashyap, Desai <kashyap.desai@xxxxxxx>
Date: Tue Mar 9 16:31:43 2010 +0530
[SCSI] mpt2sas: RESCAN Barrier work is added in case of HBA reset.
Add the cancel_pending_work flag from the fw_event_work structure, and then to
set the flag during host reset, check the flag later from work threads
context and if cancel_pending_work_flag is set ingore those events.
Now Rescan after host reset is changed.
Added special task MPT2SAS_RESCAN_AFTER_HOST_RESET. This task will be queued
at the time of HBA reset. this task is treated as barrier. All work after
MPT2SAS_RESCAN_AFTER_HOST_RESET will be treated as new work and will be
server by callback handle. If host_recovery is going on while running RESCAN
task, it will wait for shos_recovery_done completion which will be called
from HBA reset DONE context.
Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxx>
Reviewed-by: Eric Moore <eric.moore@xxxxxxx>
Signed-off-by: James Bottomley <James.Bottomley@xxxxxxx>
Portions of that patch include:
@@ -125,11 +127,11 @@ struct sense_info {
*/
struct fw_event_work {
struct list_head list;
- struct work_struct work;
+ u8 cancel_pending_work;
+ struct delayed_work delayed_work;
struct MPT2SAS_ADAPTER *ioc;
u8 VF_ID;
u8 VP_ID;
- u8 host_reset_handling;
u8 ignore;
u16 event;
void *event_data;
and
@@ -2325,8 +2327,9 @@ _scsih_fw_event_add(struct MPT2SAS_ADAPTER *ioc, struct fw_event_work *fw_event)
spin_lock_irqsave(&ioc->fw_event_lock, flags);
list_add_tail(&fw_event->list, &ioc->fw_event_list);
- INIT_WORK(&fw_event->work, _firmware_event_work);
- queue_work(ioc->firmware_event_thread, &fw_event->work);
+ INIT_DELAYED_WORK(&fw_event->delayed_work, _firmware_event_work);
+ queue_delayed_work(ioc->firmware_event_thread,
+ &fw_event->delayed_work, 0);
spin_unlock_irqrestore(&ioc->fw_event_lock, flags);
}
The delay argument to queue_delayed_work() is zero though.
So, Broadcom, presumably Joe's fix is the correct fix?
-Ewan
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
