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@xxxxxxxxxxxxxxx> > > > 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@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxx> > > Fixes: 7e10cc25bfa0 ("NFS: Don't refresh attributes with mounted- > > on-file...") > > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxx > > > > 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@xxxxxxxxxxxxxxx
