RE: [PATCH 10/16] aacraid: Terminate kthread on controller fw assert

Raghava Aditya Renukunta
2017-02-16T19:54:17Z

> -----Original Message-----
> From: Johannes Thumshirn [mailto:jthumshirn@suse.de]
> Sent: Thursday, February 16, 2017 1:31 AM
> To: Raghava Aditya Renukunta
> <RaghavaAditya.Renukunta@microsemi.com>; jejb@linux.vnet.ibm.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Dave Carroll <david.carroll@microsemi.com>; Gana Sridaran
> <gana.sridaran@microsemi.com>; Scott Benesh
> <scott.benesh@microsemi.com>; dan.carpenter@oracle.com
> Subject: Re: [PATCH 10/16] aacraid: Terminate kthread on controller fw
> assert
>
> EXTERNAL EMAIL
>
>
> On 02/15/2017 11:22 PM, Raghava Aditya Renukunta wrote:
> >>
> >> This look a bit scary. Can't the kthread be converted to a workqueue so
> >> we could call cancel_work_sync()?
> >
> > Could you please elaborate on the reasons why this fix is scary?
> > I understand that killing a thread is not standard (for any reason),
> > and if there are other nuanced issues I would like to understand them.
>
> I'm actually concerned that this could have all kinds of side effects.
> But this is just a gut feeling. I see some drm drivers are doing the
> same, so it might be possible, but IMHO this is not a good design.
>
> And IIRC kthreads do have more downsides (i.e. CPU hotplugging and
> issues with kernel live patching).
>
> I think most kthreads (haven't looked too close to the aacraid kthread I
> must admit, but I'll be doing so) can be converted to either workqueues
> or timers (or a combination of both).
>
> Thanks,
> Johannes

Makes sense, and I agree. With that being said I will withdraw this patch
and resend it out in different patch series once we rework aac_command_thread
into a work queue/timers.

Regards,
Raghava Aditya

> --
> Johannes Thumshirn Storage
> jthumshirn@suse.de +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850