From: Trond Myklebust <trondmy@hammerspace.com>
To: "jencce.kernel@gmail.com" <jencce.kernel@gmail.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
"ltp@lists.linux.it" <ltp@lists.linux.it>
Subject: Re: nfs-for-5.3-3 update "breaks" NFSv4 directIO somehow
Date: Mon, 9 Sep 2019 02:58:35 +0000 [thread overview]
Message-ID: <77a371f6d9c290de0cca00ff272ea831e0d124b8.camel@hammerspace.com> (raw)
In-Reply-To: <20190909023600.sxygdyclxm4ivllw@XZHOUW.usersys.redhat.com>
On Mon, 2019-09-09 at 10:36 +0800, Murphy Zhou wrote:
> On Wed, Aug 28, 2019 at 03:32:25PM +0000, Trond Myklebust wrote:
> > On Wed, 2019-08-28 at 18:22 +0800, Murphy Zhou wrote:
> > > Hi,
> > >
> > > If write to file with O_DIRECT, then read it without O_DIRECT,
> > > read
> > > returns 0.
> > > From tshark output, looks like the READ call is missing.
> > >
> > > LTP[1] dio tests spot this. Things work well before this update.
> > >
> > > Bisect log is pointing to:
> > >
> > > commit 7e10cc25bfa0dd3602bbcf5cc9c759a90eb675dc
> > > Author: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > Date: Fri Aug 9 12:06:43 2019 -0400
> > >
> > > NFS: Don't refresh attributes with mounted-on-file
> > > informatio
> > >
> > > With this commit reverted, the tests pass again.
> > >
> > > It's only about NFSv4(4.0 4.1 and 4.2), NFSv3 works well.
> > >
> > > Bisect log, outputs of tshark, sample test programme derived from
> > > LTP diotest02.c and a simple test script are attached.
> > >
> > > If this is an expected change, we will need to update the
> > > testcases.
> >
> > That is not intentional, so thanks for reporting it! Does the
> > following
> > fix help?
>
> Hi Trond,
>
> Will you queue this fix for v5.3 ?
>
> Thanks!
>
It is already in 5.3-rc8:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eb3d8f42231aec65b64b079dd17bd6c008a3fe29
Cheers
Trond
> > 8<------------------------
> > From ce61618bc085d8cea8a614b5e1eb09e16ea8e036 Mon Sep 17 00:00:00
> > 2001
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > Date: Wed, 28 Aug 2019 11:26:13 -0400
> > Subject: [PATCH] NFS: Fix inode fileid checks in attribute
> > revalidation code
> >
> > We want to throw out the attrbute if it refers to the mounted on
> > fileid,
> > and not the real fileid. However we do not want to block cache
> > consistency
> > updates from NFSv4 writes.
> >
> > Reported-by: Murphy Zhou <jencce.kernel@gmail.com>
> > Fixes: 7e10cc25bfa0 ("NFS: Don't refresh attributes with mounted-
> > on-file...")
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> > fs/nfs/inode.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index c764cfe456e5..d7e78b220cf6 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -1404,10 +1404,11 @@ static int
> > nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr
> > *fat
> > return 0;
> >
> > /* No fileid? Just exit */
> > - if (!(fattr->valid & NFS_ATTR_FATTR_FILEID))
> > - return 0;
> > + if (!(fattr->valid & NFS_ATTR_FATTR_FILEID)) {
> > + if (fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID)
> > + return 0;
> > /* Has the inode gone and changed behind our back? */
> > - if (nfsi->fileid != fattr->fileid) {
> > + } else if (nfsi->fileid != fattr->fileid) {
> > /* Is this perhaps the mounted-on fileid? */
> > if ((fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID)
> > &&
> > nfsi->fileid == fattr->mounted_on_fileid)
> > @@ -1808,10 +1809,11 @@ static int nfs_update_inode(struct inode
> > *inode, struct nfs_fattr *fattr)
> > atomic_read(&inode->i_count), fattr->valid);
> >
> > /* No fileid? Just exit */
> > - if (!(fattr->valid & NFS_ATTR_FATTR_FILEID))
> > - return 0;
> > + if (!(fattr->valid & NFS_ATTR_FATTR_FILEID)) {
> > + if (fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID)
> > + return 0;
> > /* Has the inode gone and changed behind our back? */
> > - if (nfsi->fileid != fattr->fileid) {
> > + } else if (nfsi->fileid != fattr->fileid) {
> > /* Is this perhaps the mounted-on fileid? */
> > if ((fattr->valid & NFS_ATTR_FATTR_MOUNTED_ON_FILEID)
> > &&
> > nfsi->fileid == fattr->mounted_on_fileid)
> > --
> > 2.21.0
> >
> > --
> > Trond Myklebust
> > Linux NFS client maintainer, Hammerspace
> > trond.myklebust@hammerspace.com
> >
> >
Trond Myklebust
CTO, Hammerspace Inc
4300 El Camino Real, Suite 105
Los Altos, CA 94022
www.hammer.space
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com
next prev parent reply other threads:[~2019-09-09 2:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-28 10:22 nfs-for-5.3-3 update "breaks" NFSv4 directIO somehow Murphy Zhou
2019-08-28 15:32 ` Trond Myklebust
2019-08-29 0:06 ` Murphy Zhou
2019-09-09 2:36 ` Murphy Zhou
2019-09-09 2:58 ` Trond Myklebust [this message]
2019-09-09 3:19 ` Murphy Zhou
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=77a371f6d9c290de0cca00ff272ea831e0d124b8.camel@hammerspace.com \
--to=trondmy@hammerspace.com \
--cc=jencce.kernel@gmail.com \
--cc=linux-nfs@vger.kernel.org \
--cc=ltp@lists.linux.it \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).