On Wed, 14 Feb 2018, Trond Myklebust wrote:
> On Wed, 2018-02-14 at 09:01 -0500, Scott Mayhew wrote:
> > Hi Trond,
> >
> > Commit ca0daa2 ("NFS: Cache aggressively when file is open for
> > writing")
> > removed the inode revalidation from do_setlk(). Why was that
> > necessary?
> > If just that part of the commit is added back in, the client still
> > seems
> > to be able to cope with out-of-order write replies.
>
> It can cope with out of order replies, but not with changes by a second
> client, which is highly likely when you are using file locking. In that
> case, we still need to invalidate the data cache.
Couldn't do_setlk incorporate a check similar to nfs_file_has_writers
and do the revalidation if there are no writers, otherwise do the
invalidation if there are writers?
>
> > Currently the client invalidates the data cache whenever it takes a
> > lock
> > and that causes performance problems for some workloads... if a
>
> Exactly which workloads?
I'll need to get specifics on the actual workload, all that was attached
to the bugzilla was a script to illustrate the behavior, basically
flock/read/unlock in a loop (and looking at the support case attached
to the bug I don't really see any specifics either).
>
> > client
> > wants to re-read portions of a file, and no other client has
> > modified
> > that file, then why should the reads go out on the wire just because
> > locking is being used?
>
> The point of the patch is to no longer track whether or not another
> client has changed the file while the file is open. That tracking was
> producing way too many false positives for no gain, and causing heavy
> slowdowns for performance optimised workloads due to spurious cache
> invalidations. Workloads that use locking are generally _not_
> considered to be optimised for performance.
>
> IOW: The patch is restoring the behaviour of the locking code to the
> historically preferred one as described in the NFS FAQ.
> See http://nfs.sourceforge.net/#faq_a8
Doesn't that need to be reconciled with section 10.3.2 in RFCs 5661 &
7530? Maybe those need to be amended?
-Scott
>
> --
> Trond Myklebust
> Linux NFS client maintainer, PrimaryData
> trond.myklebust@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
