Re: [PATCH 06/11] pipe: no need to confirm page cache buf

From: Al Viro
Date: Mon Sep 26 2016 - 23:41:01 EST


On Wed, Sep 14, 2016 at 10:37:11AM +0200, Miklos Szeredi wrote:

> Things could happen to that page that make it not uptodate while sitting in
> the pipe, but it's questionable whether we should care about that.
> Checking for being uptodate in the face of such page state change is always
> going to be racy.

I'm not sure it's the right thing to do here; that area looks like a victim
of serious bitrot - once upon a time it was ->map(), which used to lock
page, verity that it's valid, and kmap it. ->unmap() did kunmap + unlock.

Then the validate part got split off (->pin(), later renamed to ->confirm()),
with lock _not_ held over the kmap/kunmap. That's the point when it got racy,
AFAICS. OTOH, I would really hate to hold a page locked over e.g. copying to
userland - too easy to get deadlocks that way.

Jens, could you comment? Pages definitely shouldn't be getting into pipe
without having been uptodate; the question is what (if anything) should be
done about having a page go non-uptodate (on truncate, hole-punching, etc.)
while a reference to it is sitting in a pipe buffer.