Re: [patch 1/2] mm: dont clear PG_uptodate ininvalidate_complete_page2()

From: Linus Torvalds
Date: Wed Jun 25 2008 - 11:12:33 EST




On Wed, 25 Jun 2008, Miklos Szeredi wrote:
>
> Clearing the uptodate page flag will cause page_cache_pipe_buf_confirm()
> to return -ENODATA if that page was in the buffer. This in turn will cause
> splice() to return a short or zero count.

I really think we should just change splice itself at this point.

The VM people may end up removing the clearing of the uptodate bit for
other reasons, but there's no way I'll do this kind of thing which affects
unknown number of cases for just the splice kind of reason, when we could
just change the one place you care about right now - which is just
FUSE/NFSD/page_cache_pipe_buf_confirm.

I also really don't think this even fixes the problems you have with
FUSE/NFSD - because you'll still be reading zeroes for a truncated file.
Yes, you get the rigth counts, but you don't get the right data.

(And no, your previous patch that removed the asnchronous stuff and the
pipe_buf_confirm entirely didn't fix it _either_ - it's simply unavoidable
when you just pass unlocked pages around. There is no serialization with
other people doing truncates etc)

That's "correct" from a splice() kind of standpoint (it's essentially a
temporary mmap() with MAP_PRIVATE), but the thing is, it just sounds like
the whole "page went away" thing is a more fundamental issue. It sounds
like nfds should hold a read-lock on the file while it has any IO in
flight, or something like that.

IOW, I don't think your patch really is in the right area. I *do* agree
that page_cache_pipe_buf_confirm() itself probably is worth changing, but
I think you're trying to treat some of the individual symptoms here (and
not even all), and there's a more fundamental underlying issue that you're
not loooking at.

Maybe.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/