Re: cto changes for v4 atomic open

[Trond Myklebust <trondmy@hammerspace.com>]

On Wed, 2021-08-04 at 09:47 +1000, NeilBrown wrote:
> 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

No. What you propose is to optimise for a fringe case, which we cannot
guarantee will work anyway. I'd much rather optimise for the common
case, which is the only case with predictable semantics.


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com