linux-kernel - Re: [PATCH 0/2] fs: make the i_size_read/write helpers be smp_load_acquire/store

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 24 Jan 2024 16:06:13 +0800
From: Baokun Li <libaokun1@...wei.com>
To: Jan Kara <jack@...e.cz>, Christian Brauner <brauner@...nel.org>
CC: <torvalds@...ux-foundation.org>, <viro@...iv.linux.org.uk>,
	<willy@...radead.org>, <akpm@...ux-foundation.org>,
	<linux-kernel@...r.kernel.org>, <yi.zhang@...wei.com>,
	<yangerkun@...wei.com>, <yukuai3@...wei.com>,
	<linux-fsdevel@...r.kernel.org>, Baokun Li <libaokun1@...wei.com>
Subject: Re: [PATCH 0/2] fs: make the i_size_read/write helpers be
 smp_load_acquire/store_release()

On 2024/1/24 2:56, Jan Kara wrote:
> On Mon 22-01-24 12:14:52, Christian Brauner wrote:
>> On Mon, 22 Jan 2024 17:45:34 +0800, Baokun Li wrote:
>>> This patchset follows the linus suggestion to make the i_size_read/write
>>> helpers be smp_load_acquire/store_release(), after which the extra smp_rmb
>>> in filemap_read() is no longer needed, so it is removed.
>>>
>>> Functional tests were performed and no new problems were found.
>>>
>>> Here are the results of unixbench tests based on 6.7.0-next-20240118 on
>>> arm64, with some degradation in single-threading and some optimization in
>>> multi-threading, but overall the impact is not significant.
>>>
>>> [...]
>> Hm, we can certainly try but I wouldn't rule it out that someone will
>> complain aobut the "non-significant" degradation in single-threading.
>> We'll see. Let that performance bot chew on it for a bit as well.
> Yeah, over 5% regression in buffered read/write cost is a bit hard to
> swallow. I somewhat wonder why this is so much - maybe people call
> i_size_read() without thinking too much and now it becomes atomic op on
> arm? Also LKP tests only on x86 (where these changes are going to be
> for noop) and I'm not sure anybody else runs performance tests on
> linux-next, even less so on ARM... So not sure anybody will complain until
> this gets into some distro (such as Android).
>
>> But I agree that the smp_load_acquire()/smp_store_release() is clearer
>> than the open-coded smp_rmb().
> Agreed, conceptually this is nice and it will also silence some KCSAN
> warnings about i_size updates vs reads.
>
> 								Honza
Hello Honza!

Are there any other performance tests you'd like to perform?
I can test it on my machine if you have any.

Cheers!
-- 
With Best Regards,
Baokun Li
.