2023-08-01 22:26:48

by Peter Zijlstra

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

Use guards to reduce gotos and simplify control flow.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
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;
}

/*

2023-08-06 23:09:22

by Joel Fernandes

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

On Sun, Aug 6, 2023 at 9:52 AM Peter Zijlstra <[email protected]> wrote:
>
> Use guards to reduce gotos and simplify control flow.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> 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) <[email protected]>

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

2023-08-09 20:24:28

by Peter Zijlstra

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

On Sun, Aug 06, 2023 at 05:39:24PM -0400, Joel Fernandes wrote:
> On Sun, Aug 6, 2023 at 9:52 AM Peter Zijlstra <[email protected]> wrote:
> >
> > Use guards to reduce gotos and simplify control flow.
> >
> > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> > ---
> > 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) <[email protected]>
>
> 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'.

2023-08-11 08:00:48

by Joel Fernandes

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

On Wed, Aug 9, 2023 at 3:48 PM Peter Zijlstra <[email protected]> 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 <[email protected]> wrote:
> > >
> > > Use guards to reduce gotos and simplify control flow.
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> > > ---
> > > 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) <[email protected]>
> >
> > 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

2023-08-14 15:15:06

by tip-bot2 for Alexey Makhalov

Subject: [tip: sched/core] sched: Simplify get_nohz_timer_target()

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 <[email protected]>
AuthorDate: Tue, 01 Aug 2023 22:41:22 +02:00
Committer: Peter Zijlstra <[email protected]>
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) <[email protected]>
Reviewed-by: Joel Fernandes (Google) <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
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;
}

/*