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 691811DF980 for ; Thu, 20 Nov 2025 07:20:46 +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=1763623247; cv=none; b=AmG/HqZzlJXy2ZsoYXzuq0XMC45hCkaFLXhcDnb0hklXvWJRMhTz/BzVPcnXnVr7i+TFRjsKOn/fBTf13tU8FSLwh2Qgmz1+9KIfz8rjtEUbYczM90FQNmxIrDl7QsjoXvx9UZRYjsXZMhqslWTuYXvKG+QIEQisQE6traZve9Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763623247; c=relaxed/simple; bh=XqfYwGVpsQlsjLj+dKef6/7t4XaKLG5UffFqCC17OVU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=jBC6ZXKt+LFKU79S9g0S/kueOWgMv0hCBZrC7F7O1f0xKqCKDvTgQga44x2GimuXTrdQ59mMFZ0L1tF0KDtCc+AoNfdDthzxuVmczIrEayBeiEFj2Bhfu7v0idELCLKnYLCbtpJqe0684UEpWmYQzKjHr1oLAHdvsDj45w0zxbI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=M/HFv5at; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="M/HFv5at" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4853EC4CEF1; Thu, 20 Nov 2025 07:20:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1763623245; bh=XqfYwGVpsQlsjLj+dKef6/7t4XaKLG5UffFqCC17OVU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=M/HFv5at2vwev70YnaXCuL4wDcI/iRlmPLnMwMmZw0kURSrfLC6c9rHDOzuyKMESX pa81Btn7uT/HQZHYtllyXrbTvqouvsuPtV07v/WplkcMhBPZGsw3ykF6Frkqx+fOWO rxswcSBddqqFzt3jZDqnuHcR1k5Hz6pVO6MG3BXc= Date: Thu, 20 Nov 2025 02:20:39 -0500 From: Greg Kroah-Hartman To: Kevin Mitchell Cc: Tejun Heo , Chengming Zhou , linux-kernel@vger.kernel.org Subject: Re: [PATCH] kernfs: use atomics for open_node refcounts Message-ID: <2025112056-acorn-skeptic-63cc@gregkh> References: <20251119191758.612694-2-kevmitch@arista.com> 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: <20251119191758.612694-2-kevmitch@arista.com> On Wed, Nov 19, 2025 at 11:17:57AM -0800, Kevin Mitchell wrote: > Counts of files to be released and mmapped files were added to > kernfs_open_node in commit bdb2fd7fc56e ("kernfs: Skip > kernfs_drain_open_files() more aggressively") to optimize > kernfs_drain_open_files(). A WARN_ON_ONCE sanity check was also added in > kernfs_drain_open_files to ensure that these counters were brought to > zero once all files had been drained. > > This WARNING was found to trigger on occasion during pci hotplug link > down when mmapped pci resource files were removed in > pci_remove_resource_files. This was caused by a race condition during > the initial mmappings where incrementing nr_mmapped from multiple CPUs > such that 2 (or possibly more) simultaneous increments overwrote each > other thus appearing as only a single increment. The correct number of > kernfs_open_files with mmapped == true were nevertheless still present > in the kernfs_open_node's files list. Consequently, the iteration in > kernfs_drain_open_files would underflow nr_mmapped and the WARNING would > fire. > > Since both nr_mmapped and nr_to_release may be incremented and > decremented from multiple CPUs simultaneously without acquiring the > kernfs_open_file_mutex_lock, make these members atomic. This should > ensure that simultaneous increments and decrements don't overwrite each > other. > > Fixes: bdb2fd7fc56e ("kernfs: Skip kernfs_drain_open_files() more aggressively") > Signed-off-by: Kevin Mitchell > --- > fs/kernfs/file.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c > index 9adf36e6364b..a9d0bf82ceba 100644 > --- a/fs/kernfs/file.c > +++ b/fs/kernfs/file.c > @@ -23,8 +23,8 @@ struct kernfs_open_node { > atomic_t event; > wait_queue_head_t poll; > struct list_head files; /* goes through kernfs_open_file.list */ > - unsigned int nr_mmapped; > - unsigned int nr_to_release; > + atomic_t nr_mmapped; > + atomic_t nr_to_release; This feels wrong, shouldn't a lock be handling the access to these files instead? Having 3 atomic_t variables in the same structure feels like overkill, just use a single lock to make the contention be less? thanks, greg k-h