From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4D45335972; Wed, 16 Jul 2025 13:33:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752672796; cv=none; b=JIrpyzY4VMCBCfWraLLcFow5YRcK/B64RDPWUyjyk76q9iCUMZFBuZZMZpB6QcQqyb9t5MtsEYwROMmrPznF0tuMqZdJotVawYLXAO2RQuliqRmsTqMHpJbxG21gcgqvIxTdLsdLsxQdxYagti+D3W9j0t9W9IuXuUiG4DdVXco= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752672796; c=relaxed/simple; bh=x6i70Mv2Cezij3IaXwLb1y0bICFN4EcBfdv0Utb/q4E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=iLG3Bq5JOPt7PgKa+8qelT4hCr4wCeoGXcTr+CLRafZyt/R0Anwz5Ta81O6u/eSz7VDl/h2hulSomaGM6WgzexjJADoNwmF0G+pAKGqUdgWkrVYeu87ZDSfTuYmmEadtNFfRxecHzlcdkYYwgC5CLzCqR9OwaxydeKUO8QvDvkI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=Qy+qhD+g; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="Qy+qhD+g" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=7JIhGkXj7jed7Tn8m6IrEdyMDpGTVSbcYUAZwKdgoBE=; b=Qy+qhD+gb3pgMFanQ7Me6t26QK MYQdNecjBni5/BjUWKXv43GI+GjZlDs0xjoOe8xlnsJ3yQh+r59X2WEEyi1EXktloR40O6BKut7O4 1/vqWv+M4/Dvus8BMqQInvfCCdiYKaXa7udvtijW1d+GTfIMsczaesLkYmJvO/95ee0RKcMLw7uzE 4xzXfeJ6guQRJF/7Wn1NxRnJCKaREp/jahZaHpPyNSsMVpOncz04CHymCxmhXvsG2Hm3FmVhjDgTR sV2r5flmG6F4i4lIRggvBVximL+ir73t2o78VMWFjnKR+V3bJJd/+FYF5FvtQu0WAOyuIOa3R/O9v gciVyqtA==; Received: from 77-249-17-252.cable.dynamic.v4.ziggo.nl ([77.249.17.252] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.98.2 #2 (Red Hat Linux)) id 1uc2Fv-0000000GhqT-3Aqb; Wed, 16 Jul 2025 13:33:07 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 55F8C3007A0; Wed, 16 Jul 2025 15:33:07 +0200 (CEST) Date: Wed, 16 Jul 2025 15:33:07 +0200 From: Peter Zijlstra To: Andrea Righi Cc: Steven Rostedt , Breno Leitao , Tejun Heo , David Vernet , Changwoo Min , Ingo Molnar , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Ben Segall , Mel Gorman , Valentin Schneider , sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org, kernel-team@meta.com, jake@hillion.co.uk Subject: Re: [PATCH] sched/ext: Suppress warning in __this_cpu_write() by disabling preemption Message-ID: <20250716133307.GY905792@noisy.programming.kicks-ass.net> References: <20250716-scx_warning-v1-1-0e814f78eb8c@debian.org> <20250716085447.06feeb86@gandalf.local.home> <20250716130652.GB3429938@noisy.programming.kicks-ass.net> Precedence: bulk X-Mailing-List: sched-ext@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Wed, Jul 16, 2025 at 03:20:33PM +0200, Andrea Righi wrote: > On Wed, Jul 16, 2025 at 03:06:52PM +0200, Peter Zijlstra wrote: > > On Wed, Jul 16, 2025 at 08:54:47AM -0400, Steven Rostedt wrote: > > > On Wed, 16 Jul 2025 05:46:15 -0700 > > > Breno Leitao wrote: > > > > > > > __this_cpu_write() emits a warning if used with preemption enabled. > > > > > > > > Function update_locked_rq() might be called with preemption enabled, > > > > which causes the following warning: > > > > > > > > BUG: using __this_cpu_write() in preemptible [00000000] code: scx_layered_6-9/68770 > > > > > > > > Disable preemption around the __this_cpu_write() call in > > > > update_locked_rq() to suppress the warning, without affecting behavior. > > > > > > > > If preemption triggers a jump to another CPU during the callback it's > > > > fine, since we would track the rq state on the other CPU with its own > > > > local variable. > > > > > > > > Suggested-by: Andrea Righi > > > > Signed-off-by: Breno Leitao > > > > Fixes: 18853ba782bef ("sched_ext: Track currently locked rq") > > > > Acked-by: Andrea Righi > > > > --- > > > > kernel/sched/ext.c | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c > > > > index b498d867ba210..24fcbd7331f73 100644 > > > > --- a/kernel/sched/ext.c > > > > +++ b/kernel/sched/ext.c > > > > @@ -1258,7 +1258,14 @@ static inline void update_locked_rq(struct rq *rq) > > > > */ > > > > if (rq) > > > > lockdep_assert_rq_held(rq); > > > > > > > > > > > > If an rq lock is expected to be held, there had better be no preemption > > > enabled. How is this OK? > > > > The rq=NULL case; but from the usage I've seen that also happens with > > rq lock held. > > > > Specifically I think the check ought to be: > > > > if (rq) > > lockdep_assert_rq_held(rq) > > else > > lockdep_assert_rq_held(__this_cpu_read(locked_rq)); > > Hm... but if the same CPU invokes two "unlocked" callbacks in a row, > locked_rq would be NULL during the second call and we would check rq_held > against NULL. Current usage in SCX_CALL_OP*() seems to not generate this pattern. It is always rq,NULL in order. Ooh, there are a few SCX_CALL_OP*() instances where rq:=NULL, which messes this up. Changing them to: if (rq) update_locked_rq(rq); ... if (rq) update_locked_rq(NULL); might help.