Re: [PATCH v3] workqueue: Wrap flush_workqueue() using a macro

[Tejun Heo]

On Mon, May 16, 2022 at 02:00:37PM +0900, Tetsuo Handa wrote:
> This patch is expected to be sent to linux.git immediately before the merge
> window for 5.19 closes, for we need to wait until patches for removing
> flush_workqueue(system_*_wq) calls reached linux.git during the merge window
> for 5.19. Due to this ordering dependency, the kernel build bots complain like
> https://lkml.kernel.org/r/202205021448.6SzhD1Cz-lkp@xxxxxxxxx , but it seems
> that simply ignoring these reports is the best choice.
> 
> If it is inconvenient for you to send pull request twice from your wq.git tree
> (once for immediately after 5.18 is released, the other for immediately before
> the merge window for 5.19 closes), I can send pull request for this patch from
> my tree. In that case, please give me Acked-by: or Reviewed-by: response regarding
> this patch.

What we can do is sending the patch to Andrew. The akpm patches float above
-next and get applied the last. I won't need any special treatment.

> +/*
> + * Detect attempt to flush system-wide workqueues at compile time when possible.
> + * See https://lkml.kernel.org/r/49925af7-78a8-a3dd-bce6-cfc02e1a9236@xxxxxxxxxxxxxxxxxxx for
> + * reasons and steps for converting system-wide workqueues into local workqueues.
> + */
> +#define flush_workqueue(wq)						\
> +({									\
> +	BUILD_BUG_ON_MSG(__builtin_constant_p(&(wq) == &system_wq) &&	\
> +			 &(wq) == &system_wq,				\
> +			 "Please avoid flushing system_wq.");		\

It kinda bothers me that this causes a build failure. It'd be better if we
can trigger #warning instead. I'm not sure whether there'd be a clean way to
do it tho. Maybe just textual matching would provide similar coverage? How
did you test this?

>  #endif
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 0d2514b4ff0d..08255c332e4b 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -2794,6 +2794,7 @@ static bool flush_workqueue_prep_pwqs(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)

Maybe rename the function to __flush_workqueue() instead of undef'ing the
macro?

Thanks.

-- 
tejun