linux-kernel - Re: [PATCH] workqueue: Control the frequency of intensive warning through cmdline

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAB8ipk9R3hRSTTEEBn8nvOcTZGBtaDQfomXiQwji+DTKgLghwQ@mail.gmail.com>
Date: Wed, 21 Feb 2024 10:01:17 +0800
From: Xuewen Yan <xuewen.yan94@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: Xuewen Yan <xuewen.yan@...soc.com>, jiangshanlai@...il.com, corbet@....net, 
	paulmck@...nel.org, rdunlap@...radead.org, peterz@...radead.org, 
	yanjiewtw@...il.com, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	ke.wang@...soc.com
Subject: Re: [PATCH] workqueue: Control the frequency of intensive warning
 through cmdline

Hi Tejun

Thanks for the reply!

On Wed, Feb 21, 2024 at 12:46 AM Tejun Heo <tj@...nel.org> wrote:
>
> Hello,
>
> On Mon, Feb 19, 2024 at 03:46:34PM +0800, Xuewen Yan wrote:
> > +#ifdef CONFIG_WQ_CPU_INTENSIVE_REPORT
> > +static unsigned int wq_cpu_intensive_warning_per_count = 4;
> > +module_param_named(cpu_intensive_warning_per_count, wq_cpu_intensive_warning_per_count, uint, 0644);
> > +#endif
>
> wq_cpu_intensive_warning_nth is probably shorter and more idiomatic.
>
> > @@ -1202,7 +1206,7 @@ static void wq_cpu_intensive_report(work_func_t func)
> >                * exponentially.
> >                */
> >               cnt = atomic64_inc_return_relaxed(&ent->cnt);
> > -             if (cnt >= 4 && is_power_of_2(cnt))
> > +             if (wq_cpu_intensive_warning_per_count && !(cnt % wq_cpu_intensive_warning_per_count))
>
> But aren't you mostly interested in the first report? Note that these events
> can be very high frequency and reporting every nth event can lead to a lot
> of constant warnings. Wouldn't it make sense to keep the exponential backoff
> while allowing adjusting the initial threshold?

Yes, it makes sense to keep the exponential backoff,
but this may result in not being able to see the warning when the
threshold is exceeded for the first time.

Or what do you think about changing it to the following?

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 7b482a26d741..e783b68ce597 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1202,7 +1202,8 @@ static void wq_cpu_intensive_report(work_func_t func)
                 * exponentially.
                 */
                cnt = atomic64_inc_return_relaxed(&ent->cnt);
-               if (cnt >= 4 && is_power_of_2(cnt))
+               if (cnt == wq_cpu_intensive_warning_nth ||
+                   (cnt > wq_cpu_intensive_warning_nth && is_power_of_2(cnt)))
                        printk_deferred(KERN_WARNING "workqueue: %ps
hogged CPU for >%luus %llu times, consider switching to WQ_UNBOUND\n",
                                        ent->func, wq_cpu_intensive_thresh_us,
                                        atomic64_read(&ent->cnt));

When the cnt reaches the threshold for the first time, a warning can
be printed immediately.
When it exceeds the threshold, keep the exponential backoff.

Thanks!
BR
--
xuewen

>
> Thanks.
>
> --
> tejun