Received-SPF: pass (google.com: domain of linux-kernel+bounces-193733-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 29 May 2024 10:25:30 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Ankur Arora <ankur.a.arora@oracle.com>
Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de,
	torvalds@linux-foundation.org, paulmck@kernel.org,
	rostedt@goodmis.org, mark.rutland@arm.com, juri.lelli@redhat.com,
	joel@joelfernandes.org, raghavendra.kt@amd.com,
	sshegde@linux.ibm.com, boris.ostrovsky@oracle.com,
	konrad.wilk@oracle.com, Ingo Molnar <mingo@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>
Subject: Re: [PATCH v2 07/35] sched: define *_tsk_need_resched_lazy() helpers
Message-ID: <20240529082530.GJ26599@noisy.programming.kicks-ass.net>
References: <20240528003521.979836-1-ankur.a.arora@oracle.com>
 <20240528003521.979836-8-ankur.a.arora@oracle.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240528003521.979836-8-ankur.a.arora@oracle.com>

On Mon, May 27, 2024 at 05:34:53PM -0700, Ankur Arora wrote:

>  static inline void clear_tsk_need_resched(struct task_struct *tsk)
>  {
> -	clear_tsk_thread_flag(tsk,TIF_NEED_RESCHED);
> +	clear_tsk_thread_flag(tsk, tif_resched(RESCHED_NOW));
> +
> +	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
> +		clear_tsk_thread_flag(tsk, tif_resched(RESCHED_LAZY));
> +}

(using tif_resched() here is really uncalled for)

So this will generate rather sub-optimal code, namely 2 atomics that
really should be one.

Ideally we'd write this something like:

	unsigned long mask = _TIF_NEED_RESCHED;
	if (IS_ENABLED(CONFIG_PREEMPT_AUTO))
		mask |= _TIF_NEED_RESCHED_LAZY;

	atomic_long_andnot(mask, (atomic_long_t *)task_thread_info(tsk)->flags);

Which will clear both bits with a single atomic.