2023-06-13 09:56:42

by Tio Zhang

Subject: [PATCH] sched/isolation: add a workqueue parameter to constrain unbound CPUs

Motivation of doing this is to better improve boot times for devices when
we want to prevent our workqueue works from running on some specific CPUs,
i,e, some CPUs are busy with interrupts.

Suggested-by: Tejun Heo <[email protected]>
Signed-off-by: tiozhang <[email protected]>
---
kernel/sched/isolation.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 373d42c707bc..85e744c1349a 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -208,6 +208,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
continue;
}

+ if (!strncmp(str, "workqueue,", 10)) {
+ str += 10;
+ flags |= HK_FLAG_WQ;
+ continue;
+ }
+
if (!strncmp(str, "managed_irq,", 12)) {
str += 12;
flags |= HK_FLAG_MANAGED_IRQ;
--
2.17.1

2023-06-14 09:31:41

by Tio Zhang

Subject: [PATCH v2] sched/isolation: add a workqueue parameter to constrain unbound CPUs

Motivation of doing this is to better improve boot times for devices when
we want to prevent our workqueue works from running on some specific CPUs,
e,g, some CPUs are busy with interrupts.

Suggested-by: Tejun Heo <[email protected]>
Signed-off-by: tiozhang <[email protected]>
---
kernel/sched/isolation.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 373d42c707bc..5cd67c51622e 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -181,8 +181,8 @@ static int __init housekeeping_nohz_full_setup(char *str)
{
unsigned long flags;

- flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
- HK_FLAG_MISC | HK_FLAG_KTHREAD;
+ flags = HK_FLAG_TICK | HK_FLAG_TIMER | HK_FLAG_RCU | HK_FLAG_MISC
+ | HK_FLAG_KTHREAD;

return housekeeping_setup(str, flags);
}
@@ -208,6 +208,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
continue;
}

+ if (!strncmp(str, "workqueue,", 10)) {
+ str += 10;
+ flags |= HK_FLAG_WQ;
+ continue;
+ }
+
if (!strncmp(str, "managed_irq,", 12)) {
str += 12;
flags |= HK_FLAG_MANAGED_IRQ;
--
2.17.1

2023-06-21 21:36:44

by Tejun Heo

Subject: Re: [PATCH v2] sched/isolation: add a workqueue parameter to constrain unbound CPUs

On Wed, Jun 14, 2023 at 05:19:45PM +0800, tiozhang wrote:
...
> @@ -181,8 +181,8 @@ static int __init housekeeping_nohz_full_setup(char *str)
> {
> unsigned long flags;
>
> - flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
> - HK_FLAG_MISC | HK_FLAG_KTHREAD;
> + flags = HK_FLAG_TICK | HK_FLAG_TIMER | HK_FLAG_RCU | HK_FLAG_MISC
> + | HK_FLAG_KTHREAD;

Why is WQ being taken out of nohz_full setup? The patch description doesn't
mention anything.

> @@ -208,6 +208,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
> continue;
> }
>
> + if (!strncmp(str, "workqueue,", 10)) {
> + str += 10;
> + flags |= HK_FLAG_WQ;
> + continue;
> + }

I wonder whether it'd be better to just add a workqueue boot param but if
this works it's fine too.

Thanks.

--
tejun

2023-06-22 03:32:01

by Tio Zhang

Subject: [PATCH v3] sched/isolation: add a workqueue parameter onto isolcpus to constrain unbound CPUs

Motivation of doing this is to better improve boot times for devices when
we want to prevent our workqueue works from running on some specific CPUs,
i,e, some CPUs are busy with interrupts.

Also remove HK_FLAG_WQ from nohz_full to prevent nohz_full overriding
workqueue's cpu config.

Suggested-by: Tejun Heo <[email protected]>
Signed-off-by: tiozhang <[email protected]>
---
kernel/sched/isolation.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 373d42c707bc..5cd67c51622e 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -181,8 +181,8 @@ static int __init housekeeping_nohz_full_setup(char *str)
{
unsigned long flags;

- flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
- HK_FLAG_MISC | HK_FLAG_KTHREAD;
+ flags = HK_FLAG_TICK | HK_FLAG_TIMER | HK_FLAG_RCU | HK_FLAG_MISC
+ | HK_FLAG_KTHREAD;

return housekeeping_setup(str, flags);
}
@@ -208,6 +208,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
continue;
}

+ if (!strncmp(str, "workqueue,", 10)) {
+ str += 10;
+ flags |= HK_FLAG_WQ;
+ continue;
+ }
+
if (!strncmp(str, "managed_irq,", 12)) {
str += 12;
flags |= HK_FLAG_MANAGED_IRQ;
--
2.17.1

2023-06-22 03:53:52

by Yuanhan Zhang

Subject: Re: [PATCH v2] sched/isolation: add a workqueue parameter to constrain unbound CPUs

Tejun Heo <[email protected]> 于2023年6月22日周四 04:51写道：
>
> On Wed, Jun 14, 2023 at 05:19:45PM +0800, tiozhang wrote:
> ...
> > @@ -181,8 +181,8 @@ static int __init housekeeping_nohz_full_setup(char *str)
> > {
> > unsigned long flags;
> >
> > - flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
> > - HK_FLAG_MISC | HK_FLAG_KTHREAD;
> > + flags = HK_FLAG_TICK | HK_FLAG_TIMER | HK_FLAG_RCU | HK_FLAG_MISC
> > + | HK_FLAG_KTHREAD;
>
> Why is WQ being taken out of nohz_full setup? The patch description doesn't
> mention anything.

To prevent nohz_full from rewriting workqueue's choice. Add a
description in patch v3.

>
> > @@ -208,6 +208,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
> > continue;
> > }
> >
> > + if (!strncmp(str, "workqueue,", 10)) {
> > + str += 10;
> > + flags |= HK_FLAG_WQ;
> > + continue;
> > + }
>
> I wonder whether it'd be better to just add a workqueue boot param but if

I also write a patch this way, please see '[PATCH] workqueue: add
cmdline parameter
`unbound_workqueue_cpus` to further constrain wq_unbound_cpumask at boot time'.
If you think it is better I'm happy to continue on this :)

> this works it's fine too.

It works since we use this cpumask to constrain wq_unbound_cpumask in
workqueue's early init.

>
> Thanks.
>
> --
> tejun

Thank you,
tiozhang

2023-06-26 23:21:43

by Frederic Weisbecker

Subject: Re: [PATCH v3] sched/isolation: add a workqueue parameter onto isolcpus to constrain unbound CPUs

On Thu, Jun 22, 2023 at 11:22:17AM +0800, tiozhang wrote:
> Motivation of doing this is to better improve boot times for devices when
> we want to prevent our workqueue works from running on some specific CPUs,
> i,e, some CPUs are busy with interrupts.
>
> Also remove HK_FLAG_WQ from nohz_full to prevent nohz_full overriding
> workqueue's cpu config.

But then nohz_full= doesn't re-affine workqueues anymore, right?

>
> Suggested-by: Tejun Heo <[email protected]>
> Signed-off-by: tiozhang <[email protected]>
> ---
> kernel/sched/isolation.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 373d42c707bc..5cd67c51622e 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -181,8 +181,8 @@ static int __init housekeeping_nohz_full_setup(char *str)
> {
> unsigned long flags;
>
> - flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
> - HK_FLAG_MISC | HK_FLAG_KTHREAD;
> + flags = HK_FLAG_TICK | HK_FLAG_TIMER | HK_FLAG_RCU | HK_FLAG_MISC
> + | HK_FLAG_KTHREAD;
>
> return housekeeping_setup(str, flags);
> }
> @@ -208,6 +208,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
> continue;
> }
>
> + if (!strncmp(str, "workqueue,", 10)) {
> + str += 10;
> + flags |= HK_FLAG_WQ;
> + continue;
> + }
> +

I'm planning on removing HK_FLAG_TICK, HK_FLAG_WQ, HK_FLAG_TIMER, HK_FLAG_RCU,
HK_FLAG_MISC and HK_FLAG_KTHREAD and merge them all together. Also the problem with
these flags is that once they are modified on boot, you can't modify them later.

I much prefer what was done in this patch:

https://lore.kernel.org/all/20230623073033.GA6584@didi-ThinkCentre-M930t-N000/

Thanks!

> if (!strncmp(str, "managed_irq,", 12)) {
> str += 12;
> flags |= HK_FLAG_MANAGED_IRQ;
> --
> 2.17.1
>