[v5,0/3] fs/file.c: optimize the critical section of file_lock in

Message ID	20240717145018.3972922-1-yu.ma@intel.com (mailing list archive)
Headers	show Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.11]) (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 45BCC11CA1; Wed, 17 Jul 2024 14:24:19 +0000 (UTC) From: Yu Ma <yu.ma@intel.com> To: brauner@kernel.org, jack@suse.cz, mjguzik@gmail.com, edumazet@google.com Cc: yu.ma@intel.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, pan.deng@intel.com, tianyou.li@intel.com, tim.c.chen@intel.com, tim.c.chen@linux.intel.com, viro@zeniv.linux.org.uk Subject: [PATCH v5 0/3] fs/file.c: optimize the critical section of file_lock in Date: Wed, 17 Jul 2024 10:50:15 -0400 Message-ID: <20240717145018.3972922-1-yu.ma@intel.com> In-Reply-To: <20240614163416.728752-1-yu.ma@intel.com> References: <20240614163416.728752-1-yu.ma@intel.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	fs/file.c: optimize the critical section of file_lock in \| expand [v5,0/3] fs/file.c: optimize the critical section of file_lock in [v5,1/3] fs/file.c: remove sanity_check and add likely/unlikely in alloc_fd() [v5,2/3] fs/file.c: conditionally clear full_fds [v5,3/3] fs/file.c: add fast path in find_next_fd()

Ma, Yu July 17, 2024, 2:50 p.m. UTC

pts/blogbench-1.1.0 is a benchmark designed to replicate the
load of a real-world busy file server by multiple threads of
random reads, writes, and rewrites. When running default configuration
with multiple parallel threads, hot spin lock contention is observed
from alloc_fd(), file_closed_fd() and put_unused_fd() around file_lock.

These 3 patches are created to reduce the critical section of file_lock
in alloc_fd() and close_fd(). As a result, on top of patch 1,
pts/blogbench-1.1.0 has been improved by 22% for read and 8% for write
on Intel ICX 160 cores configuration with v6.10-rc7.

v4 -> v5:
Revised patch 3 for some code style issues.

v3 -> v4:
1. Rebased the patch set to v6.10-rc7 and updated performance results.
2. Updated patch 1 to revise the order of fd checking.
3. As proposed by Mateusz Guzik <mjguzik gmail.com>, and agreed by Jan Kara
<jack@suse.cz> and Tim Chen <tim.c.chen@linux.intel.com>, updated patch 3 with
a more generic and scalable fast path for the word contains next_fd.

v2 -> v3:
1. Rebased the patch set to latest v6.10-rc6 and updated the performance results
2. Reordered the patches as suggested by Mateusz Guzik <mjguzik gmail.com>
3. Updated the fast path from alloc_fd() to find_next_fd() as suggested by
Mateusz Guzik <mjguzik gmail.com> and Jan Kara <jack@suse.cz>, it is efficient
and more concise than v2.

v1 -> v2:
1. Rebased the patch set to latest v6.10-rc4 and updated the performance results
2. Fixed the bug identified by Mateusz Guzik in patch 1 with adding rlimit check
for fast path
3. Updated patch 3 to remove sanity_check directly per the alignment with
maintainer

Yu Ma (3):
  fs/file.c: remove sanity_check and add likely/unlikely in alloc_fd()
  fs/file.c: conditionally clear full_fds
  fs/file.c: add fast path in find_next_fd()

 fs/file.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

Christian Brauner July 22, 2024, 3:02 p.m. UTC | #1

On Wed, 17 Jul 2024 10:50:15 -0400, Yu Ma wrote:
> pts/blogbench-1.1.0 is a benchmark designed to replicate the
> load of a real-world busy file server by multiple threads of
> random reads, writes, and rewrites. When running default configuration
> with multiple parallel threads, hot spin lock contention is observed
> from alloc_fd(), file_closed_fd() and put_unused_fd() around file_lock.
> 
> These 3 patches are created to reduce the critical section of file_lock
> in alloc_fd() and close_fd(). As a result, on top of patch 1,
> pts/blogbench-1.1.0 has been improved by 22% for read and 8% for write
> on Intel ICX 160 cores configuration with v6.10-rc7.
> 
> [...]

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[1/3] fs/file.c: remove sanity_check and add likely/unlikely in alloc_fd()
      https://git.kernel.org/vfs/vfs/c/19f4a3f5712a
[2/3] fs/file.c: conditionally clear full_fds
      https://git.kernel.org/vfs/vfs/c/b483266658a8
[3/3] fs/file.c: add fast path in find_next_fd()
      https://git.kernel.org/vfs/vfs/c/3603a42c8c03

Al Viro Aug. 1, 2024, 7:13 p.m. UTC | #2

On Mon, Jul 22, 2024 at 05:02:04PM +0200, Christian Brauner wrote:

> Applied to the vfs.misc branch of the vfs/vfs.git tree.
> Patches in the vfs.misc branch should appear in linux-next soon.
> 
> Please report any outstanding bugs that were missed during review in a
> new review to the original patch series allowing us to drop it.
> 
> It's encouraged to provide Acked-bys and Reviewed-bys even though the
> patch has now been applied. If possible patch trailers will be updated.
> 
> Note that commit hashes shown below are subject to change due to rebase,
> trailer updates or similar. If in doubt, please check the listed branch.
> 
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
> branch: vfs.misc
> 
> [1/3] fs/file.c: remove sanity_check and add likely/unlikely in alloc_fd()
>       https://git.kernel.org/vfs/vfs/c/19f4a3f5712a
> [2/3] fs/file.c: conditionally clear full_fds
>       https://git.kernel.org/vfs/vfs/c/b483266658a8
> [3/3] fs/file.c: add fast path in find_next_fd()
>       https://git.kernel.org/vfs/vfs/c/3603a42c8c03

Hmm...   Something fishy's going on - those are not reachable by any branches.
As the matter of fact, none of the branches contain _anything_ recent in
fs/file.c or include/linux/fdtable.h; the only thing I see for include/linux/file.h
is (dubious, IMO) DEFINE_FREE(fput,...) - this IS_ERR_OR_NULL in there needs
a review of potential users.

I'm putting together (in viro/vfs.git) a branch for that area (#work.fdtable)
and I'm going to apply those 3 unless anyone objects.

Christian Brauner Aug. 2, 2024, 11:04 a.m. UTC | #3

> Hmm...   Something fishy's going on - those are not reachable by any branches.

Hm, they probably got dropped when rebasing to v6.11-rc1 and I did have
to play around with --onto.

> I'm putting together (in viro/vfs.git) a branch for that area (#work.fdtable)
> and I'm going to apply those 3 unless anyone objects.

Fine since they aren't in that branch. Otherwise I generally prefer to
just merge a common branch. But that's rarely needed since Linus handles
merge conflicts anyway and doesn't mind.

Al Viro Aug. 2, 2024, 2:22 p.m. UTC | #4

On Fri, Aug 02, 2024 at 01:04:44PM +0200, Christian Brauner wrote:
> > Hmm...   Something fishy's going on - those are not reachable by any branches.
> 
> Hm, they probably got dropped when rebasing to v6.11-rc1 and I did have
> to play around with --onto.
> 
> > I'm putting together (in viro/vfs.git) a branch for that area (#work.fdtable)
> > and I'm going to apply those 3 unless anyone objects.
> 
> Fine since they aren't in that branch. Otherwise I generally prefer to
> just merge a common branch.

If it's going to be rebased anyway, I don't see much difference from cherry-pick,
TBH...

Christian Brauner Aug. 5, 2024, 6:56 a.m. UTC | #5

On Fri, Aug 02, 2024 at 03:22:48PM GMT, Al Viro wrote:
> On Fri, Aug 02, 2024 at 01:04:44PM +0200, Christian Brauner wrote:
> > > Hmm...   Something fishy's going on - those are not reachable by any branches.
> > 
> > Hm, they probably got dropped when rebasing to v6.11-rc1 and I did have
> > to play around with --onto.
> > 
> > > I'm putting together (in viro/vfs.git) a branch for that area (#work.fdtable)
> > > and I'm going to apply those 3 unless anyone objects.
> > 
> > Fine since they aren't in that branch. Otherwise I generally prefer to
> > just merge a common branch.
> 
> If it's going to be rebased anyway, I don't see much difference from cherry-pick,
> TBH...

Yeah, but I generally don't rebase after -rc1 anymore unles there's
really annoying conflicts.

Ma, Yu Aug. 12, 2024, 1:31 a.m. UTC | #6

On 8/5/2024 2:56 PM, Christian Brauner wrote:
> On Fri, Aug 02, 2024 at 03:22:48PM GMT, Al Viro wrote:
>> On Fri, Aug 02, 2024 at 01:04:44PM +0200, Christian Brauner wrote:
>>>> Hmm...   Something fishy's going on - those are not reachable by any branches.
>>> Hm, they probably got dropped when rebasing to v6.11-rc1 and I did have
>>> to play around with --onto.
>>>
>>>> I'm putting together (in viro/vfs.git) a branch for that area (#work.fdtable)
>>>> and I'm going to apply those 3 unless anyone objects.
>>> Fine since they aren't in that branch. Otherwise I generally prefer to
>>> just merge a common branch.
>> If it's going to be rebased anyway, I don't see much difference from cherry-pick,
>> TBH...
> Yeah, but I generally don't rebase after -rc1 anymore unles there's
> really annoying conflicts.

Thanks Christian and Al for your time and efforts. I'm not familiar with 
the merging process, may i know about when these patches could be seen 
in master ?


Regards

Yu

Al Viro Aug. 12, 2024, 2:40 a.m. UTC | #7

On Mon, Aug 12, 2024 at 09:31:17AM +0800, Ma, Yu wrote:
> 
> On 8/5/2024 2:56 PM, Christian Brauner wrote:
> > On Fri, Aug 02, 2024 at 03:22:48PM GMT, Al Viro wrote:
> > > On Fri, Aug 02, 2024 at 01:04:44PM +0200, Christian Brauner wrote:
> > > > > Hmm...   Something fishy's going on - those are not reachable by any branches.
> > > > Hm, they probably got dropped when rebasing to v6.11-rc1 and I did have
> > > > to play around with --onto.
> > > > 
> > > > > I'm putting together (in viro/vfs.git) a branch for that area (#work.fdtable)
> > > > > and I'm going to apply those 3 unless anyone objects.
> > > > Fine since they aren't in that branch. Otherwise I generally prefer to
> > > > just merge a common branch.
> > > If it's going to be rebased anyway, I don't see much difference from cherry-pick,
> > > TBH...
> > Yeah, but I generally don't rebase after -rc1 anymore unles there's
> > really annoying conflicts.
> 
> Thanks Christian and Al for your time and efforts. I'm not familiar with the
> merging process, may i know about when these patches could be seen in master

It's in work.fdtable in my tree, will post that series tonight and add to #for-next

Ma, Yu Aug. 12, 2024, 3:09 p.m. UTC | #8

On 8/12/2024 10:40 AM, Al Viro wrote:
> On Mon, Aug 12, 2024 at 09:31:17AM +0800, Ma, Yu wrote:
>> On 8/5/2024 2:56 PM, Christian Brauner wrote:
>>> On Fri, Aug 02, 2024 at 03:22:48PM GMT, Al Viro wrote:
>>>> On Fri, Aug 02, 2024 at 01:04:44PM +0200, Christian Brauner wrote:
>>>>>> Hmm...   Something fishy's going on - those are not reachable by any branches.
>>>>> Hm, they probably got dropped when rebasing to v6.11-rc1 and I did have
>>>>> to play around with --onto.
>>>>>
>>>>>> I'm putting together (in viro/vfs.git) a branch for that area (#work.fdtable)
>>>>>> and I'm going to apply those 3 unless anyone objects.
>>>>> Fine since they aren't in that branch. Otherwise I generally prefer to
>>>>> just merge a common branch.
>>>> If it's going to be rebased anyway, I don't see much difference from cherry-pick,
>>>> TBH...
>>> Yeah, but I generally don't rebase after -rc1 anymore unles there's
>>> really annoying conflicts.
>> Thanks Christian and Al for your time and efforts. I'm not familiar with the
>> merging process, may i know about when these patches could be seen in master
> It's in work.fdtable in my tree, will post that series tonight and add to #for-next

Thanks Al for confirmation :)

[v5,0/3] fs/file.c: optimize the critical section of file_lock in

Message

Comments