Re: cto changes for v4 atomic open

["NeilBrown" <neilb@suse.de>]

On Wed, 04 Aug 2021, Trond Myklebust wrote:
> On Tue, 2021-08-03 at 17:36 -0400, bfields@fieldses.org wrote:
> > On Tue, Aug 03, 2021 at 09:07:11PM +0000, Trond Myklebust wrote:
> > > On Tue, 2021-08-03 at 16:30 -0400, J. Bruce Fields wrote:
> > > > On Fri, Jul 30, 2021 at 02:48:41PM +0000, Trond Myklebust wrote:
> > > > > On Fri, 2021-07-30 at 09:25 -0400, Benjamin Coddington wrote:
> > > > > > I have some folks unhappy about behavior changes after:
> > > > > > 479219218fbe
> > > > > > NFS:
> > > > > > Optimise away the close-to-open GETATTR when we have NFSv4
> > > > > > OPEN
> > > > > > 
> > > > > > Before this change, a client holding a RO open would
> > > > > > invalidate
> > > > > > the
> > > > > > pagecache when doing a second RW open.
> > > > > > 
> > > > > > Now the client doesn't invalidate the pagecache, though
> > > > > > technically
> > > > > > it could
> > > > > > because we see a changeattr update on the RW OPEN response.
> > > > > > 
> > > > > > I feel this is a grey area in CTO if we're already holding an
> > > > > > open. 
> > > > > > Do we
> > > > > > know how the client ought to behave in this case?  Should the
> > > > > > client's open
> > > > > > upgrade to RW invalidate the pagecache?
> > > > > > 
> > > > > 
> > > > > It's not a "grey area in close-to-open" at all. It is very cut
> > > > > and
> > > > > dried.
> > > > > 
> > > > > If you need to invalidate your page cache while the file is
> > > > > open,
> > > > > then
> > > > > by definition you are in a situation where there is a write by
> > > > > another
> > > > > client going on while you are reading. You're clearly not doing
> > > > > close-
> > > > > to-open.
> > > > 
> > > > Documentation is really unclear about this case.  Every
> > > > definition of
> > > > close-to-open that I've seen says that it requires a cache
> > > > consistency
> > > > check on every application open.  I've never seen one that says
> > > > "on
> > > > every open that doesn't overlap with an already-existing open on
> > > > that
> > > > client".
> > > > 
> > > > They *usually* also preface that by saying that this is motivated
> > > > by
> > > > the
> > > > use case where opens don't overlap.  But it's never made clear
> > > > that
> > > > that's part of the definition.
> > > > 
> > > 
> > > I'm not following your logic.
> > 
> > It's just a question of what every source I can find says close-to-
> > open
> > means.  E.g., NFS Illustrated, p. 248, "Close-to-open consistency
> > provides a guarantee of cache consistency at the level of file opens
> > and
> > closes.  When a file is closed by an application, the client flushes
> > any
> > cached changs to the server.  When a file is opened, the client
> > ignores
> > any cache time remaining (if the file data are cached) and makes an
> > explicit GETATTR call to the server to check the file modification
> > time."
> > 
> > > The close-to-open model assumes that the file is only being
> > > modified by
> > > one client at a time and it assumes that file contents may be
> > > cached
> > > while an application is holding it open.
> > > The point checks exist in order to detect if the file is being
> > > changed
> > > when the file is not open.
> > > 
> > > Linux does not have a per-application cache. It has a page cache
> > > that
> > > is shared among all applications. It is impossible for two
> > > applications
> > > to open the same file using buffered I/O, and yet see different
> > > contents.
> > 
> > Right, so based on the descriptions like the one above, I would have
> > expected both applications to see new data at that point.
> 
> Why? That would be a clear violation of the close-to-open rule that
> nobody else can write to the file while it is open.
> 

Is the rule
A -  "it is not permitted for any other application/client to write to
      the file while another has it open"
 or
B -  "it is not expected for any other application/client to write to
      the file while another has it open"

I think B, because A is clearly not enforced.  That suggests that there
is no *need* to check for changes, but equally there is no barrier to
checking for changes.  So that fact that one application has the file
open should not prevent a check when another application opens the file.
Equally it should not prevent a flush when some other application closes
the file.

It is somewhat weird that if an application on one client misbehaves by
keeping a file open, that will prevent other applications on the same
client from seeing non-local changes, but will not prevent applications
on other clients from seeing any changes.

NeilBrown