1. Patchew
  2. linux
  3. [PATCH 0/9] sched: Use lock guards, wave 1
  4. View patch

[PATCH 1/9] sched: Simplify get_nohz_timer_target()

Peter Zijlstra posted 9 patches 2 years, 1 month ago
[PATCH 1/9] sched: Simplify get_nohz_timer_target()
Posted by Peter Zijlstra 2 years, 1 month ago 
Use guards to reduce gotos and simplify control flow.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1097,25 +1097,22 @@ int get_nohz_timer_target(void)
 
 	hk_mask = housekeeping_cpumask(HK_TYPE_TIMER);
 
-	rcu_read_lock();
+	guard(rcu)();
+
 	for_each_domain(cpu, sd) {
 		for_each_cpu_and(i, sched_domain_span(sd), hk_mask) {
 			if (cpu == i)
 				continue;
 
-			if (!idle_cpu(i)) {
-				cpu = i;
-				goto unlock;
-			}
+			if (!idle_cpu(i))
+				return i;
 		}
 	}
 
 	if (default_cpu == -1)
 		default_cpu = housekeeping_any_cpu(HK_TYPE_TIMER);
-	cpu = default_cpu;
-unlock:
-	rcu_read_unlock();
-	return cpu;
+
+	return default_cpu;
 }
 
 /*
Re: [PATCH 1/9] sched: Simplify get_nohz_timer_target()
Posted by Joel Fernandes 2 years, 1 month ago 
On Sun, Aug 6, 2023 at 9:52 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Use guards to reduce gotos and simplify control flow.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/sched/core.c |   15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1097,25 +1097,22 @@ int get_nohz_timer_target(void)
>
>         hk_mask = housekeeping_cpumask(HK_TYPE_TIMER);
>
> -       rcu_read_lock();
> +       guard(rcu)();
> +
>         for_each_domain(cpu, sd) {
>                 for_each_cpu_and(i, sched_domain_span(sd), hk_mask) {
>                         if (cpu == i)
>                                 continue;
>
> -                       if (!idle_cpu(i)) {
> -                               cpu = i;
> -                               goto unlock;
> -                       }
> +                       if (!idle_cpu(i))
> +                               return i;
>                 }
>         }
>
>         if (default_cpu == -1)
>                 default_cpu = housekeeping_any_cpu(HK_TYPE_TIMER);
> -       cpu = default_cpu;
> -unlock:
> -       rcu_read_unlock();
> -       return cpu;
> +
> +       return default_cpu;
>  }

Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

I haven't looked into the actual implementation of the guard stuff,
but rcu_read_lock_guarded() is less of an eyesore to me than
guard(rcu)(); TBH.

thanks,

 - Joel
Re: [PATCH 1/9] sched: Simplify get_nohz_timer_target()
Posted by Peter Zijlstra 2 years, 1 month ago 
On Sun, Aug 06, 2023 at 05:39:24PM -0400, Joel Fernandes wrote:
> On Sun, Aug 6, 2023 at 9:52 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Use guards to reduce gotos and simplify control flow.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> >  kernel/sched/core.c |   15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -1097,25 +1097,22 @@ int get_nohz_timer_target(void)
> >
> >         hk_mask = housekeeping_cpumask(HK_TYPE_TIMER);
> >
> > -       rcu_read_lock();
> > +       guard(rcu)();
> > +
> >         for_each_domain(cpu, sd) {
> >                 for_each_cpu_and(i, sched_domain_span(sd), hk_mask) {
> >                         if (cpu == i)
> >                                 continue;
> >
> > -                       if (!idle_cpu(i)) {
> > -                               cpu = i;
> > -                               goto unlock;
> > -                       }
> > +                       if (!idle_cpu(i))
> > +                               return i;
> >                 }
> >         }
> >
> >         if (default_cpu == -1)
> >                 default_cpu = housekeeping_any_cpu(HK_TYPE_TIMER);
> > -       cpu = default_cpu;
> > -unlock:
> > -       rcu_read_unlock();
> > -       return cpu;
> > +
> > +       return default_cpu;
> >  }
> 
> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> 
> I haven't looked into the actual implementation of the guard stuff,
> but rcu_read_lock_guarded() is less of an eyesore to me than
> guard(rcu)(); TBH.

I readily admit it isn't the prettiest construct, my brain is warped by
many years of C++ and I can read it as: guard<rcu>(), but I'm not sure
that's actually better :-)

The advantage of all this is that you also get:

	scoped_guard (rcu) {
	}

for 'free'.
Re: [PATCH 1/9] sched: Simplify get_nohz_timer_target()
Posted by Joel Fernandes 2 years ago 
On Wed, Aug 9, 2023 at 3:48 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Sun, Aug 06, 2023 at 05:39:24PM -0400, Joel Fernandes wrote:
> > On Sun, Aug 6, 2023 at 9:52 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > Use guards to reduce gotos and simplify control flow.
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > ---
> > >  kernel/sched/core.c |   15 ++++++---------
> > >  1 file changed, 6 insertions(+), 9 deletions(-)
> > >
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -1097,25 +1097,22 @@ int get_nohz_timer_target(void)
> > >
> > >         hk_mask = housekeeping_cpumask(HK_TYPE_TIMER);
> > >
> > > -       rcu_read_lock();
> > > +       guard(rcu)();
> > > +
> > >         for_each_domain(cpu, sd) {
> > >                 for_each_cpu_and(i, sched_domain_span(sd), hk_mask) {
> > >                         if (cpu == i)
> > >                                 continue;
> > >
> > > -                       if (!idle_cpu(i)) {
> > > -                               cpu = i;
> > > -                               goto unlock;
> > > -                       }
> > > +                       if (!idle_cpu(i))
> > > +                               return i;
> > >                 }
> > >         }
> > >
> > >         if (default_cpu == -1)
> > >                 default_cpu = housekeeping_any_cpu(HK_TYPE_TIMER);
> > > -       cpu = default_cpu;
> > > -unlock:
> > > -       rcu_read_unlock();
> > > -       return cpu;
> > > +
> > > +       return default_cpu;
> > >  }
> >
> > Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> >
> > I haven't looked into the actual implementation of the guard stuff,
> > but rcu_read_lock_guarded() is less of an eyesore to me than
> > guard(rcu)(); TBH.
>
> I readily admit it isn't the prettiest construct, my brain is warped by
> many years of C++ and I can read it as: guard<rcu>(), but I'm not sure
> that's actually better :-)
>
> The advantage of all this is that you also get:
>
>         scoped_guard (rcu) {
>         }
>
> for 'free'.

Yes, overall the readability improvement is quite appealing. Thank you Peter!

 - Joel
[tip: sched/core] sched: Simplify get_nohz_timer_target()
Posted by tip-bot2 for Peter Zijlstra 2 years ago 
The following commit has been merged into the sched/core branch of tip:

Commit-ID:     7537b90c0036759e0b1b43dfbc6224dc5e900b13
Gitweb:        https://git.kernel.org/tip/7537b90c0036759e0b1b43dfbc6224dc5e900b13
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 01 Aug 2023 22:41:22 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 14 Aug 2023 17:01:24 +02:00

sched: Simplify get_nohz_timer_target()

Use guards to reduce gotos and simplify control flow.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
Link: https://lore.kernel.org/r/20230801211811.828443100@infradead.org
---
 kernel/sched/core.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a97eab3..6cda296 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1097,25 +1097,22 @@ int get_nohz_timer_target(void)
 
 	hk_mask = housekeeping_cpumask(HK_TYPE_TIMER);
 
-	rcu_read_lock();
+	guard(rcu)();
+
 	for_each_domain(cpu, sd) {
 		for_each_cpu_and(i, sched_domain_span(sd), hk_mask) {
 			if (cpu == i)
 				continue;
 
-			if (!idle_cpu(i)) {
-				cpu = i;
-				goto unlock;
-			}
+			if (!idle_cpu(i))
+				return i;
 		}
 	}
 
 	if (default_cpu == -1)
 		default_cpu = housekeeping_any_cpu(HK_TYPE_TIMER);
-	cpu = default_cpu;
-unlock:
-	rcu_read_unlock();
-	return cpu;
+
+	return default_cpu;
 }
 
 /*

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.