On Sat, Dec 8, 2018 at 8:36 AM Darrick J. Wong <darrick.w...@oracle.com> wrote:
>
> Finally, the most important fix is to the pipe splicing code (aka the
> generic copy_file_range fallback) to avoid pointless short directio
> reads by only asking the filesystem for as much data as there are
> available pages in the pipe buffer.  Our previous fix (simulated short
> directio reads because the number of pages didn't match the length of
> the read requested) caused subtle problems on overlayfs, so that part is
> reverted.

Honestly, I really wish you simply wouldn't send me "xfs" fixes that
aren't really xfs-specific at all.

All the splice patches (and honestly, I feel some of the iomap ones
too) that have come in through the xfs tree should have been handled
separately as actual VFS patches. Or at least had acks from Al or
something.

I'm looking at that splice patch, and my initial reaction was "Hmm.
but that breaks 0 vs -EAGAIN". But then I realized that that's why
you're validating pipe->nrbufs, because the special temporary
per-thread pipe is always supposed to be empty on entry, so a zero
length can't happen.

But just the fact that I felt like I had to go and look at one of
these commits makes me go "this is not an XFS fix at all, it shouldn't
have been treated as an XFS patch, and the original commit that
*caused* the problem shouldn't have been treated as one either".

So please. I want to feel like when I get a XFS pull from the XFS
maintainer, I don't need to worry about it, and I can just do the git
pull without having to check details.

But that means that when you do changes outside of XFS code, those
changes need to be handled _differently_. And they shouldn't be
bypassing Al etc. And even if you can't get an Ack from Al, send them
separately, so that I can check them without there being any XFS
issues that are mixed up with the pull.

So the patch looks good, and I'm merging this, but I really really
don't have to feel like I need to look at xfs pulls this way.

                 Linus

Reply via email to