On Mon, 2016-07-18 at 00:32 -0400, Trond Myklebust wrote:
> Hi Christoph,
>
> >
> > On Jul 17, 2016, at 23:48, Christoph Hellwig <hch@xxxxxxxxxxxxx>
> > wrote:
> >
> > On Wed, Jul 06, 2016 at 06:30:01PM -0400, Trond Myklebust wrote:
> > >
> > > When retrieving stat() information, NFS unfortunately does
> > > require us to
> > > sync writes to disk in order to ensure that mtime and ctime are
> > > up to
> > > date. However we shouldn't have to ensure that those writes are
> > > persisted.
> > >
> > > Relaxing that requirement does mean that we may see an
> > > mtime/ctime change
> > > if the server reboots and forces us to replay all writes.
> > >
> > > The exception to this rule are pNFS clients that are required to
> > > send
> > > layoutcommit, however that is dealt with by the call to
> > > pnfs_sync_inode()
> > > in _nfs_revalidate_inode().
> >
> > This one breaks xfstests generic/207 on block/scsi layout for
> > me. The
> > reason for that is that we need a layoutcommit after writing out
> > all
> > data for the file for the file size to be updated on the server.
> >
> > Below is my attempt to fix this by re-adding pnfs_sync_inode to
> > nfs_getattr. The call in _nfs_revalidate_inode isn't enough as it
> > doesn't get called in most cases we care about.
> >
>
> I’m not understanding this argument. Why do we care if the file size
> is up to date on the server if we’re not sending an actual GETATTR on
> the wire to retrieve the file size?
>
> Cheers
> Trond
Actually... The problem might be that a previous attribute update is
marking the attribute cache as being revalidated. Does the following
patch help?
Cheers
Trond
8<-----------------------------------------------------------
>From 10b7e9ad44881fcd46ac24eb7374377c6e8962ed Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
Date: Mon, 18 Jul 2016 00:51:01 -0400
Subject: [PATCH] pNFS: Don't mark the inode as revalidated if a LAYOUTCOMMIT
is outstanding
We know that the attributes will need updating if there is still a
LAYOUTCOMMIT outstanding.
Reported-by: Christoph Hellwig <hch@xxxxxx>
Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
---
fs/nfs/inode.c | 5 ++++-
fs/nfs/pnfs.h | 7 +++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 35fda08dc4f6..9df45832e28b 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1664,7 +1664,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
unsigned long now = jiffies;
unsigned long save_cache_validity;
bool have_writers = nfs_file_has_buffered_writers(nfsi);
- bool cache_revalidated = true;
+ bool cache_revalidated;
dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%x)\n",
__func__, inode->i_sb->s_id, inode->i_ino,
@@ -1713,6 +1713,9 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
/* Do atomic weak cache consistency updates */
invalid |= nfs_wcc_update_inode(inode, fattr);
+
+ cache_revalidated = !pnfs_layoutcommit_outstanding(inode);
+
/* More cache consistency checks */
if (fattr->valid & NFS_ATTR_FATTR_CHANGE) {
if (inode->i_version != fattr->change_attr) {
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index d6be5299a55a..181283c4ebc3 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -629,6 +629,13 @@ pnfs_sync_inode(struct inode *inode, bool datasync)
}
static inline bool
+pnfs_layoutcommit_outstanding(struct inode *inode)
+{
+ return false;
+}
+
+
+static inline bool
pnfs_roc(struct inode *ino)
{
return false;
--
2.7.4
Trond Myklebust
Principal System Architect
4300 El Camino Real | Suite 100
Los Altos, CA 94022
W: 650-422-3800
C: 801-921-4583
www.primarydata.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
