2016-12-04 23:10:11

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH] NFS: Fix incorrect size revalidation when holding a delegation

We should only care about checking the attributes if the page cache
is marked as dubious (using NFS_INO_REVAL_PAGECACHE) and the
NFS_INO_REVAL_FORCED flag is set.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/file.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 9ea85ae23c32..64c11f399b3d 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -102,8 +102,11 @@ static int nfs_revalidate_file_size(struct inode *inode, struct file *filp)
{
struct nfs_server *server = NFS_SERVER(inode);
struct nfs_inode *nfsi = NFS_I(inode);
+ const unsigned long force_reval = NFS_INO_REVAL_PAGECACHE|NFS_INO_REVAL_FORCED;
+ unsigned long cache_validity = nfsi->cache_validity;

- if (nfs_have_delegated_attributes(inode))
+ if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ) &&
+ (cache_validity & force_reval) != force_reval)
goto out_noreval;

if (filp->f_flags & O_DIRECT)
--
2.9.3



2016-12-05 20:28:14

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH] NFS: Fix incorrect size revalidation when holding a delegation

Hi Trond,

On 12/04/2016 06:10 PM, Trond Myklebust wrote:
> We should only care about checking the attributes if the page cache
> is marked as dubious (using NFS_INO_REVAL_PAGECACHE) and the
> NFS_INO_REVAL_FORCED flag is set.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/file.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 9ea85ae23c32..64c11f399b3d 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -102,8 +102,11 @@ static int nfs_revalidate_file_size(struct inode *inode, struct file *filp)
> {
> struct nfs_server *server = NFS_SERVER(inode);
> struct nfs_inode *nfsi = NFS_I(inode);
> + const unsigned long force_reval = NFS_INO_REVAL_PAGECACHE|NFS_INO_REVAL_FORCED;

Would it make sense to declare this in a header file somewhere, rather than repeating this in file.c and inode.c? (and any other places we might need to "force_reval" in the future?)

Anna

> + unsigned long cache_validity = nfsi->cache_validity;
>
> - if (nfs_have_delegated_attributes(inode))
> + if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ) &&
> + (cache_validity & force_reval) != force_reval)
> goto out_noreval;
>
> if (filp->f_flags & O_DIRECT)
>

2016-12-05 21:12:28

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] NFS: Fix incorrect size revalidation when holding a delegation

DQo+IE9uIERlYyA1LCAyMDE2LCBhdCAxNToyOCwgQW5uYSBTY2h1bWFrZXIgPHNjaHVtYWtlci5h
bm5hQGdtYWlsLmNvbT4gd3JvdGU6DQo+IA0KPiBIaSBUcm9uZCwNCj4gDQo+IE9uIDEyLzA0LzIw
MTYgMDY6MTAgUE0sIFRyb25kIE15a2xlYnVzdCB3cm90ZToNCj4+IFdlIHNob3VsZCBvbmx5IGNh
cmUgYWJvdXQgY2hlY2tpbmcgdGhlIGF0dHJpYnV0ZXMgaWYgdGhlIHBhZ2UgY2FjaGUNCj4+IGlz
IG1hcmtlZCBhcyBkdWJpb3VzICh1c2luZyBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRSkgYW5kIHRo
ZQ0KPj4gTkZTX0lOT19SRVZBTF9GT1JDRUQgZmxhZyBpcyBzZXQuDQo+PiANCj4+IFNpZ25lZC1v
ZmYtYnk6IFRyb25kIE15a2xlYnVzdCA8dHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbT4N
Cj4+IC0tLQ0KPj4gZnMvbmZzL2ZpbGUuYyB8IDUgKysrKy0NCj4+IDEgZmlsZSBjaGFuZ2VkLCA0
IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkNCj4+IA0KPj4gZGlmZiAtLWdpdCBhL2ZzL25m
cy9maWxlLmMgYi9mcy9uZnMvZmlsZS5jDQo+PiBpbmRleCA5ZWE4NWFlMjNjMzIuLjY0YzExZjM5
OWIzZCAxMDA2NDQNCj4+IC0tLSBhL2ZzL25mcy9maWxlLmMNCj4+ICsrKyBiL2ZzL25mcy9maWxl
LmMNCj4+IEBAIC0xMDIsOCArMTAyLDExIEBAIHN0YXRpYyBpbnQgbmZzX3JldmFsaWRhdGVfZmls
ZV9zaXplKHN0cnVjdCBpbm9kZSAqaW5vZGUsIHN0cnVjdCBmaWxlICpmaWxwKQ0KPj4gew0KPj4g
CXN0cnVjdCBuZnNfc2VydmVyICpzZXJ2ZXIgPSBORlNfU0VSVkVSKGlub2RlKTsNCj4+IAlzdHJ1
Y3QgbmZzX2lub2RlICpuZnNpID0gTkZTX0koaW5vZGUpOw0KPj4gKwljb25zdCB1bnNpZ25lZCBs
b25nIGZvcmNlX3JldmFsID0gTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEV8TkZTX0lOT19SRVZBTF9G
T1JDRUQ7DQo+IA0KPiBXb3VsZCBpdCBtYWtlIHNlbnNlIHRvIGRlY2xhcmUgdGhpcyBpbiBhIGhl
YWRlciBmaWxlIHNvbWV3aGVyZSwgcmF0aGVyIHRoYW4gcmVwZWF0aW5nIHRoaXMgaW4gZmlsZS5j
IGFuZCBpbm9kZS5jPyAoYW5kIGFueSBvdGhlciBwbGFjZXMgd2UgbWlnaHQgbmVlZCB0byAiZm9y
Y2VfcmV2YWwiIGluIHRoZSBmdXR1cmU/KQ0KDQpJIGhhdmUgc29tZSBpZGVhcyBmb3Igc29tZSBj
bGVhbnVwcyB0aGF0IHNob3VsZCBoZWxwIGNsZWFyIHVwIHRoZSBzaXR1YXRpb24gd2l0aCB0aGUg
YXR0cmlidXRlIGNhY2hlLiBJ4oCZbGwgc2VlIGlmIEkgZ2V0IHJvdW5kIHRvIHRoZW0gdGhpcyB3
ZWVrLg0KDQpDaGVlcnMNCiAgVHJvbmQ=