Re: [PATCH 0/2] fs: make the i_size_read/write helpers be smp_load_acquire/store_release()

[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
.