linux-kernel - Re: [PATCH v3] workqueue: Wrap flush

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 May 2022 22:01:25 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] workqueue: Wrap flush_workqueue() using a macro

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@intel.com , 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@I-love.SAKURA.ne.jp 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