Re: [PATCH] checkpatch: warn about flushing system-wide workqueues

From: Joe Perches
Date: Thu May 05 2022 - 11:58:13 EST


On Thu, 2022-05-05 at 22:42 +0900, Tetsuo Handa wrote:
> On 2022/04/25 9:33, Tetsuo Handa wrote:
> > On 2022/04/25 8:45, Joe Perches wrote:
> > > And it's probably more readable using separate lines and it looks as
> > > if the 3rd test is unnecessary as it could be combined with the 2nd.
> > >
> > > if ($line =~ /\bflush_scheduled_work\s*\(/ ||
> > > $line =~ /\bflush_workqueue\s*\(\s*system(_\w*)?_wq\s*\)/) {
> >
> > We don't need to worry about possibility like
> >
> > flush_workqueue(system_module1_wq);
> >
> > ? Then, we can simplify like you suggested.
>
> I initially thought that also doing static checks by scripts/checkpatch.pl
> is better than only doing runtime WARN_ON(). But not all patches are checked
> by scripts/checkpatch.pl . Thus, as an attempt to check without exemptions,
> I now think that doing static checks via BUILD_BUG_ON() is better than
> scripts/checkpatch.pl . I sent below patch to linux-next.git , and so far
> it seems working (I mean, no build failure reports caused by compilers).
>
> Subject: workqueue: Wrap flush_workqueue() using a macro
>
> A conversion to stop flushing kernel-global workqueues is in progress.
> Wrap flush_workqueue() and inject BUILD_BUG_ON() checks, in order to
> prevent users who are not aware of this conversion from again start
> flushing kernel-global workqueues.
[]
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
[]
> @@ -2813,6 +2813,7 @@ static void warn_flush_attempt(struct workqueue_struct *wq)
> * This function sleeps until all work items which were queued on entry
> * have finished execution, but it is not livelocked by new incoming ones.
> */
> +#undef flush_workqueue
> void flush_workqueue(struct workqueue_struct *wq)

The #undef flush_workqueue could be removed by using

void (flush_workqueue)(struct workqueue_struct *wq)
{
...
}