On Fri, 15 Nov 2024 at 12:42, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> I don't think you need it:
D'oh!
I think I was even aware of that originally, but now that I rebased it
I just did a mindless "what's the difference between the fast-path and
the slow path" thing.
The other case I noticed - but ignored - is this part of the slow case:
/*
* i_size must be checked after we know the pages are Uptodate.
*
* Checking i_size after the check allows us to calculate
* the correct value for "nr", which means the zero-filled
* part of the page is not copied back to userspace (unless
* another truncate extends the file - this is desired though).
*/
isize = i_size_read(inode);
if (unlikely(iocb->ki_pos >= isize))
goto put_folios;
...
/*
* If users can be writing to this folio using arbitrary
* virtual addresses, take care of potential aliasing
* before reading the folio on the kernel side.
*/
if (writably_mapped)
flush_dcache_folio(folio);
but I thought that the truncate case shouldn't matter - because a
truncate can still happen *during* the copy - and the second case is
not relevant on cache-coherent architectures.
In fact, now I looked at generic/095 some more, and as far as I can
see, that fio script doesn't do any small reads at all!
Which makes me even more confused about how it was affected by this
change? Funky funky.
But maybe that file size check. Because clearly there *is* some
difference here, even if it has worked for me and it all looks
ObviouslyCorrect(tm).
Oh well. Maybe in another 9 months I'll figure it out. I'll keep the
patch in my pile of random patches.
Linus
