Messages in this thread | | | Date | Wed, 24 Jan 2024 16:06:13 +0800 | Subject | Re: [PATCH 0/2] fs: make the i_size_read/write helpers be smp_load_acquire/store_release() | From | Baokun Li <> |
| |
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 .
| |