From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 6CEDF308F13 for ; Tue, 2 Dec 2025 09:05:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764666310; cv=none; b=tQHtz31FtyOaWHEGCCyvtEiBy4bXL8Ew3Jpq10pRmuRtSSNPEutWc0lHbQAZHL633W65QuhO9mnfB6/nG0vhNRreQdB+h3CpRyQTAmalhbEteIKBSg7f/rtN9pXCQbuLCP/uIGuCEEhyFCkZhYMeB2SXQ/MGG7bBJkByjyJjSPo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764666310; c=relaxed/simple; bh=x+cdz8dRilvvwx96GxrNA6kZIxOMBTj6HR3VetYbY5c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=EnkQbFVwVrgKOK38pYpzlOW7+4vcrXKqggMWBb1T5qxZK178RpTDPHO6Ks24CH8QGg65wtmBW9a1xP9O9tl9zOeSFOxqX1ELVEQws7rCUx1X4TAgOetG6dXfe+Dt7MXCi6plLE5PJaBO5cJFXxr1jPgELqL3gZcMLCxHqwzdUks= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LplHLtFs; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LplHLtFs" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E2A21C4CEF1; Tue, 2 Dec 2025 09:05:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1764666309; bh=x+cdz8dRilvvwx96GxrNA6kZIxOMBTj6HR3VetYbY5c=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LplHLtFsAr9ch73j3Lr5WqYNUSogzkTSTt44qsOWkiRIZA/hz/DaNKOfaqJNSFWKy y/51suxThQkq67pA9CQ/gKjIvei9WrTbu16q81PgPJARLsynrGNnr5gz2Ce8hF4A9g tMJtfs5CINFrnzyzf3zl05gsZ3RV3rRnobunbe9rurylA+/fGBMRDSEcG2qiqsgHzy cJjOmA37vN2iUCYBlaTX98hD0CiQDl+GP5XmtbARLnQsZ+wploZVq4Pasl/0eNVRJc w0lxkZPFLYcODfr0Pzet5v1C1Xa1nzte/uc91XgHUDQ7dBEwkUJiUAVhNU2bLrt/f0 j6m9qV13WjqbA== Date: Tue, 2 Dec 2025 10:05:06 +0100 From: Ingo Molnar To: Oleg Nesterov Cc: Peter Zijlstra , linux-kernel@vger.kernel.org Subject: [PATCH -v2] seqlock, procfs: Match scoped_seqlock_read() critical section vs. RCU ordering in do_task_stat() to do_io_accounting() Message-ID: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: * Oleg Nesterov wrote: > On 12/02, Ingo Molnar wrote: > > > > RCU read-lock should not nest inside a read-seqlock > > irqsave ->stats_lock IRQs-off critical section, > > Hmm... I agree with this patch, but is it actually wrong? > > I thought that rcu_read_lock/unlock is safe under spin_lock_irq... Yeah, true - it's allowed and not a bug, merely discouraged inside irqs-off sections if it can be avoided, and it's an inconsistency versus do_io_accounting(). How about the -v2 phrasing below? I also removed the Fixes tags. Thanks, Ingo ===================================> From: Ingo Molnar Date: Tue, 2 Dec 2025 05:09:28 +0100 Subject: [PATCH] seqlock, procfs: Match scoped_seqlock_read() critical section vs. RCU ordering in do_task_stat() to do_io_accounting() There's two patterns of taking the RCU read-lock and the sig->stats_lock read-seqlock in do_task_stat() and do_io_accounting(), with a different ordering: # do_io_accounting(): guard(rcu)(); scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) { # do_task_stat(): scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) { ... rcu_read_lock(); The ordering is RCU-read+seqlock_read in the first case, seqlock_read+RCU-read in the second case. While technically these read locks can be taken in any order, nevertheless it's good practice to use the more intrusive lock on the inside (which is the IRQs-off section in this case), and reduces head-scratching during review when done consistently, so let's use the do_io_accounting() pattern in do_task_stat(). This will also reduce irqs-off latencies in do_task_stat() a tiny bit. Signed-off-by: Ingo Molnar Cc: Oleg Nesterov Cc: Peter Zijlstra Link: https://patch.msgid.link/aS5mdHYhHi9Gi5-r@gmail.com --- fs/proc/array.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index cbd4bc4a58e4..42932f88141a 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -537,27 +537,27 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns, if (permitted && (!whole || num_threads < 2)) wchan = !task_is_running(task); - scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) { - cmin_flt = sig->cmin_flt; - cmaj_flt = sig->cmaj_flt; - cutime = sig->cutime; - cstime = sig->cstime; - cgtime = sig->cgtime; - - if (whole) { - struct task_struct *t; - - min_flt = sig->min_flt; - maj_flt = sig->maj_flt; - gtime = sig->gtime; - - rcu_read_lock(); - __for_each_thread(sig, t) { - min_flt += t->min_flt; - maj_flt += t->maj_flt; - gtime += task_gtime(t); + scoped_guard(rcu) { + scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) { + cmin_flt = sig->cmin_flt; + cmaj_flt = sig->cmaj_flt; + cutime = sig->cutime; + cstime = sig->cstime; + cgtime = sig->cgtime; + + if (whole) { + struct task_struct *t; + + min_flt = sig->min_flt; + maj_flt = sig->maj_flt; + gtime = sig->gtime; + + __for_each_thread(sig, t) { + min_flt += t->min_flt; + maj_flt += t->maj_flt; + gtime += task_gtime(t); + } } - rcu_read_unlock(); } }