| Message ID | 1453853442-16777-1-git-send-email-kamal@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 01/27/16 01:10, Kamal Mostafa wrote: > This is a note to let you know that I have just added a patch titled > > NFS: Fix attribute cache revalidation > > to the linux-4.2.y-queue branch of the 4.2.y-ckt extended stable tree > which can be found at: > > http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-4.2.y-queue > > This patch is scheduled to be released in version 4.2.8-ckt3. > > If you, or anyone else, feels it should not be added to this tree, please > reply to this email. You might consider to include NFS: Ensure we revalidate attributes before using execute_ok() NFSv4: Don't perform cached access checks before we've OPENed the file as well, as in combination they fix https://bugzilla.kernel.org/show_bug.cgi?id=109771 Regards Donald > > For more information about the 4.2.y-ckt tree, see > https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable > > Thanks. > -Kamal > > ---8<------------------------------------------------------------ > > From 37c95c36efa1173b25a53241362f100ed2e3baa8 Mon Sep 17 00:00:00 2001 > From: Trond Myklebust <trond.myklebust@primarydata.com> > Date: Tue, 29 Dec 2015 18:55:19 -0500 > Subject: NFS: Fix attribute cache revalidation > > commit ade14a7df796d4e86bd9d181193c883a57b13db0 upstream. > > If a NFSv4 client uses the cache_consistency_bitmask in order to > request only information about the change attribute, timestamps and > size, then it has not revalidated all attributes, and hence the > attribute timeout timestamp should not be updated. > > Reported-by: Donald Buczek <buczek@molgen.mpg.de> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > fs/nfs/inode.c | 54 +++++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 39 insertions(+), 15 deletions(-) > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index 1c65e3f..7bf2c65 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -1630,6 +1630,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > unsigned long invalid = 0; > unsigned long now = jiffies; > unsigned long save_cache_validity; > + bool cache_revalidated = true; > > 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, > @@ -1691,22 +1692,28 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > nfs_force_lookup_revalidate(inode); > inode->i_version = fattr->change_attr; > } > - } else > + } else { > nfsi->cache_validity |= save_cache_validity; > + cache_revalidated = false; > + } > > if (fattr->valid & NFS_ATTR_FATTR_MTIME) { > memcpy(&inode->i_mtime, &fattr->mtime, sizeof(inode->i_mtime)); > - } else if (server->caps & NFS_CAP_MTIME) > + } else if (server->caps & NFS_CAP_MTIME) { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATTR > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > if (fattr->valid & NFS_ATTR_FATTR_CTIME) { > memcpy(&inode->i_ctime, &fattr->ctime, sizeof(inode->i_ctime)); > - } else if (server->caps & NFS_CAP_CTIME) > + } else if (server->caps & NFS_CAP_CTIME) { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATTR > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > /* Check if our cached file size is stale */ > if (fattr->valid & NFS_ATTR_FATTR_SIZE) { > @@ -1726,19 +1733,23 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > (long long)cur_isize, > (long long)new_isize); > } > - } else > + } else { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATTR > | NFS_INO_REVAL_PAGECACHE > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > > if (fattr->valid & NFS_ATTR_FATTR_ATIME) > memcpy(&inode->i_atime, &fattr->atime, sizeof(inode->i_atime)); > - else if (server->caps & NFS_CAP_ATIME) > + else if (server->caps & NFS_CAP_ATIME) { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATIME > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > if (fattr->valid & NFS_ATTR_FATTR_MODE) { > if ((inode->i_mode & S_IALLUGO) != (fattr->mode & S_IALLUGO)) { > @@ -1747,36 +1758,42 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > inode->i_mode = newmode; > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; > } > - } else if (server->caps & NFS_CAP_MODE) > + } else if (server->caps & NFS_CAP_MODE) { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATTR > | NFS_INO_INVALID_ACCESS > | NFS_INO_INVALID_ACL > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > if (fattr->valid & NFS_ATTR_FATTR_OWNER) { > if (!uid_eq(inode->i_uid, fattr->uid)) { > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; > inode->i_uid = fattr->uid; > } > - } else if (server->caps & NFS_CAP_OWNER) > + } else if (server->caps & NFS_CAP_OWNER) { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATTR > | NFS_INO_INVALID_ACCESS > | NFS_INO_INVALID_ACL > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > if (fattr->valid & NFS_ATTR_FATTR_GROUP) { > if (!gid_eq(inode->i_gid, fattr->gid)) { > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; > inode->i_gid = fattr->gid; > } > - } else if (server->caps & NFS_CAP_OWNER_GROUP) > + } else if (server->caps & NFS_CAP_OWNER_GROUP) { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATTR > | NFS_INO_INVALID_ACCESS > | NFS_INO_INVALID_ACL > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > if (fattr->valid & NFS_ATTR_FATTR_NLINK) { > if (inode->i_nlink != fattr->nlink) { > @@ -1785,19 +1802,22 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > invalid |= NFS_INO_INVALID_DATA; > set_nlink(inode, fattr->nlink); > } > - } else if (server->caps & NFS_CAP_NLINK) > + } else if (server->caps & NFS_CAP_NLINK) { > nfsi->cache_validity |= save_cache_validity & > (NFS_INO_INVALID_ATTR > | NFS_INO_REVAL_FORCED); > + cache_revalidated = false; > + } > > if (fattr->valid & NFS_ATTR_FATTR_SPACE_USED) { > /* > * report the blocks in 512byte units > */ > inode->i_blocks = nfs_calc_block_size(fattr->du.nfs3.used); > - } > - if (fattr->valid & NFS_ATTR_FATTR_BLOCKS_USED) > + } else if (fattr->valid & NFS_ATTR_FATTR_BLOCKS_USED) > inode->i_blocks = fattr->du.nfs2.blocks; > + else > + cache_revalidated = false; > > /* Update attrtimeo value if we're out of the unstable period */ > if (invalid & NFS_INO_INVALID_ATTR) { > @@ -1807,9 +1827,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > /* Set barrier to be more recent than all outstanding updates */ > nfsi->attr_gencount = nfs_inc_attr_generation_counter(); > } else { > - if (!time_in_range_open(now, nfsi->attrtimeo_timestamp, nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { > - if ((nfsi->attrtimeo <<= 1) > NFS_MAXATTRTIMEO(inode)) > - nfsi->attrtimeo = NFS_MAXATTRTIMEO(inode); > + if (cache_revalidated) { > + if (!time_in_range_open(now, nfsi->attrtimeo_timestamp, > + nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { > + nfsi->attrtimeo <<= 1; > + if (nfsi->attrtimeo > NFS_MAXATTRTIMEO(inode)) > + nfsi->attrtimeo = NFS_MAXATTRTIMEO(inode); > + } > nfsi->attrtimeo_timestamp = now; > } > /* Set the barrier to be more recent than this fattr */ > @@ -1818,7 +1842,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > } > > /* Don't declare attrcache up to date if there were no attrs! */ > - if (fattr->valid != 0) > + if (cache_revalidated) > invalid &= ~NFS_INO_INVALID_ATTR; > > /* Don't invalidate the data if we were to blame */ > -- > 1.9.1 >
On Wed, 2016-01-27 at 08:26 +0100, Donald Buczek wrote: > On 01/27/16 01:10, Kamal Mostafa wrote: > > This is a note to let you know that I have just added a patch titled > > > > NFS: Fix attribute cache revalidation > > > > to the linux-4.2.y-queue branch of the 4.2.y-ckt extended stable tree > You might consider to include > > NFS: Ensure we revalidate attributes before using execute_ok() > NFSv4: Don't perform cached access checks before we've OPENed the file > > as well, as in combination they fix > https://bugzilla.kernel.org/show_bug.cgi?id=109771 Thanks very much Donald. Yes, I'll queue up those two for 4.2-stable also. -Kamal
On Wed, Jan 27, 2016 at 08:26:45AM +0100, Donald Buczek wrote: > On 01/27/16 01:10, Kamal Mostafa wrote: > >This is a note to let you know that I have just added a patch titled > > > > NFS: Fix attribute cache revalidation > > > >to the linux-4.2.y-queue branch of the 4.2.y-ckt extended stable tree > >which can be found at: > > > > http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-4.2.y-queue > > > >This patch is scheduled to be released in version 4.2.8-ckt3. > > > >If you, or anyone else, feels it should not be added to this tree, please > >reply to this email. > > You might consider to include > > NFS: Ensure we revalidate attributes before using execute_ok() > NFSv4: Don't perform cached access checks before we've OPENed the file > > as well, as in combination they fix > https://bugzilla.kernel.org/show_bug.cgi?id=109771 > These 2 extra commits seem relevant to the 3.16 kernel as well. I'll queue them for the next release. Cheers, -- Luís > Regards > > Donald > > > > > >For more information about the 4.2.y-ckt tree, see > >https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable > > > >Thanks. > >-Kamal > > > >---8<------------------------------------------------------------ > > > > From 37c95c36efa1173b25a53241362f100ed2e3baa8 Mon Sep 17 00:00:00 2001 > >From: Trond Myklebust <trond.myklebust@primarydata.com> > >Date: Tue, 29 Dec 2015 18:55:19 -0500 > >Subject: NFS: Fix attribute cache revalidation > > > >commit ade14a7df796d4e86bd9d181193c883a57b13db0 upstream. > > > >If a NFSv4 client uses the cache_consistency_bitmask in order to > >request only information about the change attribute, timestamps and > >size, then it has not revalidated all attributes, and hence the > >attribute timeout timestamp should not be updated. > > > >Reported-by: Donald Buczek <buczek@molgen.mpg.de> > >Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > >Signed-off-by: Kamal Mostafa <kamal@canonical.com> > >--- > > fs/nfs/inode.c | 54 +++++++++++++++++++++++++++++++++++++++--------------- > > 1 file changed, 39 insertions(+), 15 deletions(-) > > > >diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > >index 1c65e3f..7bf2c65 100644 > >--- a/fs/nfs/inode.c > >+++ b/fs/nfs/inode.c > >@@ -1630,6 +1630,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > > unsigned long invalid = 0; > > unsigned long now = jiffies; > > unsigned long save_cache_validity; > >+ bool cache_revalidated = true; > > > > 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, > >@@ -1691,22 +1692,28 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > > nfs_force_lookup_revalidate(inode); > > inode->i_version = fattr->change_attr; > > } > >- } else > >+ } else { > > nfsi->cache_validity |= save_cache_validity; > >+ cache_revalidated = false; > >+ } > > > > if (fattr->valid & NFS_ATTR_FATTR_MTIME) { > > memcpy(&inode->i_mtime, &fattr->mtime, sizeof(inode->i_mtime)); > >- } else if (server->caps & NFS_CAP_MTIME) > >+ } else if (server->caps & NFS_CAP_MTIME) { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATTR > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > if (fattr->valid & NFS_ATTR_FATTR_CTIME) { > > memcpy(&inode->i_ctime, &fattr->ctime, sizeof(inode->i_ctime)); > >- } else if (server->caps & NFS_CAP_CTIME) > >+ } else if (server->caps & NFS_CAP_CTIME) { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATTR > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > /* Check if our cached file size is stale */ > > if (fattr->valid & NFS_ATTR_FATTR_SIZE) { > >@@ -1726,19 +1733,23 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > > (long long)cur_isize, > > (long long)new_isize); > > } > >- } else > >+ } else { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATTR > > | NFS_INO_REVAL_PAGECACHE > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > > > if (fattr->valid & NFS_ATTR_FATTR_ATIME) > > memcpy(&inode->i_atime, &fattr->atime, sizeof(inode->i_atime)); > >- else if (server->caps & NFS_CAP_ATIME) > >+ else if (server->caps & NFS_CAP_ATIME) { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATIME > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > if (fattr->valid & NFS_ATTR_FATTR_MODE) { > > if ((inode->i_mode & S_IALLUGO) != (fattr->mode & S_IALLUGO)) { > >@@ -1747,36 +1758,42 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > > inode->i_mode = newmode; > > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; > > } > >- } else if (server->caps & NFS_CAP_MODE) > >+ } else if (server->caps & NFS_CAP_MODE) { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATTR > > | NFS_INO_INVALID_ACCESS > > | NFS_INO_INVALID_ACL > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > if (fattr->valid & NFS_ATTR_FATTR_OWNER) { > > if (!uid_eq(inode->i_uid, fattr->uid)) { > > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; > > inode->i_uid = fattr->uid; > > } > >- } else if (server->caps & NFS_CAP_OWNER) > >+ } else if (server->caps & NFS_CAP_OWNER) { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATTR > > | NFS_INO_INVALID_ACCESS > > | NFS_INO_INVALID_ACL > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > if (fattr->valid & NFS_ATTR_FATTR_GROUP) { > > if (!gid_eq(inode->i_gid, fattr->gid)) { > > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; > > inode->i_gid = fattr->gid; > > } > >- } else if (server->caps & NFS_CAP_OWNER_GROUP) > >+ } else if (server->caps & NFS_CAP_OWNER_GROUP) { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATTR > > | NFS_INO_INVALID_ACCESS > > | NFS_INO_INVALID_ACL > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > if (fattr->valid & NFS_ATTR_FATTR_NLINK) { > > if (inode->i_nlink != fattr->nlink) { > >@@ -1785,19 +1802,22 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > > invalid |= NFS_INO_INVALID_DATA; > > set_nlink(inode, fattr->nlink); > > } > >- } else if (server->caps & NFS_CAP_NLINK) > >+ } else if (server->caps & NFS_CAP_NLINK) { > > nfsi->cache_validity |= save_cache_validity & > > (NFS_INO_INVALID_ATTR > > | NFS_INO_REVAL_FORCED); > >+ cache_revalidated = false; > >+ } > > > > if (fattr->valid & NFS_ATTR_FATTR_SPACE_USED) { > > /* > > * report the blocks in 512byte units > > */ > > inode->i_blocks = nfs_calc_block_size(fattr->du.nfs3.used); > >- } > >- if (fattr->valid & NFS_ATTR_FATTR_BLOCKS_USED) > >+ } else if (fattr->valid & NFS_ATTR_FATTR_BLOCKS_USED) > > inode->i_blocks = fattr->du.nfs2.blocks; > >+ else > >+ cache_revalidated = false; > > > > /* Update attrtimeo value if we're out of the unstable period */ > > if (invalid & NFS_INO_INVALID_ATTR) { > >@@ -1807,9 +1827,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > > /* Set barrier to be more recent than all outstanding updates */ > > nfsi->attr_gencount = nfs_inc_attr_generation_counter(); > > } else { > >- if (!time_in_range_open(now, nfsi->attrtimeo_timestamp, nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { > >- if ((nfsi->attrtimeo <<= 1) > NFS_MAXATTRTIMEO(inode)) > >- nfsi->attrtimeo = NFS_MAXATTRTIMEO(inode); > >+ if (cache_revalidated) { > >+ if (!time_in_range_open(now, nfsi->attrtimeo_timestamp, > >+ nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { > >+ nfsi->attrtimeo <<= 1; > >+ if (nfsi->attrtimeo > NFS_MAXATTRTIMEO(inode)) > >+ nfsi->attrtimeo = NFS_MAXATTRTIMEO(inode); > >+ } > > nfsi->attrtimeo_timestamp = now; > > } > > /* Set the barrier to be more recent than this fattr */ > >@@ -1818,7 +1842,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) > > } > > > > /* Don't declare attrcache up to date if there were no attrs! */ > >- if (fattr->valid != 0) > >+ if (cache_revalidated) > > invalid &= ~NFS_INO_INVALID_ATTR; > > > > /* Don't invalidate the data if we were to blame */ > >-- > >1.9.1 > > > > > -- > Donald Buczek > buczek@molgen.mpg.de > Tel: +49 30 8413 1433 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 1c65e3f..7bf2c65 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1630,6 +1630,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) unsigned long invalid = 0; unsigned long now = jiffies; unsigned long save_cache_validity; + bool cache_revalidated = true; 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, @@ -1691,22 +1692,28 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) nfs_force_lookup_revalidate(inode); inode->i_version = fattr->change_attr; } - } else + } else { nfsi->cache_validity |= save_cache_validity; + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_MTIME) { memcpy(&inode->i_mtime, &fattr->mtime, sizeof(inode->i_mtime)); - } else if (server->caps & NFS_CAP_MTIME) + } else if (server->caps & NFS_CAP_MTIME) { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATTR | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_CTIME) { memcpy(&inode->i_ctime, &fattr->ctime, sizeof(inode->i_ctime)); - } else if (server->caps & NFS_CAP_CTIME) + } else if (server->caps & NFS_CAP_CTIME) { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATTR | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } /* Check if our cached file size is stale */ if (fattr->valid & NFS_ATTR_FATTR_SIZE) { @@ -1726,19 +1733,23 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) (long long)cur_isize, (long long)new_isize); } - } else + } else { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATTR | NFS_INO_REVAL_PAGECACHE | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_ATIME) memcpy(&inode->i_atime, &fattr->atime, sizeof(inode->i_atime)); - else if (server->caps & NFS_CAP_ATIME) + else if (server->caps & NFS_CAP_ATIME) { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_MODE) { if ((inode->i_mode & S_IALLUGO) != (fattr->mode & S_IALLUGO)) { @@ -1747,36 +1758,42 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) inode->i_mode = newmode; invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; } - } else if (server->caps & NFS_CAP_MODE) + } else if (server->caps & NFS_CAP_MODE) { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATTR | NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_OWNER) { if (!uid_eq(inode->i_uid, fattr->uid)) { invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; inode->i_uid = fattr->uid; } - } else if (server->caps & NFS_CAP_OWNER) + } else if (server->caps & NFS_CAP_OWNER) { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATTR | NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_GROUP) { if (!gid_eq(inode->i_gid, fattr->gid)) { invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; inode->i_gid = fattr->gid; } - } else if (server->caps & NFS_CAP_OWNER_GROUP) + } else if (server->caps & NFS_CAP_OWNER_GROUP) { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATTR | NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_NLINK) { if (inode->i_nlink != fattr->nlink) { @@ -1785,19 +1802,22 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) invalid |= NFS_INO_INVALID_DATA; set_nlink(inode, fattr->nlink); } - } else if (server->caps & NFS_CAP_NLINK) + } else if (server->caps & NFS_CAP_NLINK) { nfsi->cache_validity |= save_cache_validity & (NFS_INO_INVALID_ATTR | NFS_INO_REVAL_FORCED); + cache_revalidated = false; + } if (fattr->valid & NFS_ATTR_FATTR_SPACE_USED) { /* * report the blocks in 512byte units */ inode->i_blocks = nfs_calc_block_size(fattr->du.nfs3.used); - } - if (fattr->valid & NFS_ATTR_FATTR_BLOCKS_USED) + } else if (fattr->valid & NFS_ATTR_FATTR_BLOCKS_USED) inode->i_blocks = fattr->du.nfs2.blocks; + else + cache_revalidated = false; /* Update attrtimeo value if we're out of the unstable period */ if (invalid & NFS_INO_INVALID_ATTR) { @@ -1807,9 +1827,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) /* Set barrier to be more recent than all outstanding updates */ nfsi->attr_gencount = nfs_inc_attr_generation_counter(); } else { - if (!time_in_range_open(now, nfsi->attrtimeo_timestamp, nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { - if ((nfsi->attrtimeo <<= 1) > NFS_MAXATTRTIMEO(inode)) - nfsi->attrtimeo = NFS_MAXATTRTIMEO(inode); + if (cache_revalidated) { + if (!time_in_range_open(now, nfsi->attrtimeo_timestamp, + nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { + nfsi->attrtimeo <<= 1; + if (nfsi->attrtimeo > NFS_MAXATTRTIMEO(inode)) + nfsi->attrtimeo = NFS_MAXATTRTIMEO(inode); + } nfsi->attrtimeo_timestamp = now; } /* Set the barrier to be more recent than this fattr */ @@ -1818,7 +1842,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) } /* Don't declare attrcache up to date if there were no attrs! */ - if (fattr->valid != 0) + if (cache_revalidated) invalid &= ~NFS_INO_INVALID_ATTR; /* Don't invalidate the data if we were to blame */
This is a note to let you know that I have just added a patch titled NFS: Fix attribute cache revalidation to the linux-4.2.y-queue branch of the 4.2.y-ckt extended stable tree which can be found at: http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-4.2.y-queue This patch is scheduled to be released in version 4.2.8-ckt3. If you, or anyone else, feels it should not be added to this tree, please reply to this email. For more information about the 4.2.y-ckt tree, see https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable Thanks. -Kamal ---8<------------------------------------------------------------ From 37c95c36efa1173b25a53241362f100ed2e3baa8 Mon Sep 17 00:00:00 2001 From: Trond Myklebust <trond.myklebust@primarydata.com> Date: Tue, 29 Dec 2015 18:55:19 -0500 Subject: NFS: Fix attribute cache revalidation commit ade14a7df796d4e86bd9d181193c883a57b13db0 upstream. If a NFSv4 client uses the cache_consistency_bitmask in order to request only information about the change attribute, timestamps and size, then it has not revalidated all attributes, and hence the attribute timeout timestamp should not be updated. Reported-by: Donald Buczek <buczek@molgen.mpg.de> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> Signed-off-by: Kamal Mostafa <kamal@canonical.com> --- fs/nfs/inode.c | 54 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 15 deletions(-) -- 1.9.1