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

[Xuewen Yan]

Hi Tejun

Thanks for the reply!

On Wed, Feb 21, 2024 at 12:46 AM Tejun Heo <tj@xxxxxxxxxx> 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