All of lore.kernel.org
 help / color / mirror / Atom feed
From: "bfields@fieldses.org" <bfields@fieldses.org>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "plambri@redhat.com" <plambri@redhat.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"bcodding@redhat.com" <bcodding@redhat.com>
Subject: Re: cto changes for v4 atomic open
Date: Tue, 3 Aug 2021 17:36:42 -0400	[thread overview]
Message-ID: <20210803213642.GA4042@fieldses.org> (raw)
In-Reply-To: <3feb71ab232b26df6d63111ee8226f6bb7e8dc36.camel@hammerspace.com>

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.

Maybe that's not practical to implement.  It'd be nice at least if that
was explicit in the documentation.

--b.

  reply	other threads:[~2021-08-03 21:36 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 13:25 cto changes for v4 atomic open Benjamin Coddington
2021-07-30 14:48 ` Trond Myklebust
2021-07-30 15:14   ` Benjamin Coddington
2021-08-03 20:30   ` J. Bruce Fields
2021-08-03 21:07     ` Trond Myklebust
2021-08-03 21:36       ` bfields [this message]
2021-08-03 21:43         ` Trond Myklebust
2021-08-03 23:47           ` NeilBrown
2021-08-04  0:00             ` Trond Myklebust
2021-08-04  0:04               ` Trond Myklebust
2021-08-04  0:57               ` NeilBrown
2021-08-04  1:03                 ` Trond Myklebust
2021-08-04  1:16                   ` bfields
2021-08-04  1:25                     ` Trond Myklebust
2021-08-04  1:30                   ` NeilBrown
2021-08-04  1:38                     ` Trond Myklebust
2021-08-09  4:20                       ` NeilBrown
2021-08-09 14:22                         ` Trond Myklebust
2021-08-09 14:43                           ` Chuck Lever III
2021-08-04  1:43         ` Matt Benjamin
2021-08-04  1:51           ` Matt Benjamin
2021-08-04  2:10             ` Trond Myklebust
2021-08-04 14:49               ` Patrick Goetz
2021-08-04 15:42                 ` Rick Macklem
2021-08-04 18:24                 ` Anna Schumaker
2021-08-06 18:58                   ` Patrick Goetz
2021-08-07  1:03                     ` Rick Macklem
2021-08-04 18:33               ` Matt Benjamin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210803213642.GA4042@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=bcodding@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=plambri@redhat.com \
    --cc=trondmy@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.