Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 26 May 2023 17:25:58 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     torvalds@linux-foundation.org, keescook@chromium.org,
        pbonzini@redhat.com, linux-kernel@vger.kernel.org,
        ojeda@kernel.org, ndesaulniers@google.com, mingo@redhat.com,
        will@kernel.org, longman@redhat.com, boqun.feng@gmail.com,
        juri.lelli@redhat.com, vincent.guittot@linaro.org,
        dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
        mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com,
        paulmck@kernel.org, frederic@kernel.org, quic_neeraju@quicinc.com,
        joel@joelfernandes.org, josh@joshtriplett.org,
        mathieu.desnoyers@efficios.com, jiangshanlai@gmail.com,
        qiang1.zhang@intel.com, rcu@vger.kernel.org, tj@kernel.org,
        tglx@linutronix.de
Subject: Re: [RFC][PATCH 2/2] sched: Use fancy new guards
Message-ID: <2023052626-blunderer-delegator-4b82@gregkh>
References: <20230526150549.250372621@infradead.org>
 <20230526151947.027972233@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230526151947.027972233@infradead.org>
Precedence: bulk

On Fri, May 26, 2023 at 05:05:51PM +0200, Peter Zijlstra wrote:
> Convert kernel/sched/core.c to use the fancy new guards to simplify
> the error paths.

That's slightly crazy...

I like the idea, but is this really correct:


> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/sched/core.c  | 1223 +++++++++++++++++++++++----------------------------
>  kernel/sched/sched.h |   39 +
>  2 files changed, 595 insertions(+), 667 deletions(-)
> 
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1097,24 +1097,21 @@ int get_nohz_timer_target(void)
>  
>  	hk_mask = housekeeping_cpumask(HK_TYPE_TIMER);
>  
> -	rcu_read_lock();
> -	for_each_domain(cpu, sd) {
> -		for_each_cpu_and(i, sched_domain_span(sd), hk_mask) {
> -			if (cpu == i)
> -				continue;
> +	void_scope(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;

You can call return from within a "scope" and it will clean up properly?

I tried to read the cpp "mess" but couldn't figure out how to validate
this at all, have a set of tests for this somewhere?

Anyway, the naming is whack, but I don't have a proposed better name,
except you might want to put "scope_" as the prefix not the suffix, but
then that might look odd to, so who knows.

But again, the idea is good, it might save us lots of "you forgot to
clean this up on the error path" mess that we are getting constant churn
for these days...

thanks,

greg k-h