mbox series

[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
Series fs/file.c: optimize the critical section of file_lock in | expand

Message

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(-)

Comments

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 :)
Jan Kara Nov. 6, 2024, 5:44 p.m. UTC | #9
On Mon 12-08-24 03:40:44, 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

Al, it seems you didn't push the patches to Linus during the last merge
window. Do you plan to push them during the coming one?

								Honza
Al Viro Nov. 6, 2024, 5:59 p.m. UTC | #10
On Wed, Nov 06, 2024 at 06:44:23PM +0100, Jan Kara wrote:
> On Mon 12-08-24 03:40:44, 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
> 
> Al, it seems you didn't push the patches to Linus during the last merge
> window. Do you plan to push them during the coming one?

Yes, I do.  I can merge that branch into viro/vfs.git#for-next, but that would be
redundant - note that vfs/vfs.git#workd.fdtable is identical to it (same sha1 of
head) and vfs/vfs.git#vfs.file contains a merge from it and vfs/vfs.git#vfs.all
merges #vfs.file.  So it's already in linux-next in the current form and until
something else gets added to the branch I see no point.