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

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

[tiozhang]

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 <tj@kernel.org>
Signed-off-by: tiozhang <tiozhang@didiglobal.com>
---
 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

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

[tiozhang]

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 <tj@kernel.org>
Signed-off-by: tiozhang <tiozhang@didiglobal.com>
---
 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

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

[Tejun Heo]

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

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

[tiozhang]

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 <tj@kernel.org>
Signed-off-by: tiozhang <tiozhang@didiglobal.com>
---
 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

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

[Yuanhan Zhang]

Tejun Heo <tj@kernel.org> 于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

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

[Tejun Heo]

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.
> 
> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: tiozhang <tiozhang@didiglobal.com>

Let's just do the workqueue specific parameter. I'll apply the other one to
the workqueue branch after the merge window.

Thanks.

-- 
tejun

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

[Frederic Weisbecker]

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 <tj@kernel.org>
> Signed-off-by: tiozhang <tiozhang@didiglobal.com>
> ---
>  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
>