* [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
@ 2012-12-10 14:25 Jeff Layton
2012-12-10 14:42 ` Myklebust, Trond
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2012-12-10 14:25 UTC (permalink / raw)
To: trond.myklebust; +Cc: jiali, linux-nfs
Jian reported that the following sequence would leave "testfile" with
corrupt data:
# mount localhost:/export /mnt/nfs/ -o vers=3
# echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
# cat -v /export/testfile
abc
^@^@^@^@ghi
While there's no locking involved here, the operations are serialized,
so CTO should prevent corruption.
The first write to the file is fine and writes 4 bytes. The file is then
extended on the server. When it's reopened a GETATTR is issued and the
size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
the file. Because the file is opened for write only,
nfs_want_read_modify_write() returns 0 to nfs_write_begin().
nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
still set on the file at that point, but that flag is ignored and
nfs_pageuptodate erroneously extends the write to cover the whole page,
with the write done on the server side filled in with zeroes.
This patch just has that function check for NFS_INO_INVALID_DATA in
addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
over the code, I wonder if we might have a similar bug in
nfs_revalidate_size(). The difference between those two flags is very
subtle, so it seems like we ought to be checking for
NFS_INO_INVALID_DATA in most of the places that we look for
NFS_INO_REVAL_PAGECACHE.
I believe this is regression introduced by commit 8d197a568. The code
did check for NFS_INO_INVALID_DATA prior to that patch.
Original bug report is here:
https://bugzilla.redhat.com/show_bug.cgi?id=885743
Cc: <stable@vger.kernel.org> # 3.5+
Reported-by: Jian Li <jiali@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/write.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 9347ab7..83aec9b 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -884,7 +884,7 @@ static bool nfs_write_pageuptodate(struct page *page, struct inode *inode)
{
if (nfs_have_delegated_attributes(inode))
goto out;
- if (NFS_I(inode)->cache_validity & NFS_INO_REVAL_PAGECACHE)
+ if (NFS_I(inode)->cache_validity & (NFS_INO_INVALID_DATA|NFS_INO_REVAL_PAGECACHE))
return false;
out:
return PageUptodate(page) != 0;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 14:25 [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid Jeff Layton
@ 2012-12-10 14:42 ` Myklebust, Trond
2012-12-10 16:05 ` Jeff Layton
0 siblings, 1 reply; 12+ messages in thread
From: Myklebust, Trond @ 2012-12-10 14:42 UTC (permalink / raw)
To: Jeff Layton; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
T24gTW9uLCAyMDEyLTEyLTEwIGF0IDA5OjI1IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
SmlhbiByZXBvcnRlZCB0aGF0IHRoZSBmb2xsb3dpbmcgc2VxdWVuY2Ugd291bGQgbGVhdmUgInRl
c3RmaWxlIiB3aXRoDQo+IGNvcnJ1cHQgZGF0YToNCj4gDQo+ICAgICAjIG1vdW50IGxvY2FsaG9z
dDovZXhwb3J0IC9tbnQvbmZzLyAtbyB2ZXJzPTMNCj4gICAgICMgZWNobyBhYmMgPiAvbW50L25m
cy90ZXN0ZmlsZTsgZWNobyBkZWYgPj4gL2V4cG9ydC90ZXN0ZmlsZTsgZWNobyBnaGkgPj4gL21u
dC9uZnMvdGVzdGZpbGUNCj4gICAgICMgY2F0IC12IC9leHBvcnQvdGVzdGZpbGUNCj4gICAgIGFi
Yw0KPiAgICAgXkBeQF5AXkBnaGkNCj4gDQo+IFdoaWxlIHRoZXJlJ3Mgbm8gbG9ja2luZyBpbnZv
bHZlZCBoZXJlLCB0aGUgb3BlcmF0aW9ucyBhcmUgc2VyaWFsaXplZCwNCj4gc28gQ1RPIHNob3Vs
ZCBwcmV2ZW50IGNvcnJ1cHRpb24uDQo+IA0KPiBUaGUgZmlyc3Qgd3JpdGUgdG8gdGhlIGZpbGUg
aXMgZmluZSBhbmQgd3JpdGVzIDQgYnl0ZXMuIFRoZSBmaWxlIGlzIHRoZW4NCj4gZXh0ZW5kZWQg
b24gdGhlIHNlcnZlci4gV2hlbiBpdCdzIHJlb3BlbmVkIGEgR0VUQVRUUiBpcyBpc3N1ZWQgYW5k
IHRoZQ0KPiBzaXplIGNoYW5nZSBpcyBub3RpY2VkLiBUaGlzIGNhdXNlcyBORlNfSU5PX0lOVkFM
SURfREFUQSB0byBiZSBzZXQgb24NCj4gdGhlIGZpbGUuIEJlY2F1c2UgdGhlIGZpbGUgaXMgb3Bl
bmVkIGZvciB3cml0ZSBvbmx5LA0KPiBuZnNfd2FudF9yZWFkX21vZGlmeV93cml0ZSgpIHJldHVy
bnMgMCB0byBuZnNfd3JpdGVfYmVnaW4oKS4NCj4gbmZzX3VwZGF0ZXBhZ2UgdGhlbiBjYWxscyBu
ZnNfd3JpdGVfcGFnZXVwdG9kYXRlKCkgdG8gc2VlIGlmIGl0IHNob3VsZA0KPiBleHRlbmQgdGhl
IG5mc19wYWdlIHRvIGNvdmVyIHRoZSB3aG9sZSBwYWdlLiBORlNfSU5PX0lOVkFMSURfREFUQSBp
cw0KPiBzdGlsbCBzZXQgb24gdGhlIGZpbGUgYXQgdGhhdCBwb2ludCwgYnV0IHRoYXQgZmxhZyBp
cyBpZ25vcmVkIGFuZA0KPiBuZnNfcGFnZXVwdG9kYXRlIGVycm9uZW91c2x5IGV4dGVuZHMgdGhl
IHdyaXRlIHRvIGNvdmVyIHRoZSB3aG9sZSBwYWdlLA0KPiB3aXRoIHRoZSB3cml0ZSBkb25lIG9u
IHRoZSBzZXJ2ZXIgc2lkZSBmaWxsZWQgaW4gd2l0aCB6ZXJvZXMuDQo+IA0KPiBUaGlzIHBhdGNo
IGp1c3QgaGFzIHRoYXQgZnVuY3Rpb24gY2hlY2sgZm9yIE5GU19JTk9fSU5WQUxJRF9EQVRBIGlu
DQo+IGFkZGl0aW9uIHRvIE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFLiBUaGlzIGZpeGVzIHRoZSBi
dWcsIGJ1dCBsb29raW5nDQo+IG92ZXIgdGhlIGNvZGUsIEkgd29uZGVyIGlmIHdlIG1pZ2h0IGhh
dmUgYSBzaW1pbGFyIGJ1ZyBpbg0KPiBuZnNfcmV2YWxpZGF0ZV9zaXplKCkuIFRoZSBkaWZmZXJl
bmNlIGJldHdlZW4gdGhvc2UgdHdvIGZsYWdzIGlzIHZlcnkNCj4gc3VidGxlLCBzbyBpdCBzZWVt
cyBsaWtlIHdlIG91Z2h0IHRvIGJlIGNoZWNraW5nIGZvcg0KPiBORlNfSU5PX0lOVkFMSURfREFU
QSBpbiBtb3N0IG9mIHRoZSBwbGFjZXMgdGhhdCB3ZSBsb29rIGZvcg0KPiBORlNfSU5PX1JFVkFM
X1BBR0VDQUNIRS4NCj4gDQo+IEkgYmVsaWV2ZSB0aGlzIGlzIHJlZ3Jlc3Npb24gaW50cm9kdWNl
ZCBieSBjb21taXQgOGQxOTdhNTY4LiBUaGUgY29kZQ0KPiBkaWQgY2hlY2sgZm9yIE5GU19JTk9f
SU5WQUxJRF9EQVRBIHByaW9yIHRvIHRoYXQgcGF0Y2guDQo+IA0KPiBPcmlnaW5hbCBidWcgcmVw
b3J0IGlzIGhlcmU6DQo+IA0KPiAgICAgaHR0cHM6Ly9idWd6aWxsYS5yZWRoYXQuY29tL3Nob3df
YnVnLmNnaT9pZD04ODU3NDMNCg0KSGkgSmVmZiwNCg0KVGhlIHBvaW50IG9mIE5GU19JTk9fUkVW
QUxfUEFHRUNBQ0hFIGlzIHRvIGJlIGFibGUgdG8gZGlzdGluZ3Vpc2gNCmJldHdlZW4gc2l0dWF0
aW9ucyB3aGVyZSB0aGUgZmlsZSBzaXplIG9yIGNoYW5nZSBhdHRyaWJ1dGUgaXMgaW4gZG91YnQN
CihhbmQgc28gdGhlIHBhZ2UgY2FjaGUgdmFsaWRpdHkgaXMgaW4gZG91YnQpLCBhbmQgc2l0dWF0
aW9ucyB3aGVyZSBvdGhlcg0KYXR0cmlidXRlcyBhcmUgaW4gZG91YnQgKHN1Y2ggYXMgQUNDRVNT
IGNhY2hlcywgZmlsZSBvd25lciwgbmxpbmtzLC4uLikuDQpUaGlzIGlzIHdoeSB3ZSdyZSBvbmx5
IGNoZWNraW5nIGZvciBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRSBhbmQgbm90DQpORlNfSU5PX0lO
VkFMSURfREFUQSBpbiB0aGUgZnVuY3Rpb25zIHlvdSBtZW50aW9uIGFib3ZlLg0KDQpTbyB0aGUg
Zml4IGhlcmUgc2hvdWxkIGJlIHRvIHNldCBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRSB3aGVuIHRo
ZSBjaGFuZ2UNCmF0dHJpYnV0ZSBhbmQvb3Igc2l6ZSBjaGFuZ2UgaXMgbm90aWNlZC4gVGhlIG9u
bHkgZnVuY3Rpb24gSSBjYW4gc2VlDQp0aGF0IGFwcGVhcnMgdG8gZ2V0IHRoaXMgd3JvbmcgaXMg
bmZzX3djY191cGRhdGVfaW5vZGUoKS4gRG9lcyBmaXhpbmcNCnRoYXQgbGF5IHRoZSBidWcgdG8g
cmVzdD8NCg0KQ2hlZXJzDQogIFRyb25kDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZT
IGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20N
Cnd3dy5uZXRhcHAuY29tDQo=
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 14:42 ` Myklebust, Trond
@ 2012-12-10 16:05 ` Jeff Layton
2012-12-10 16:28 ` Myklebust, Trond
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2012-12-10 16:05 UTC (permalink / raw)
To: Myklebust, Trond; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
On Mon, 10 Dec 2012 14:42:23 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> On Mon, 2012-12-10 at 09:25 -0500, Jeff Layton wrote:
> > Jian reported that the following sequence would leave "testfile" with
> > corrupt data:
> >
> > # mount localhost:/export /mnt/nfs/ -o vers=3
> > # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
> > # cat -v /export/testfile
> > abc
> > ^@^@^@^@ghi
> >
> > While there's no locking involved here, the operations are serialized,
> > so CTO should prevent corruption.
> >
> > The first write to the file is fine and writes 4 bytes. The file is then
> > extended on the server. When it's reopened a GETATTR is issued and the
> > size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
> > the file. Because the file is opened for write only,
> > nfs_want_read_modify_write() returns 0 to nfs_write_begin().
> > nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
> > extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
> > still set on the file at that point, but that flag is ignored and
> > nfs_pageuptodate erroneously extends the write to cover the whole page,
> > with the write done on the server side filled in with zeroes.
> >
> > This patch just has that function check for NFS_INO_INVALID_DATA in
> > addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
> > over the code, I wonder if we might have a similar bug in
> > nfs_revalidate_size(). The difference between those two flags is very
> > subtle, so it seems like we ought to be checking for
> > NFS_INO_INVALID_DATA in most of the places that we look for
> > NFS_INO_REVAL_PAGECACHE.
> >
> > I believe this is regression introduced by commit 8d197a568. The code
> > did check for NFS_INO_INVALID_DATA prior to that patch.
> >
> > Original bug report is here:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=885743
>
> Hi Jeff,
>
> The point of NFS_INO_REVAL_PAGECACHE is to be able to distinguish
> between situations where the file size or change attribute is in doubt
> (and so the page cache validity is in doubt), and situations where other
> attributes are in doubt (such as ACCESS caches, file owner, nlinks,...).
> This is why we're only checking for NFS_INO_REVAL_PAGECACHE and not
> NFS_INO_INVALID_DATA in the functions you mention above.
>
Ok, now I'm really confused... I thought that:
NFS_INO_INVALID_DATA == pagecache data was known to be wrong
NFS_INO_REVAL_PAGECACHE == pagecache data is questionable
...but if I understand what you're saying above, the "data" that
NFS_INO_INVALID_DATA refers to is in peripheral caches, like the ACCESS
cache? I guess then I don't quite understand what NFS_INO_INVALID_ATTR
and NFS_INO_INVALID_ACCESS are for. Aren't they supposed to indicate that
the attrs and access caches are invalid?
> So the fix here should be to set NFS_INO_REVAL_PAGECACHE when the change
> attribute and/or size change is noticed. The only function I can see
> that appears to get this wrong is nfs_wcc_update_inode(). Does fixing
> that lay the bug to rest?
>
No, that doesn't fix it. There's another place that misses setting that
flag too in nfs_update_inode() where the size changes. I added it there
too and that also didn't fix it. Here's why (I think):
When the size change is noticed via the CTO GETATTR call, then my
patched kernel now sets NFS_INO_REVAL_PAGECACHE in nfs_update_inode.
Now cache_validity is set to 0x2a. The kernel then makes an ACCESS call
that also ultimately ends up in nfs_update_inode.
nfs_update_inode saves off the cache_validity flags and clears all of
them except for NFS_INO_INVALID_DATA. As best I can tell, the
saved_cache_validity is then ending up discarded and on exit the
cache_validity is ending up set to just NFS_INO_INVALID_DATA.
It seems like we ought to be restoring the save_cache_validity flags
before exiting that function, but I confess that I don't quite grasp
the logic in nfs_update_inode. Thoughts?
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 16:05 ` Jeff Layton
@ 2012-12-10 16:28 ` Myklebust, Trond
2012-12-10 16:53 ` Jeff Layton
0 siblings, 1 reply; 12+ messages in thread
From: Myklebust, Trond @ 2012-12-10 16:28 UTC (permalink / raw)
To: Jeff Layton; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
T24gTW9uLCAyMDEyLTEyLTEwIGF0IDExOjA1IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxNDo0MjoyMyArMDAwMA0KPiAiTXlrbGVidXN0LCBUcm9uZCIg
PFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+ID4gT24gTW9uLCAyMDEy
LTEyLTEwIGF0IDA5OjI1IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4gPiA+IEppYW4gcmVw
b3J0ZWQgdGhhdCB0aGUgZm9sbG93aW5nIHNlcXVlbmNlIHdvdWxkIGxlYXZlICJ0ZXN0ZmlsZSIg
d2l0aA0KPiA+ID4gY29ycnVwdCBkYXRhOg0KPiA+ID4gDQo+ID4gPiAgICAgIyBtb3VudCBsb2Nh
bGhvc3Q6L2V4cG9ydCAvbW50L25mcy8gLW8gdmVycz0zDQo+ID4gPiAgICAgIyBlY2hvIGFiYyA+
IC9tbnQvbmZzL3Rlc3RmaWxlOyBlY2hvIGRlZiA+PiAvZXhwb3J0L3Rlc3RmaWxlOyBlY2hvIGdo
aSA+PiAvbW50L25mcy90ZXN0ZmlsZQ0KPiA+ID4gICAgICMgY2F0IC12IC9leHBvcnQvdGVzdGZp
bGUNCj4gPiA+ICAgICBhYmMNCj4gPiA+ICAgICBeQF5AXkBeQGdoaQ0KPiA+ID4gDQo+ID4gPiBX
aGlsZSB0aGVyZSdzIG5vIGxvY2tpbmcgaW52b2x2ZWQgaGVyZSwgdGhlIG9wZXJhdGlvbnMgYXJl
IHNlcmlhbGl6ZWQsDQo+ID4gPiBzbyBDVE8gc2hvdWxkIHByZXZlbnQgY29ycnVwdGlvbi4NCj4g
PiA+IA0KPiA+ID4gVGhlIGZpcnN0IHdyaXRlIHRvIHRoZSBmaWxlIGlzIGZpbmUgYW5kIHdyaXRl
cyA0IGJ5dGVzLiBUaGUgZmlsZSBpcyB0aGVuDQo+ID4gPiBleHRlbmRlZCBvbiB0aGUgc2VydmVy
LiBXaGVuIGl0J3MgcmVvcGVuZWQgYSBHRVRBVFRSIGlzIGlzc3VlZCBhbmQgdGhlDQo+ID4gPiBz
aXplIGNoYW5nZSBpcyBub3RpY2VkLiBUaGlzIGNhdXNlcyBORlNfSU5PX0lOVkFMSURfREFUQSB0
byBiZSBzZXQgb24NCj4gPiA+IHRoZSBmaWxlLiBCZWNhdXNlIHRoZSBmaWxlIGlzIG9wZW5lZCBm
b3Igd3JpdGUgb25seSwNCj4gPiA+IG5mc193YW50X3JlYWRfbW9kaWZ5X3dyaXRlKCkgcmV0dXJu
cyAwIHRvIG5mc193cml0ZV9iZWdpbigpLg0KPiA+ID4gbmZzX3VwZGF0ZXBhZ2UgdGhlbiBjYWxs
cyBuZnNfd3JpdGVfcGFnZXVwdG9kYXRlKCkgdG8gc2VlIGlmIGl0IHNob3VsZA0KPiA+ID4gZXh0
ZW5kIHRoZSBuZnNfcGFnZSB0byBjb3ZlciB0aGUgd2hvbGUgcGFnZS4gTkZTX0lOT19JTlZBTElE
X0RBVEEgaXMNCj4gPiA+IHN0aWxsIHNldCBvbiB0aGUgZmlsZSBhdCB0aGF0IHBvaW50LCBidXQg
dGhhdCBmbGFnIGlzIGlnbm9yZWQgYW5kDQo+ID4gPiBuZnNfcGFnZXVwdG9kYXRlIGVycm9uZW91
c2x5IGV4dGVuZHMgdGhlIHdyaXRlIHRvIGNvdmVyIHRoZSB3aG9sZSBwYWdlLA0KPiA+ID4gd2l0
aCB0aGUgd3JpdGUgZG9uZSBvbiB0aGUgc2VydmVyIHNpZGUgZmlsbGVkIGluIHdpdGggemVyb2Vz
Lg0KPiA+ID4gDQo+ID4gPiBUaGlzIHBhdGNoIGp1c3QgaGFzIHRoYXQgZnVuY3Rpb24gY2hlY2sg
Zm9yIE5GU19JTk9fSU5WQUxJRF9EQVRBIGluDQo+ID4gPiBhZGRpdGlvbiB0byBORlNfSU5PX1JF
VkFMX1BBR0VDQUNIRS4gVGhpcyBmaXhlcyB0aGUgYnVnLCBidXQgbG9va2luZw0KPiA+ID4gb3Zl
ciB0aGUgY29kZSwgSSB3b25kZXIgaWYgd2UgbWlnaHQgaGF2ZSBhIHNpbWlsYXIgYnVnIGluDQo+
ID4gPiBuZnNfcmV2YWxpZGF0ZV9zaXplKCkuIFRoZSBkaWZmZXJlbmNlIGJldHdlZW4gdGhvc2Ug
dHdvIGZsYWdzIGlzIHZlcnkNCj4gPiA+IHN1YnRsZSwgc28gaXQgc2VlbXMgbGlrZSB3ZSBvdWdo
dCB0byBiZSBjaGVja2luZyBmb3INCj4gPiA+IE5GU19JTk9fSU5WQUxJRF9EQVRBIGluIG1vc3Qg
b2YgdGhlIHBsYWNlcyB0aGF0IHdlIGxvb2sgZm9yDQo+ID4gPiBORlNfSU5PX1JFVkFMX1BBR0VD
QUNIRS4NCj4gPiA+IA0KPiA+ID4gSSBiZWxpZXZlIHRoaXMgaXMgcmVncmVzc2lvbiBpbnRyb2R1
Y2VkIGJ5IGNvbW1pdCA4ZDE5N2E1NjguIFRoZSBjb2RlDQo+ID4gPiBkaWQgY2hlY2sgZm9yIE5G
U19JTk9fSU5WQUxJRF9EQVRBIHByaW9yIHRvIHRoYXQgcGF0Y2guDQo+ID4gPiANCj4gPiA+IE9y
aWdpbmFsIGJ1ZyByZXBvcnQgaXMgaGVyZToNCj4gPiA+IA0KPiA+ID4gICAgIGh0dHBzOi8vYnVn
emlsbGEucmVkaGF0LmNvbS9zaG93X2J1Zy5jZ2k/aWQ9ODg1NzQzDQo+ID4gDQo+ID4gSGkgSmVm
ZiwNCj4gPiANCj4gPiBUaGUgcG9pbnQgb2YgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgaXMgdG8g
YmUgYWJsZSB0byBkaXN0aW5ndWlzaA0KPiA+IGJldHdlZW4gc2l0dWF0aW9ucyB3aGVyZSB0aGUg
ZmlsZSBzaXplIG9yIGNoYW5nZSBhdHRyaWJ1dGUgaXMgaW4gZG91YnQNCj4gPiAoYW5kIHNvIHRo
ZSBwYWdlIGNhY2hlIHZhbGlkaXR5IGlzIGluIGRvdWJ0KSwgYW5kIHNpdHVhdGlvbnMgd2hlcmUg
b3RoZXINCj4gPiBhdHRyaWJ1dGVzIGFyZSBpbiBkb3VidCAoc3VjaCBhcyBBQ0NFU1MgY2FjaGVz
LCBmaWxlIG93bmVyLCBubGlua3MsLi4uKS4NCj4gPiBUaGlzIGlzIHdoeSB3ZSdyZSBvbmx5IGNo
ZWNraW5nIGZvciBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRSBhbmQgbm90DQo+ID4gTkZTX0lOT19J
TlZBTElEX0RBVEEgaW4gdGhlIGZ1bmN0aW9ucyB5b3UgbWVudGlvbiBhYm92ZS4NCj4gPiANCj4g
DQo+IE9rLCBub3cgSSdtIHJlYWxseSBjb25mdXNlZC4uLiBJIHRob3VnaHQgdGhhdDoNCj4gDQo+
IE5GU19JTk9fSU5WQUxJRF9EQVRBID09IHBhZ2VjYWNoZSBkYXRhIHdhcyBrbm93biB0byBiZSB3
cm9uZw0KPiBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRSA9PSBwYWdlY2FjaGUgZGF0YSBpcyBxdWVz
dGlvbmFibGUNCg0KVGhlcmUgaXMgbm8gImtub3duIHRvIGJlIHdyb25nIiBzdGF0ZS4gQm90aCBt
ZWFuICJuZWVkcyByZXZhbGlkYXRpb24iLg0KDQo+IC4uLmJ1dCBpZiBJIHVuZGVyc3RhbmQgd2hh
dCB5b3UncmUgc2F5aW5nIGFib3ZlLCB0aGUgImRhdGEiIHRoYXQNCj4gTkZTX0lOT19JTlZBTElE
X0RBVEEgcmVmZXJzIHRvIGlzIGluIHBlcmlwaGVyYWwgY2FjaGVzLCBsaWtlIHRoZSBBQ0NFU1MN
Cj4gY2FjaGU/IEkgZ3Vlc3MgdGhlbiBJIGRvbid0IHF1aXRlIHVuZGVyc3RhbmQgd2hhdCBORlNf
SU5PX0lOVkFMSURfQVRUUg0KPiBhbmQgTkZTX0lOT19JTlZBTElEX0FDQ0VTUyBhcmUgZm9yLiBB
cmVuJ3QgdGhleSBzdXBwb3NlZCB0byBpbmRpY2F0ZSB0aGF0DQo+IHRoZSBhdHRycyBhbmQgYWNj
ZXNzIGNhY2hlcyBhcmUgaW52YWxpZD8NCg0KTkZTX0lOT19JTlZBTElEX0FUVFIgYmFzaWNhbGx5
IG1lYW5zIHRoYXQgc29tZSBhc3BlY3Qgb2YgdGhlIGZpbGUNCm1ldGFkYXRhIG5lZWRzIHJldmFs
aWRhdGlvbiwgYW5kIHNvIGEgY2FsbCB0byBuZnNfcmV2YWxpZGF0ZV9pbm9kZSgpDQp3aWxsIGFs
d2F5cyByZXN1bHQgaW4gYSBHRVRBVFRSIHJwYyBjYWxsLg0KDQpORlNfSU5PX0lOVkFMSURfQUND
RVNTIG1lYW5zIHRoYXQgdGhlIGZpbGUgYWNjZXNzIGNhY2hlIG5lZWRzDQpyZXZhbGlkYXRpb24u
IEEgY2FsbCB0byBuZnNfZG9fYWNjZXNzKCkgd2lsbCBhbHdheXMgcmVzdWx0IGluIGFuIEFDQ0VT
Uw0KcnBjIGNhbGwuDQoNCk5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFIG1lYW5zIHRoYXQgdGhlIGZp
bGUgcGFnZSBjYWNoZSBkYXRhIG5lZWRzDQpyZXZhbGlkYXRpb24uIEEgY2FsbCB0byBuZnNfcmV2
YWxpZGF0ZV9maWxlX3NpemUoKSBvcg0KbmZzX3JldmFsaWRhdGVfbWFwcGluZygpIHdpbGwgcmVz
dWx0IGluIGEgcmV2YWxpZGF0aW9uIG9mIHRoZSBwYWdlDQpjYWNoZS4gSW4gcHJhY3RpY2UgdGhh
dCBtZWFucyBhIEdFVEFUVFIgcnBjIGNhbGwsIGJ1dCB0aGF0IG1heSBjaGFuZ2UgaWYNCmZ1dHVy
ZSB2ZXJzaW9ucyBvZiBORlMgYWxsb3cgZm9yIGRpZmZlcmVudCBtZXRob2RzIG9mIHJldmFsaWRh
dGluZyB0aGUNCnBhZ2UgY2FjaGU7IGNvbnNpZGVyLCBmb3IgaW5zdGFuY2UsIHRoZSBieXRlLXJh
bmdlIGRlbGVnYXRpb24gUkZDIHRoYXQNCkJydWNlIGFuZCBJIHB1Ymxpc2hlZCBhIGZldyB5ZWFy
cyBiYWNrLg0KDQo+ID4gU28gdGhlIGZpeCBoZXJlIHNob3VsZCBiZSB0byBzZXQgTkZTX0lOT19S
RVZBTF9QQUdFQ0FDSEUgd2hlbiB0aGUgY2hhbmdlDQo+ID4gYXR0cmlidXRlIGFuZC9vciBzaXpl
IGNoYW5nZSBpcyBub3RpY2VkLiBUaGUgb25seSBmdW5jdGlvbiBJIGNhbiBzZWUNCj4gPiB0aGF0
IGFwcGVhcnMgdG8gZ2V0IHRoaXMgd3JvbmcgaXMgbmZzX3djY191cGRhdGVfaW5vZGUoKS4gRG9l
cyBmaXhpbmcNCj4gPiB0aGF0IGxheSB0aGUgYnVnIHRvIHJlc3Q/DQo+ID4gDQo+IA0KPiBObywg
dGhhdCBkb2Vzbid0IGZpeCBpdC4gVGhlcmUncyBhbm90aGVyIHBsYWNlIHRoYXQgbWlzc2VzIHNl
dHRpbmcgdGhhdA0KPiBmbGFnIHRvbyBpbiBuZnNfdXBkYXRlX2lub2RlKCkgd2hlcmUgdGhlIHNp
emUgY2hhbmdlcy4gSSBhZGRlZCBpdCB0aGVyZQ0KPiB0b28gYW5kIHRoYXQgYWxzbyBkaWRuJ3Qg
Zml4IGl0LiBIZXJlJ3Mgd2h5IChJIHRoaW5rKToNCj4gDQo+IFdoZW4gdGhlIHNpemUgY2hhbmdl
IGlzIG5vdGljZWQgdmlhIHRoZSBDVE8gR0VUQVRUUiBjYWxsLCB0aGVuIG15DQo+IHBhdGNoZWQg
a2VybmVsIG5vdyBzZXRzIE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFIGluIG5mc191cGRhdGVfaW5v
ZGUuDQo+IE5vdyBjYWNoZV92YWxpZGl0eSBpcyBzZXQgdG8gMHgyYS4gVGhlIGtlcm5lbCB0aGVu
IG1ha2VzIGFuIEFDQ0VTUyBjYWxsDQo+IHRoYXQgYWxzbyB1bHRpbWF0ZWx5IGVuZHMgdXAgaW4g
bmZzX3VwZGF0ZV9pbm9kZS4NCj4gDQo+IG5mc191cGRhdGVfaW5vZGUgc2F2ZXMgb2ZmIHRoZSBj
YWNoZV92YWxpZGl0eSBmbGFncyBhbmQgY2xlYXJzIGFsbCBvZg0KPiB0aGVtIGV4Y2VwdCBmb3Ig
TkZTX0lOT19JTlZBTElEX0RBVEEuIEFzIGJlc3QgSSBjYW4gdGVsbCwgdGhlDQo+IHNhdmVkX2Nh
Y2hlX3ZhbGlkaXR5IGlzIHRoZW4gZW5kaW5nIHVwIGRpc2NhcmRlZCBhbmQgb24gZXhpdCB0aGUN
Cj4gY2FjaGVfdmFsaWRpdHkgaXMgZW5kaW5nIHVwIHNldCB0byBqdXN0IE5GU19JTk9fSU5WQUxJ
RF9EQVRBLg0KPiANCj4gSXQgc2VlbXMgbGlrZSB3ZSBvdWdodCB0byBiZSByZXN0b3JpbmcgdGhl
IHNhdmVfY2FjaGVfdmFsaWRpdHkgZmxhZ3MNCj4gYmVmb3JlIGV4aXRpbmcgdGhhdCBmdW5jdGlv
biwgYnV0IEkgY29uZmVzcyB0aGF0IEkgZG9uJ3QgcXVpdGUgZ3Jhc3ANCj4gdGhlIGxvZ2ljIGlu
IG5mc191cGRhdGVfaW5vZGUuIFRob3VnaHRzPw0KDQpJZiB0aGUgQUNDRVNTIGNhbGwgaXNuJ3Qg
cmV2YWxpZGF0aW5nIHRoZSBmaWxlIHNpemUsIHRoZW4gaXQgc2hvdWxkIG5vdA0KYmUgY2xlYXJp
bmcgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEU7IGl0IGRvZXMgdGhpcyBieSByZXN0b3JpbmcgdGhh
dCBiaXQNCmZyb20gdGhlICJzYXZlX2NhY2hlX3ZhbGlkaXR5IiB2YXJpYWJsZSBhbmQgc3Rvcmlu
ZyBpdCBpbiAiaW52YWxpZCIuIEFzDQpmYXIgYXMgSSBjYW4gdGVsbCwgdGhhdCBjb2RlIGlzIGNv
cnJlY3QgaW4gdXBzdHJlYW0uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xp
ZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3
Lm5ldGFwcC5jb20NCg==
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 16:28 ` Myklebust, Trond
@ 2012-12-10 16:53 ` Jeff Layton
2012-12-10 17:02 ` Myklebust, Trond
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2012-12-10 16:53 UTC (permalink / raw)
To: Myklebust, Trond; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
On Mon, 10 Dec 2012 16:28:47 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> On Mon, 2012-12-10 at 11:05 -0500, Jeff Layton wrote:
> > On Mon, 10 Dec 2012 14:42:23 +0000
> > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> >
> > > On Mon, 2012-12-10 at 09:25 -0500, Jeff Layton wrote:
> > > > Jian reported that the following sequence would leave "testfile" with
> > > > corrupt data:
> > > >
> > > > # mount localhost:/export /mnt/nfs/ -o vers=3
> > > > # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
> > > > # cat -v /export/testfile
> > > > abc
> > > > ^@^@^@^@ghi
> > > >
> > > > While there's no locking involved here, the operations are serialized,
> > > > so CTO should prevent corruption.
> > > >
> > > > The first write to the file is fine and writes 4 bytes. The file is then
> > > > extended on the server. When it's reopened a GETATTR is issued and the
> > > > size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
> > > > the file. Because the file is opened for write only,
> > > > nfs_want_read_modify_write() returns 0 to nfs_write_begin().
> > > > nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
> > > > extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
> > > > still set on the file at that point, but that flag is ignored and
> > > > nfs_pageuptodate erroneously extends the write to cover the whole page,
> > > > with the write done on the server side filled in with zeroes.
> > > >
> > > > This patch just has that function check for NFS_INO_INVALID_DATA in
> > > > addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
> > > > over the code, I wonder if we might have a similar bug in
> > > > nfs_revalidate_size(). The difference between those two flags is very
> > > > subtle, so it seems like we ought to be checking for
> > > > NFS_INO_INVALID_DATA in most of the places that we look for
> > > > NFS_INO_REVAL_PAGECACHE.
> > > >
> > > > I believe this is regression introduced by commit 8d197a568. The code
> > > > did check for NFS_INO_INVALID_DATA prior to that patch.
> > > >
> > > > Original bug report is here:
> > > >
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=885743
> > >
> > > Hi Jeff,
> > >
> > > The point of NFS_INO_REVAL_PAGECACHE is to be able to distinguish
> > > between situations where the file size or change attribute is in doubt
> > > (and so the page cache validity is in doubt), and situations where other
> > > attributes are in doubt (such as ACCESS caches, file owner, nlinks,...).
> > > This is why we're only checking for NFS_INO_REVAL_PAGECACHE and not
> > > NFS_INO_INVALID_DATA in the functions you mention above.
> > >
> >
> > Ok, now I'm really confused... I thought that:
> >
> > NFS_INO_INVALID_DATA == pagecache data was known to be wrong
> > NFS_INO_REVAL_PAGECACHE == pagecache data is questionable
>
> There is no "known to be wrong" state. Both mean "needs revalidation".
>
> > ...but if I understand what you're saying above, the "data" that
> > NFS_INO_INVALID_DATA refers to is in peripheral caches, like the ACCESS
> > cache? I guess then I don't quite understand what NFS_INO_INVALID_ATTR
> > and NFS_INO_INVALID_ACCESS are for. Aren't they supposed to indicate that
> > the attrs and access caches are invalid?
>
> NFS_INO_INVALID_ATTR basically means that some aspect of the file
> metadata needs revalidation, and so a call to nfs_revalidate_inode()
> will always result in a GETATTR rpc call.
>
> NFS_INO_INVALID_ACCESS means that the file access cache needs
> revalidation. A call to nfs_do_access() will always result in an ACCESS
> rpc call.
>
> NFS_INO_REVAL_PAGECACHE means that the file page cache data needs
> revalidation. A call to nfs_revalidate_file_size() or
> nfs_revalidate_mapping() will result in a revalidation of the page
> cache. In practice that means a GETATTR rpc call, but that may change if
> future versions of NFS allow for different methods of revalidating the
> page cache; consider, for instance, the byte-range delegation RFC that
> Bruce and I published a few years back.
>
Ok, thanks. Another question though -- what does NFS_INO_INVALID_DATA
mean, and what distinguishes it from NFS_INO_REVAL_PAGECACHE?
> > > So the fix here should be to set NFS_INO_REVAL_PAGECACHE when the change
> > > attribute and/or size change is noticed. The only function I can see
> > > that appears to get this wrong is nfs_wcc_update_inode(). Does fixing
> > > that lay the bug to rest?
> > >
> >
> > No, that doesn't fix it. There's another place that misses setting that
> > flag too in nfs_update_inode() where the size changes. I added it there
> > too and that also didn't fix it. Here's why (I think):
> >
> > When the size change is noticed via the CTO GETATTR call, then my
> > patched kernel now sets NFS_INO_REVAL_PAGECACHE in nfs_update_inode.
> > Now cache_validity is set to 0x2a. The kernel then makes an ACCESS call
> > that also ultimately ends up in nfs_update_inode.
> >
> > nfs_update_inode saves off the cache_validity flags and clears all of
> > them except for NFS_INO_INVALID_DATA. As best I can tell, the
> > saved_cache_validity is then ending up discarded and on exit the
> > cache_validity is ending up set to just NFS_INO_INVALID_DATA.
> >
> > It seems like we ought to be restoring the save_cache_validity flags
> > before exiting that function, but I confess that I don't quite grasp
> > the logic in nfs_update_inode. Thoughts?
>
> If the ACCESS call isn't revalidating the file size, then it should not
> be clearing NFS_INO_REVAL_PAGECACHE; it does this by restoring that bit
> from the "save_cache_validity" variable and storing it in "invalid". As
> far as I can tell, that code is correct in upstream.
>
FWIW, I'm working on a 3.7-rc8 kernel here...
The ACCESS call ends up with an fattr that has NFS_ATTR_FATTR_SIZE set,
it just doesn't detect any change in size due to the fact that the CTO
GETATTR call has already updated it.
------------------[snip]------------------------
/* Check if our cached file size is stale */
if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
new_isize = nfs_size_to_loff_t(fattr->size);
cur_isize = i_size_read(inode);
if (new_isize != cur_isize) {
/* Do we perhaps have any outstanding writes, or has
* the file grown beyond our last write? */
if ((nfsi->npages == 0 && !test_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->flags)) ||
new_isize > cur_isize) {
i_size_write(inode, new_isize);
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
}
dprintk("NFS: isize change on server for file %s/%ld "
"(%Ld to %Ld)\n",
inode->i_sb->s_id,
inode->i_ino,
(long long)cur_isize,
(long long)new_isize);
}
} else
invalid |= save_cache_validity & (NFS_INO_INVALID_ATTR
| NFS_INO_REVAL_PAGECACHE
| NFS_INO_REVAL_FORCED);
------------------[snip]------------------------
So here, we only restore set the REVAL_PAGECACHE flag in invalid if the
size changed, or there was no size attribute present in the response.
The size hasn't changed since the GETATTR call, so REVAL_PAGECACHE is
not restored.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 16:53 ` Jeff Layton
@ 2012-12-10 17:02 ` Myklebust, Trond
2012-12-10 17:20 ` Jeff Layton
0 siblings, 1 reply; 12+ messages in thread
From: Myklebust, Trond @ 2012-12-10 17:02 UTC (permalink / raw)
To: Jeff Layton; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
T24gTW9uLCAyMDEyLTEyLTEwIGF0IDExOjUzIC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxNjoyODo0NyArMDAwMA0KPiAiTXlrbGVidXN0LCBUcm9uZCIg
PFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+ID4gT24gTW9uLCAyMDEy
LTEyLTEwIGF0IDExOjA1IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4gPiA+IE9uIE1vbiwg
MTAgRGVjIDIwMTIgMTQ6NDI6MjMgKzAwMDANCj4gPiA+ICJNeWtsZWJ1c3QsIFRyb25kIiA8VHJv
bmQuTXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiA+ID4gDQo+ID4gPiA+IE9uIE1vbiwg
MjAxMi0xMi0xMCBhdCAwOToyNSAtMDUwMCwgSmVmZiBMYXl0b24gd3JvdGU6DQo+ID4gPiA+ID4g
SmlhbiByZXBvcnRlZCB0aGF0IHRoZSBmb2xsb3dpbmcgc2VxdWVuY2Ugd291bGQgbGVhdmUgInRl
c3RmaWxlIiB3aXRoDQo+ID4gPiA+ID4gY29ycnVwdCBkYXRhOg0KPiA+ID4gPiA+IA0KPiA+ID4g
PiA+ICAgICAjIG1vdW50IGxvY2FsaG9zdDovZXhwb3J0IC9tbnQvbmZzLyAtbyB2ZXJzPTMNCj4g
PiA+ID4gPiAgICAgIyBlY2hvIGFiYyA+IC9tbnQvbmZzL3Rlc3RmaWxlOyBlY2hvIGRlZiA+PiAv
ZXhwb3J0L3Rlc3RmaWxlOyBlY2hvIGdoaSA+PiAvbW50L25mcy90ZXN0ZmlsZQ0KPiA+ID4gPiA+
ICAgICAjIGNhdCAtdiAvZXhwb3J0L3Rlc3RmaWxlDQo+ID4gPiA+ID4gICAgIGFiYw0KPiA+ID4g
PiA+ICAgICBeQF5AXkBeQGdoaQ0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IFdoaWxlIHRoZXJlJ3Mg
bm8gbG9ja2luZyBpbnZvbHZlZCBoZXJlLCB0aGUgb3BlcmF0aW9ucyBhcmUgc2VyaWFsaXplZCwN
Cj4gPiA+ID4gPiBzbyBDVE8gc2hvdWxkIHByZXZlbnQgY29ycnVwdGlvbi4NCj4gPiA+ID4gPiAN
Cj4gPiA+ID4gPiBUaGUgZmlyc3Qgd3JpdGUgdG8gdGhlIGZpbGUgaXMgZmluZSBhbmQgd3JpdGVz
IDQgYnl0ZXMuIFRoZSBmaWxlIGlzIHRoZW4NCj4gPiA+ID4gPiBleHRlbmRlZCBvbiB0aGUgc2Vy
dmVyLiBXaGVuIGl0J3MgcmVvcGVuZWQgYSBHRVRBVFRSIGlzIGlzc3VlZCBhbmQgdGhlDQo+ID4g
PiA+ID4gc2l6ZSBjaGFuZ2UgaXMgbm90aWNlZC4gVGhpcyBjYXVzZXMgTkZTX0lOT19JTlZBTElE
X0RBVEEgdG8gYmUgc2V0IG9uDQo+ID4gPiA+ID4gdGhlIGZpbGUuIEJlY2F1c2UgdGhlIGZpbGUg
aXMgb3BlbmVkIGZvciB3cml0ZSBvbmx5LA0KPiA+ID4gPiA+IG5mc193YW50X3JlYWRfbW9kaWZ5
X3dyaXRlKCkgcmV0dXJucyAwIHRvIG5mc193cml0ZV9iZWdpbigpLg0KPiA+ID4gPiA+IG5mc191
cGRhdGVwYWdlIHRoZW4gY2FsbHMgbmZzX3dyaXRlX3BhZ2V1cHRvZGF0ZSgpIHRvIHNlZSBpZiBp
dCBzaG91bGQNCj4gPiA+ID4gPiBleHRlbmQgdGhlIG5mc19wYWdlIHRvIGNvdmVyIHRoZSB3aG9s
ZSBwYWdlLiBORlNfSU5PX0lOVkFMSURfREFUQSBpcw0KPiA+ID4gPiA+IHN0aWxsIHNldCBvbiB0
aGUgZmlsZSBhdCB0aGF0IHBvaW50LCBidXQgdGhhdCBmbGFnIGlzIGlnbm9yZWQgYW5kDQo+ID4g
PiA+ID4gbmZzX3BhZ2V1cHRvZGF0ZSBlcnJvbmVvdXNseSBleHRlbmRzIHRoZSB3cml0ZSB0byBj
b3ZlciB0aGUgd2hvbGUgcGFnZSwNCj4gPiA+ID4gPiB3aXRoIHRoZSB3cml0ZSBkb25lIG9uIHRo
ZSBzZXJ2ZXIgc2lkZSBmaWxsZWQgaW4gd2l0aCB6ZXJvZXMuDQo+ID4gPiA+ID4gDQo+ID4gPiA+
ID4gVGhpcyBwYXRjaCBqdXN0IGhhcyB0aGF0IGZ1bmN0aW9uIGNoZWNrIGZvciBORlNfSU5PX0lO
VkFMSURfREFUQSBpbg0KPiA+ID4gPiA+IGFkZGl0aW9uIHRvIE5GU19JTk9fUkVWQUxfUEFHRUNB
Q0hFLiBUaGlzIGZpeGVzIHRoZSBidWcsIGJ1dCBsb29raW5nDQo+ID4gPiA+ID4gb3ZlciB0aGUg
Y29kZSwgSSB3b25kZXIgaWYgd2UgbWlnaHQgaGF2ZSBhIHNpbWlsYXIgYnVnIGluDQo+ID4gPiA+
ID4gbmZzX3JldmFsaWRhdGVfc2l6ZSgpLiBUaGUgZGlmZmVyZW5jZSBiZXR3ZWVuIHRob3NlIHR3
byBmbGFncyBpcyB2ZXJ5DQo+ID4gPiA+ID4gc3VidGxlLCBzbyBpdCBzZWVtcyBsaWtlIHdlIG91
Z2h0IHRvIGJlIGNoZWNraW5nIGZvcg0KPiA+ID4gPiA+IE5GU19JTk9fSU5WQUxJRF9EQVRBIGlu
IG1vc3Qgb2YgdGhlIHBsYWNlcyB0aGF0IHdlIGxvb2sgZm9yDQo+ID4gPiA+ID4gTkZTX0lOT19S
RVZBTF9QQUdFQ0FDSEUuDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gSSBiZWxpZXZlIHRoaXMgaXMg
cmVncmVzc2lvbiBpbnRyb2R1Y2VkIGJ5IGNvbW1pdCA4ZDE5N2E1NjguIFRoZSBjb2RlDQo+ID4g
PiA+ID4gZGlkIGNoZWNrIGZvciBORlNfSU5PX0lOVkFMSURfREFUQSBwcmlvciB0byB0aGF0IHBh
dGNoLg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IE9yaWdpbmFsIGJ1ZyByZXBvcnQgaXMgaGVyZToN
Cj4gPiA+ID4gPiANCj4gPiA+ID4gPiAgICAgaHR0cHM6Ly9idWd6aWxsYS5yZWRoYXQuY29tL3No
b3dfYnVnLmNnaT9pZD04ODU3NDMNCj4gPiA+ID4gDQo+ID4gPiA+IEhpIEplZmYsDQo+ID4gPiA+
IA0KPiA+ID4gPiBUaGUgcG9pbnQgb2YgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgaXMgdG8gYmUg
YWJsZSB0byBkaXN0aW5ndWlzaA0KPiA+ID4gPiBiZXR3ZWVuIHNpdHVhdGlvbnMgd2hlcmUgdGhl
IGZpbGUgc2l6ZSBvciBjaGFuZ2UgYXR0cmlidXRlIGlzIGluIGRvdWJ0DQo+ID4gPiA+IChhbmQg
c28gdGhlIHBhZ2UgY2FjaGUgdmFsaWRpdHkgaXMgaW4gZG91YnQpLCBhbmQgc2l0dWF0aW9ucyB3
aGVyZSBvdGhlcg0KPiA+ID4gPiBhdHRyaWJ1dGVzIGFyZSBpbiBkb3VidCAoc3VjaCBhcyBBQ0NF
U1MgY2FjaGVzLCBmaWxlIG93bmVyLCBubGlua3MsLi4uKS4NCj4gPiA+ID4gVGhpcyBpcyB3aHkg
d2UncmUgb25seSBjaGVja2luZyBmb3IgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgYW5kIG5vdA0K
PiA+ID4gPiBORlNfSU5PX0lOVkFMSURfREFUQSBpbiB0aGUgZnVuY3Rpb25zIHlvdSBtZW50aW9u
IGFib3ZlLg0KPiA+ID4gPiANCj4gPiA+IA0KPiA+ID4gT2ssIG5vdyBJJ20gcmVhbGx5IGNvbmZ1
c2VkLi4uIEkgdGhvdWdodCB0aGF0Og0KPiA+ID4gDQo+ID4gPiBORlNfSU5PX0lOVkFMSURfREFU
QSA9PSBwYWdlY2FjaGUgZGF0YSB3YXMga25vd24gdG8gYmUgd3JvbmcNCj4gPiA+IE5GU19JTk9f
UkVWQUxfUEFHRUNBQ0hFID09IHBhZ2VjYWNoZSBkYXRhIGlzIHF1ZXN0aW9uYWJsZQ0KPiA+IA0K
PiA+IFRoZXJlIGlzIG5vICJrbm93biB0byBiZSB3cm9uZyIgc3RhdGUuIEJvdGggbWVhbiAibmVl
ZHMgcmV2YWxpZGF0aW9uIi4NCj4gPiANCj4gPiA+IC4uLmJ1dCBpZiBJIHVuZGVyc3RhbmQgd2hh
dCB5b3UncmUgc2F5aW5nIGFib3ZlLCB0aGUgImRhdGEiIHRoYXQNCj4gPiA+IE5GU19JTk9fSU5W
QUxJRF9EQVRBIHJlZmVycyB0byBpcyBpbiBwZXJpcGhlcmFsIGNhY2hlcywgbGlrZSB0aGUgQUND
RVNTDQo+ID4gPiBjYWNoZT8gSSBndWVzcyB0aGVuIEkgZG9uJ3QgcXVpdGUgdW5kZXJzdGFuZCB3
aGF0IE5GU19JTk9fSU5WQUxJRF9BVFRSDQo+ID4gPiBhbmQgTkZTX0lOT19JTlZBTElEX0FDQ0VT
UyBhcmUgZm9yLiBBcmVuJ3QgdGhleSBzdXBwb3NlZCB0byBpbmRpY2F0ZSB0aGF0DQo+ID4gPiB0
aGUgYXR0cnMgYW5kIGFjY2VzcyBjYWNoZXMgYXJlIGludmFsaWQ/DQo+ID4gDQo+ID4gTkZTX0lO
T19JTlZBTElEX0FUVFIgYmFzaWNhbGx5IG1lYW5zIHRoYXQgc29tZSBhc3BlY3Qgb2YgdGhlIGZp
bGUNCj4gPiBtZXRhZGF0YSBuZWVkcyByZXZhbGlkYXRpb24sIGFuZCBzbyBhIGNhbGwgdG8gbmZz
X3JldmFsaWRhdGVfaW5vZGUoKQ0KPiA+IHdpbGwgYWx3YXlzIHJlc3VsdCBpbiBhIEdFVEFUVFIg
cnBjIGNhbGwuDQo+ID4gDQo+ID4gTkZTX0lOT19JTlZBTElEX0FDQ0VTUyBtZWFucyB0aGF0IHRo
ZSBmaWxlIGFjY2VzcyBjYWNoZSBuZWVkcw0KPiA+IHJldmFsaWRhdGlvbi4gQSBjYWxsIHRvIG5m
c19kb19hY2Nlc3MoKSB3aWxsIGFsd2F5cyByZXN1bHQgaW4gYW4gQUNDRVNTDQo+ID4gcnBjIGNh
bGwuDQo+ID4gDQo+ID4gTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgbWVhbnMgdGhhdCB0aGUgZmls
ZSBwYWdlIGNhY2hlIGRhdGEgbmVlZHMNCj4gPiByZXZhbGlkYXRpb24uIEEgY2FsbCB0byBuZnNf
cmV2YWxpZGF0ZV9maWxlX3NpemUoKSBvcg0KPiA+IG5mc19yZXZhbGlkYXRlX21hcHBpbmcoKSB3
aWxsIHJlc3VsdCBpbiBhIHJldmFsaWRhdGlvbiBvZiB0aGUgcGFnZQ0KPiA+IGNhY2hlLiBJbiBw
cmFjdGljZSB0aGF0IG1lYW5zIGEgR0VUQVRUUiBycGMgY2FsbCwgYnV0IHRoYXQgbWF5IGNoYW5n
ZSBpZg0KPiA+IGZ1dHVyZSB2ZXJzaW9ucyBvZiBORlMgYWxsb3cgZm9yIGRpZmZlcmVudCBtZXRo
b2RzIG9mIHJldmFsaWRhdGluZyB0aGUNCj4gPiBwYWdlIGNhY2hlOyBjb25zaWRlciwgZm9yIGlu
c3RhbmNlLCB0aGUgYnl0ZS1yYW5nZSBkZWxlZ2F0aW9uIFJGQyB0aGF0DQo+ID4gQnJ1Y2UgYW5k
IEkgcHVibGlzaGVkIGEgZmV3IHllYXJzIGJhY2suDQo+ID4gDQo+IA0KPiBPaywgdGhhbmtzLiBB
bm90aGVyIHF1ZXN0aW9uIHRob3VnaCAtLSB3aGF0IGRvZXMgTkZTX0lOT19JTlZBTElEX0RBVEEN
Cj4gbWVhbiwgYW5kIHdoYXQgZGlzdGluZ3Vpc2hlcyBpdCBmcm9tIE5GU19JTk9fUkVWQUxfUEFH
RUNBQ0hFPw0KDQpTZWUgYWJvdmUuIFRoZXkgYWZmZWN0IF9kaWZmZXJlbnRfIHJldmFsaWRhdGlv
biBzaXR1YXRpb25zIChyZXByZXNlbnRlZA0KYnkgZGlmZmVyZW50IHJldmFsaWRhdGlvbiBmdW5j
dGlvbnMpLg0KDQo+ID4gPiA+IFNvIHRoZSBmaXggaGVyZSBzaG91bGQgYmUgdG8gc2V0IE5GU19J
Tk9fUkVWQUxfUEFHRUNBQ0hFIHdoZW4gdGhlIGNoYW5nZQ0KPiA+ID4gPiBhdHRyaWJ1dGUgYW5k
L29yIHNpemUgY2hhbmdlIGlzIG5vdGljZWQuIFRoZSBvbmx5IGZ1bmN0aW9uIEkgY2FuIHNlZQ0K
PiA+ID4gPiB0aGF0IGFwcGVhcnMgdG8gZ2V0IHRoaXMgd3JvbmcgaXMgbmZzX3djY191cGRhdGVf
aW5vZGUoKS4gRG9lcyBmaXhpbmcNCj4gPiA+ID4gdGhhdCBsYXkgdGhlIGJ1ZyB0byByZXN0Pw0K
PiA+ID4gPiANCj4gPiA+IA0KPiA+ID4gTm8sIHRoYXQgZG9lc24ndCBmaXggaXQuIFRoZXJlJ3Mg
YW5vdGhlciBwbGFjZSB0aGF0IG1pc3NlcyBzZXR0aW5nIHRoYXQNCj4gPiA+IGZsYWcgdG9vIGlu
IG5mc191cGRhdGVfaW5vZGUoKSB3aGVyZSB0aGUgc2l6ZSBjaGFuZ2VzLiBJIGFkZGVkIGl0IHRo
ZXJlDQo+ID4gPiB0b28gYW5kIHRoYXQgYWxzbyBkaWRuJ3QgZml4IGl0LiBIZXJlJ3Mgd2h5IChJ
IHRoaW5rKToNCj4gPiA+IA0KPiA+ID4gV2hlbiB0aGUgc2l6ZSBjaGFuZ2UgaXMgbm90aWNlZCB2
aWEgdGhlIENUTyBHRVRBVFRSIGNhbGwsIHRoZW4gbXkNCj4gPiA+IHBhdGNoZWQga2VybmVsIG5v
dyBzZXRzIE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFIGluIG5mc191cGRhdGVfaW5vZGUuDQo+ID4g
PiBOb3cgY2FjaGVfdmFsaWRpdHkgaXMgc2V0IHRvIDB4MmEuIFRoZSBrZXJuZWwgdGhlbiBtYWtl
cyBhbiBBQ0NFU1MgY2FsbA0KPiA+ID4gdGhhdCBhbHNvIHVsdGltYXRlbHkgZW5kcyB1cCBpbiBu
ZnNfdXBkYXRlX2lub2RlLg0KPiA+ID4gDQo+ID4gPiBuZnNfdXBkYXRlX2lub2RlIHNhdmVzIG9m
ZiB0aGUgY2FjaGVfdmFsaWRpdHkgZmxhZ3MgYW5kIGNsZWFycyBhbGwgb2YNCj4gPiA+IHRoZW0g
ZXhjZXB0IGZvciBORlNfSU5PX0lOVkFMSURfREFUQS4gQXMgYmVzdCBJIGNhbiB0ZWxsLCB0aGUN
Cj4gPiA+IHNhdmVkX2NhY2hlX3ZhbGlkaXR5IGlzIHRoZW4gZW5kaW5nIHVwIGRpc2NhcmRlZCBh
bmQgb24gZXhpdCB0aGUNCj4gPiA+IGNhY2hlX3ZhbGlkaXR5IGlzIGVuZGluZyB1cCBzZXQgdG8g
anVzdCBORlNfSU5PX0lOVkFMSURfREFUQS4NCj4gPiA+IA0KPiA+ID4gSXQgc2VlbXMgbGlrZSB3
ZSBvdWdodCB0byBiZSByZXN0b3JpbmcgdGhlIHNhdmVfY2FjaGVfdmFsaWRpdHkgZmxhZ3MNCj4g
PiA+IGJlZm9yZSBleGl0aW5nIHRoYXQgZnVuY3Rpb24sIGJ1dCBJIGNvbmZlc3MgdGhhdCBJIGRv
bid0IHF1aXRlIGdyYXNwDQo+ID4gPiB0aGUgbG9naWMgaW4gbmZzX3VwZGF0ZV9pbm9kZS4gVGhv
dWdodHM/DQo+ID4gDQo+ID4gSWYgdGhlIEFDQ0VTUyBjYWxsIGlzbid0IHJldmFsaWRhdGluZyB0
aGUgZmlsZSBzaXplLCB0aGVuIGl0IHNob3VsZCBub3QNCj4gPiBiZSBjbGVhcmluZyBORlNfSU5P
X1JFVkFMX1BBR0VDQUNIRTsgaXQgZG9lcyB0aGlzIGJ5IHJlc3RvcmluZyB0aGF0IGJpdA0KPiA+
IGZyb20gdGhlICJzYXZlX2NhY2hlX3ZhbGlkaXR5IiB2YXJpYWJsZSBhbmQgc3RvcmluZyBpdCBp
biAiaW52YWxpZCIuIEFzDQo+ID4gZmFyIGFzIEkgY2FuIHRlbGwsIHRoYXQgY29kZSBpcyBjb3Jy
ZWN0IGluIHVwc3RyZWFtLg0KPiA+IA0KPiANCj4gRldJVywgSSdtIHdvcmtpbmcgb24gYSAzLjct
cmM4IGtlcm5lbCBoZXJlLi4uDQo+IA0KPiBUaGUgQUNDRVNTIGNhbGwgZW5kcyB1cCB3aXRoIGFu
IGZhdHRyIHRoYXQgaGFzIE5GU19BVFRSX0ZBVFRSX1NJWkUgc2V0LA0KPiBpdCBqdXN0IGRvZXNu
J3QgZGV0ZWN0IGFueSBjaGFuZ2UgaW4gc2l6ZSBkdWUgdG8gdGhlIGZhY3QgdGhhdCB0aGUgQ1RP
DQo+IEdFVEFUVFIgY2FsbCBoYXMgYWxyZWFkeSB1cGRhdGVkIGl0Lg0KPiANCj4gLS0tLS0tLS0t
LS0tLS0tLS0tW3NuaXBdLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+ICAgICAgICAgLyogQ2hl
Y2sgaWYgb3VyIGNhY2hlZCBmaWxlIHNpemUgaXMgc3RhbGUgKi8NCj4gICAgICAgICBpZiAoZmF0
dHItPnZhbGlkICYgTkZTX0FUVFJfRkFUVFJfU0laRSkgew0KPiAgICAgICAgICAgICAgICAgbmV3
X2lzaXplID0gbmZzX3NpemVfdG9fbG9mZl90KGZhdHRyLT5zaXplKTsNCj4gICAgICAgICAgICAg
ICAgIGN1cl9pc2l6ZSA9IGlfc2l6ZV9yZWFkKGlub2RlKTsNCj4gICAgICAgICAgICAgICAgIGlm
IChuZXdfaXNpemUgIT0gY3VyX2lzaXplKSB7DQo+ICAgICAgICAgICAgICAgICAgICAgICAgIC8q
IERvIHdlIHBlcmhhcHMgaGF2ZSBhbnkgb3V0c3RhbmRpbmcgd3JpdGVzLCBvciBoYXMNCj4gICAg
ICAgICAgICAgICAgICAgICAgICAgICogdGhlIGZpbGUgZ3Jvd24gYmV5b25kIG91ciBsYXN0IHdy
aXRlPyAqLw0KPiAgICAgICAgICAgICAgICAgICAgICAgICBpZiAoKG5mc2ktPm5wYWdlcyA9PSAw
ICYmICF0ZXN0X2JpdChORlNfSU5PX0xBWU9VVENPTU1JVCwgJm5mc2ktPmZsYWdzKSkgfHwNCj4g
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBuZXdfaXNpemUgPiBjdXJfaXNpemUpIHsNCj4g
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBpX3NpemVfd3JpdGUoaW5vZGUsIG5ld19p
c2l6ZSk7DQo+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaW52YWxpZCB8PSBORlNf
SU5PX0lOVkFMSURfQVRUUnxORlNfSU5PX0lOVkFMSURfREFUQTsNCj4gICAgICAgICAgICAgICAg
ICAgICAgICAgfQ0KPiAgICAgICAgICAgICAgICAgICAgICAgICBkcHJpbnRrKCJORlM6IGlzaXpl
IGNoYW5nZSBvbiBzZXJ2ZXIgZm9yIGZpbGUgJXMvJWxkICINCj4gICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICIoJUxkIHRvICVMZClcbiIsDQo+ICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICBpbm9kZS0+aV9zYi0+c19pZCwNCj4gICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGlub2RlLT5pX2lubywNCj4gICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIChsb25nIGxvbmcpY3VyX2lzaXplLA0KPiAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKGxvbmcgbG9uZyluZXdfaXNp
emUpOw0KPiAgICAgICAgICAgICAgICAgfQ0KPiAgICAgICAgIH0gZWxzZQ0KPiAgICAgICAgICAg
ICAgICAgaW52YWxpZCB8PSBzYXZlX2NhY2hlX3ZhbGlkaXR5ICYgKE5GU19JTk9fSU5WQUxJRF9B
VFRSDQo+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCBORlNfSU5PX1JFVkFMX1BB
R0VDQUNIRQ0KPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgTkZTX0lOT19SRVZB
TF9GT1JDRUQpOw0KPiAtLS0tLS0tLS0tLS0tLS0tLS1bc25pcF0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0NCj4gDQo+IFNvIGhlcmUsIHdlIG9ubHkgcmVzdG9yZSBzZXQgdGhlIFJFVkFMX1BBR0VD
QUNIRSBmbGFnIGluIGludmFsaWQgaWYgdGhlDQo+IHNpemUgY2hhbmdlZCwgb3IgdGhlcmUgd2Fz
IG5vIHNpemUgYXR0cmlidXRlIHByZXNlbnQgaW4gdGhlIHJlc3BvbnNlLg0KPiBUaGUgc2l6ZSBo
YXNuJ3QgY2hhbmdlZCBzaW5jZSB0aGUgR0VUQVRUUiBjYWxsLCBzbyBSRVZBTF9QQUdFQ0FDSEUg
aXMNCj4gbm90IHJlc3RvcmVkLg0KDQpJIGRvbid0IHVuZGVyc3RhbmQuIFdoeSBkb2VzIGl0IG5l
ZWQgdG8gYmUgcmVzdG9yZWQ/IFRoZQ0KTkZTX0FUVFJfRkFUVFJfU0laRSBpcyBzZXQsIHNvIHRo
ZSBzZXJ2ZXIgaGFzIHNlbnQgdXMgYW4gdXBkYXRlZCB2YWx1ZQ0KZm9yIHRoZSBzaXplLiBJcyBp
dCBseWluZyB0byB1cz8NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQg
bWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0
YXBwLmNvbQ0K
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 17:02 ` Myklebust, Trond
@ 2012-12-10 17:20 ` Jeff Layton
2012-12-10 18:00 ` Myklebust, Trond
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2012-12-10 17:20 UTC (permalink / raw)
To: Myklebust, Trond; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
On Mon, 10 Dec 2012 17:02:00 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> On Mon, 2012-12-10 at 11:53 -0500, Jeff Layton wrote:
> > On Mon, 10 Dec 2012 16:28:47 +0000
> > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> >
> > > On Mon, 2012-12-10 at 11:05 -0500, Jeff Layton wrote:
> > > > On Mon, 10 Dec 2012 14:42:23 +0000
> > > > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > > >
> > > > > On Mon, 2012-12-10 at 09:25 -0500, Jeff Layton wrote:
> > > > > > Jian reported that the following sequence would leave "testfile" with
> > > > > > corrupt data:
> > > > > >
> > > > > > # mount localhost:/export /mnt/nfs/ -o vers=3
> > > > > > # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
> > > > > > # cat -v /export/testfile
> > > > > > abc
> > > > > > ^@^@^@^@ghi
> > > > > >
> > > > > > While there's no locking involved here, the operations are serialized,
> > > > > > so CTO should prevent corruption.
> > > > > >
> > > > > > The first write to the file is fine and writes 4 bytes. The file is then
> > > > > > extended on the server. When it's reopened a GETATTR is issued and the
> > > > > > size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
> > > > > > the file. Because the file is opened for write only,
> > > > > > nfs_want_read_modify_write() returns 0 to nfs_write_begin().
> > > > > > nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
> > > > > > extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
> > > > > > still set on the file at that point, but that flag is ignored and
> > > > > > nfs_pageuptodate erroneously extends the write to cover the whole page,
> > > > > > with the write done on the server side filled in with zeroes.
> > > > > >
> > > > > > This patch just has that function check for NFS_INO_INVALID_DATA in
> > > > > > addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
> > > > > > over the code, I wonder if we might have a similar bug in
> > > > > > nfs_revalidate_size(). The difference between those two flags is very
> > > > > > subtle, so it seems like we ought to be checking for
> > > > > > NFS_INO_INVALID_DATA in most of the places that we look for
> > > > > > NFS_INO_REVAL_PAGECACHE.
> > > > > >
> > > > > > I believe this is regression introduced by commit 8d197a568. The code
> > > > > > did check for NFS_INO_INVALID_DATA prior to that patch.
> > > > > >
> > > > > > Original bug report is here:
> > > > > >
> > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=885743
> > > > >
> > > > > Hi Jeff,
> > > > >
> > > > > The point of NFS_INO_REVAL_PAGECACHE is to be able to distinguish
> > > > > between situations where the file size or change attribute is in doubt
> > > > > (and so the page cache validity is in doubt), and situations where other
> > > > > attributes are in doubt (such as ACCESS caches, file owner, nlinks,...).
> > > > > This is why we're only checking for NFS_INO_REVAL_PAGECACHE and not
> > > > > NFS_INO_INVALID_DATA in the functions you mention above.
> > > > >
> > > >
> > > > Ok, now I'm really confused... I thought that:
> > > >
> > > > NFS_INO_INVALID_DATA == pagecache data was known to be wrong
> > > > NFS_INO_REVAL_PAGECACHE == pagecache data is questionable
> > >
> > > There is no "known to be wrong" state. Both mean "needs revalidation".
> > >
> > > > ...but if I understand what you're saying above, the "data" that
> > > > NFS_INO_INVALID_DATA refers to is in peripheral caches, like the ACCESS
> > > > cache? I guess then I don't quite understand what NFS_INO_INVALID_ATTR
> > > > and NFS_INO_INVALID_ACCESS are for. Aren't they supposed to indicate that
> > > > the attrs and access caches are invalid?
> > >
> > > NFS_INO_INVALID_ATTR basically means that some aspect of the file
> > > metadata needs revalidation, and so a call to nfs_revalidate_inode()
> > > will always result in a GETATTR rpc call.
> > >
> > > NFS_INO_INVALID_ACCESS means that the file access cache needs
> > > revalidation. A call to nfs_do_access() will always result in an ACCESS
> > > rpc call.
> > >
> > > NFS_INO_REVAL_PAGECACHE means that the file page cache data needs
> > > revalidation. A call to nfs_revalidate_file_size() or
> > > nfs_revalidate_mapping() will result in a revalidation of the page
> > > cache. In practice that means a GETATTR rpc call, but that may change if
> > > future versions of NFS allow for different methods of revalidating the
> > > page cache; consider, for instance, the byte-range delegation RFC that
> > > Bruce and I published a few years back.
> > >
> >
> > Ok, thanks. Another question though -- what does NFS_INO_INVALID_DATA
> > mean, and what distinguishes it from NFS_INO_REVAL_PAGECACHE?
>
> See above. They affect _different_ revalidation situations (represented
> by different revalidation functions).
>
> > > > > So the fix here should be to set NFS_INO_REVAL_PAGECACHE when the change
> > > > > attribute and/or size change is noticed. The only function I can see
> > > > > that appears to get this wrong is nfs_wcc_update_inode(). Does fixing
> > > > > that lay the bug to rest?
> > > > >
> > > >
> > > > No, that doesn't fix it. There's another place that misses setting that
> > > > flag too in nfs_update_inode() where the size changes. I added it there
> > > > too and that also didn't fix it. Here's why (I think):
> > > >
> > > > When the size change is noticed via the CTO GETATTR call, then my
> > > > patched kernel now sets NFS_INO_REVAL_PAGECACHE in nfs_update_inode.
> > > > Now cache_validity is set to 0x2a. The kernel then makes an ACCESS call
> > > > that also ultimately ends up in nfs_update_inode.
> > > >
> > > > nfs_update_inode saves off the cache_validity flags and clears all of
> > > > them except for NFS_INO_INVALID_DATA. As best I can tell, the
> > > > saved_cache_validity is then ending up discarded and on exit the
> > > > cache_validity is ending up set to just NFS_INO_INVALID_DATA.
> > > >
> > > > It seems like we ought to be restoring the save_cache_validity flags
> > > > before exiting that function, but I confess that I don't quite grasp
> > > > the logic in nfs_update_inode. Thoughts?
> > >
> > > If the ACCESS call isn't revalidating the file size, then it should not
> > > be clearing NFS_INO_REVAL_PAGECACHE; it does this by restoring that bit
> > > from the "save_cache_validity" variable and storing it in "invalid". As
> > > far as I can tell, that code is correct in upstream.
> > >
> >
> > FWIW, I'm working on a 3.7-rc8 kernel here...
> >
> > The ACCESS call ends up with an fattr that has NFS_ATTR_FATTR_SIZE set,
> > it just doesn't detect any change in size due to the fact that the CTO
> > GETATTR call has already updated it.
> >
> > ------------------[snip]------------------------
> > /* Check if our cached file size is stale */
> > if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
> > new_isize = nfs_size_to_loff_t(fattr->size);
> > cur_isize = i_size_read(inode);
> > if (new_isize != cur_isize) {
> > /* Do we perhaps have any outstanding writes, or has
> > * the file grown beyond our last write? */
> > if ((nfsi->npages == 0 && !test_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->flags)) ||
> > new_isize > cur_isize) {
> > i_size_write(inode, new_isize);
> > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
> > }
> > dprintk("NFS: isize change on server for file %s/%ld "
> > "(%Ld to %Ld)\n",
> > inode->i_sb->s_id,
> > inode->i_ino,
> > (long long)cur_isize,
> > (long long)new_isize);
> > }
> > } else
> > invalid |= save_cache_validity & (NFS_INO_INVALID_ATTR
> > | NFS_INO_REVAL_PAGECACHE
> > | NFS_INO_REVAL_FORCED);
> > ------------------[snip]------------------------
> >
> > So here, we only restore set the REVAL_PAGECACHE flag in invalid if the
> > size changed, or there was no size attribute present in the response.
> > The size hasn't changed since the GETATTR call, so REVAL_PAGECACHE is
> > not restored.
>
> I don't understand. Why does it need to be restored? The
> NFS_ATTR_FATTR_SIZE is set, so the server has sent us an updated value
> for the size. Is it lying to us?
>
No, this is NFSv3 and IIUC, NFS_ATTR_FATTR_SIZE just means that
fattr->size is populated with a value sent by the server. In fact,
looking at decode_fattr3():
fattr->valid |= NFS_ATTR_FATTR_V3;
...and NFS_ATTR_FATTR_V3 contains NFS_ATTR_FATTR_SIZE.
Maybe this flag doesn't need to be restored. I really don't understand
the current logic in nfs_update_inode at all so I'll have to defer to
your judgement here.
All I know is that when the CTO GETATTR call is done, the kernel sees
that the size has changed. Presumably, it sets NFS_INO_REVAL_PAGECACHE
to reflect that fact. Then, on the following ACCESS call that gets back
attributes, it clears that flag without invalidating the cache.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 17:20 ` Jeff Layton
@ 2012-12-10 18:00 ` Myklebust, Trond
2012-12-10 18:08 ` Jeff Layton
2012-12-10 18:28 ` Jeff Layton
0 siblings, 2 replies; 12+ messages in thread
From: Myklebust, Trond @ 2012-12-10 18:00 UTC (permalink / raw)
To: Jeff Layton; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
T24gTW9uLCAyMDEyLTEyLTEwIGF0IDEyOjIwIC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxNzowMjowMCArMDAwMA0KPiAiTXlrbGVidXN0LCBUcm9uZCIg
PFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+ID4gT24gTW9uLCAyMDEy
LTEyLTEwIGF0IDExOjUzIC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4gPiA+IE9uIE1vbiwg
MTAgRGVjIDIwMTIgMTY6Mjg6NDcgKzAwMDANCj4gPiA+ICJNeWtsZWJ1c3QsIFRyb25kIiA8VHJv
bmQuTXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiA+ID4gDQo+ID4gPiA+IE9uIE1vbiwg
MjAxMi0xMi0xMCBhdCAxMTowNSAtMDUwMCwgSmVmZiBMYXl0b24gd3JvdGU6DQo+ID4gPiA+ID4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxNDo0MjoyMyArMDAwMA0KPiA+ID4gPiA+ICJNeWtsZWJ1c3Qs
IFRyb25kIiA8VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiA+ID4gPiA+IA0K
PiA+ID4gPiA+ID4gT24gTW9uLCAyMDEyLTEyLTEwIGF0IDA5OjI1IC0wNTAwLCBKZWZmIExheXRv
biB3cm90ZToNCj4gPiA+ID4gPiA+ID4gSmlhbiByZXBvcnRlZCB0aGF0IHRoZSBmb2xsb3dpbmcg
c2VxdWVuY2Ugd291bGQgbGVhdmUgInRlc3RmaWxlIiB3aXRoDQo+ID4gPiA+ID4gPiA+IGNvcnJ1
cHQgZGF0YToNCj4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+ICAgICAjIG1vdW50IGxvY2Fs
aG9zdDovZXhwb3J0IC9tbnQvbmZzLyAtbyB2ZXJzPTMNCj4gPiA+ID4gPiA+ID4gICAgICMgZWNo
byBhYmMgPiAvbW50L25mcy90ZXN0ZmlsZTsgZWNobyBkZWYgPj4gL2V4cG9ydC90ZXN0ZmlsZTsg
ZWNobyBnaGkgPj4gL21udC9uZnMvdGVzdGZpbGUNCj4gPiA+ID4gPiA+ID4gICAgICMgY2F0IC12
IC9leHBvcnQvdGVzdGZpbGUNCj4gPiA+ID4gPiA+ID4gICAgIGFiYw0KPiA+ID4gPiA+ID4gPiAg
ICAgXkBeQF5AXkBnaGkNCj4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+IFdoaWxlIHRoZXJl
J3Mgbm8gbG9ja2luZyBpbnZvbHZlZCBoZXJlLCB0aGUgb3BlcmF0aW9ucyBhcmUgc2VyaWFsaXpl
ZCwNCj4gPiA+ID4gPiA+ID4gc28gQ1RPIHNob3VsZCBwcmV2ZW50IGNvcnJ1cHRpb24uDQo+ID4g
PiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiBUaGUgZmlyc3Qgd3JpdGUgdG8gdGhlIGZpbGUgaXMg
ZmluZSBhbmQgd3JpdGVzIDQgYnl0ZXMuIFRoZSBmaWxlIGlzIHRoZW4NCj4gPiA+ID4gPiA+ID4g
ZXh0ZW5kZWQgb24gdGhlIHNlcnZlci4gV2hlbiBpdCdzIHJlb3BlbmVkIGEgR0VUQVRUUiBpcyBp
c3N1ZWQgYW5kIHRoZQ0KPiA+ID4gPiA+ID4gPiBzaXplIGNoYW5nZSBpcyBub3RpY2VkLiBUaGlz
IGNhdXNlcyBORlNfSU5PX0lOVkFMSURfREFUQSB0byBiZSBzZXQgb24NCj4gPiA+ID4gPiA+ID4g
dGhlIGZpbGUuIEJlY2F1c2UgdGhlIGZpbGUgaXMgb3BlbmVkIGZvciB3cml0ZSBvbmx5LA0KPiA+
ID4gPiA+ID4gPiBuZnNfd2FudF9yZWFkX21vZGlmeV93cml0ZSgpIHJldHVybnMgMCB0byBuZnNf
d3JpdGVfYmVnaW4oKS4NCj4gPiA+ID4gPiA+ID4gbmZzX3VwZGF0ZXBhZ2UgdGhlbiBjYWxscyBu
ZnNfd3JpdGVfcGFnZXVwdG9kYXRlKCkgdG8gc2VlIGlmIGl0IHNob3VsZA0KPiA+ID4gPiA+ID4g
PiBleHRlbmQgdGhlIG5mc19wYWdlIHRvIGNvdmVyIHRoZSB3aG9sZSBwYWdlLiBORlNfSU5PX0lO
VkFMSURfREFUQSBpcw0KPiA+ID4gPiA+ID4gPiBzdGlsbCBzZXQgb24gdGhlIGZpbGUgYXQgdGhh
dCBwb2ludCwgYnV0IHRoYXQgZmxhZyBpcyBpZ25vcmVkIGFuZA0KPiA+ID4gPiA+ID4gPiBuZnNf
cGFnZXVwdG9kYXRlIGVycm9uZW91c2x5IGV4dGVuZHMgdGhlIHdyaXRlIHRvIGNvdmVyIHRoZSB3
aG9sZSBwYWdlLA0KPiA+ID4gPiA+ID4gPiB3aXRoIHRoZSB3cml0ZSBkb25lIG9uIHRoZSBzZXJ2
ZXIgc2lkZSBmaWxsZWQgaW4gd2l0aCB6ZXJvZXMuDQo+ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+
ID4gPiBUaGlzIHBhdGNoIGp1c3QgaGFzIHRoYXQgZnVuY3Rpb24gY2hlY2sgZm9yIE5GU19JTk9f
SU5WQUxJRF9EQVRBIGluDQo+ID4gPiA+ID4gPiA+IGFkZGl0aW9uIHRvIE5GU19JTk9fUkVWQUxf
UEFHRUNBQ0hFLiBUaGlzIGZpeGVzIHRoZSBidWcsIGJ1dCBsb29raW5nDQo+ID4gPiA+ID4gPiA+
IG92ZXIgdGhlIGNvZGUsIEkgd29uZGVyIGlmIHdlIG1pZ2h0IGhhdmUgYSBzaW1pbGFyIGJ1ZyBp
bg0KPiA+ID4gPiA+ID4gPiBuZnNfcmV2YWxpZGF0ZV9zaXplKCkuIFRoZSBkaWZmZXJlbmNlIGJl
dHdlZW4gdGhvc2UgdHdvIGZsYWdzIGlzIHZlcnkNCj4gPiA+ID4gPiA+ID4gc3VidGxlLCBzbyBp
dCBzZWVtcyBsaWtlIHdlIG91Z2h0IHRvIGJlIGNoZWNraW5nIGZvcg0KPiA+ID4gPiA+ID4gPiBO
RlNfSU5PX0lOVkFMSURfREFUQSBpbiBtb3N0IG9mIHRoZSBwbGFjZXMgdGhhdCB3ZSBsb29rIGZv
cg0KPiA+ID4gPiA+ID4gPiBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRS4NCj4gPiA+ID4gPiA+ID4g
DQo+ID4gPiA+ID4gPiA+IEkgYmVsaWV2ZSB0aGlzIGlzIHJlZ3Jlc3Npb24gaW50cm9kdWNlZCBi
eSBjb21taXQgOGQxOTdhNTY4LiBUaGUgY29kZQ0KPiA+ID4gPiA+ID4gPiBkaWQgY2hlY2sgZm9y
IE5GU19JTk9fSU5WQUxJRF9EQVRBIHByaW9yIHRvIHRoYXQgcGF0Y2guDQo+ID4gPiA+ID4gPiA+
IA0KPiA+ID4gPiA+ID4gPiBPcmlnaW5hbCBidWcgcmVwb3J0IGlzIGhlcmU6DQo+ID4gPiA+ID4g
PiA+IA0KPiA+ID4gPiA+ID4gPiAgICAgaHR0cHM6Ly9idWd6aWxsYS5yZWRoYXQuY29tL3Nob3df
YnVnLmNnaT9pZD04ODU3NDMNCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gSGkgSmVmZiwNCj4g
PiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gVGhlIHBvaW50IG9mIE5GU19JTk9fUkVWQUxfUEFHRUNB
Q0hFIGlzIHRvIGJlIGFibGUgdG8gZGlzdGluZ3Vpc2gNCj4gPiA+ID4gPiA+IGJldHdlZW4gc2l0
dWF0aW9ucyB3aGVyZSB0aGUgZmlsZSBzaXplIG9yIGNoYW5nZSBhdHRyaWJ1dGUgaXMgaW4gZG91
YnQNCj4gPiA+ID4gPiA+IChhbmQgc28gdGhlIHBhZ2UgY2FjaGUgdmFsaWRpdHkgaXMgaW4gZG91
YnQpLCBhbmQgc2l0dWF0aW9ucyB3aGVyZSBvdGhlcg0KPiA+ID4gPiA+ID4gYXR0cmlidXRlcyBh
cmUgaW4gZG91YnQgKHN1Y2ggYXMgQUNDRVNTIGNhY2hlcywgZmlsZSBvd25lciwgbmxpbmtzLC4u
LikuDQo+ID4gPiA+ID4gPiBUaGlzIGlzIHdoeSB3ZSdyZSBvbmx5IGNoZWNraW5nIGZvciBORlNf
SU5PX1JFVkFMX1BBR0VDQUNIRSBhbmQgbm90DQo+ID4gPiA+ID4gPiBORlNfSU5PX0lOVkFMSURf
REFUQSBpbiB0aGUgZnVuY3Rpb25zIHlvdSBtZW50aW9uIGFib3ZlLg0KPiA+ID4gPiA+ID4gDQo+
ID4gPiA+ID4gDQo+ID4gPiA+ID4gT2ssIG5vdyBJJ20gcmVhbGx5IGNvbmZ1c2VkLi4uIEkgdGhv
dWdodCB0aGF0Og0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IE5GU19JTk9fSU5WQUxJRF9EQVRBID09
IHBhZ2VjYWNoZSBkYXRhIHdhcyBrbm93biB0byBiZSB3cm9uZw0KPiA+ID4gPiA+IE5GU19JTk9f
UkVWQUxfUEFHRUNBQ0hFID09IHBhZ2VjYWNoZSBkYXRhIGlzIHF1ZXN0aW9uYWJsZQ0KPiA+ID4g
PiANCj4gPiA+ID4gVGhlcmUgaXMgbm8gImtub3duIHRvIGJlIHdyb25nIiBzdGF0ZS4gQm90aCBt
ZWFuICJuZWVkcyByZXZhbGlkYXRpb24iLg0KPiA+ID4gPiANCj4gPiA+ID4gPiAuLi5idXQgaWYg
SSB1bmRlcnN0YW5kIHdoYXQgeW91J3JlIHNheWluZyBhYm92ZSwgdGhlICJkYXRhIiB0aGF0DQo+
ID4gPiA+ID4gTkZTX0lOT19JTlZBTElEX0RBVEEgcmVmZXJzIHRvIGlzIGluIHBlcmlwaGVyYWwg
Y2FjaGVzLCBsaWtlIHRoZSBBQ0NFU1MNCj4gPiA+ID4gPiBjYWNoZT8gSSBndWVzcyB0aGVuIEkg
ZG9uJ3QgcXVpdGUgdW5kZXJzdGFuZCB3aGF0IE5GU19JTk9fSU5WQUxJRF9BVFRSDQo+ID4gPiA+
ID4gYW5kIE5GU19JTk9fSU5WQUxJRF9BQ0NFU1MgYXJlIGZvci4gQXJlbid0IHRoZXkgc3VwcG9z
ZWQgdG8gaW5kaWNhdGUgdGhhdA0KPiA+ID4gPiA+IHRoZSBhdHRycyBhbmQgYWNjZXNzIGNhY2hl
cyBhcmUgaW52YWxpZD8NCj4gPiA+ID4gDQo+ID4gPiA+IE5GU19JTk9fSU5WQUxJRF9BVFRSIGJh
c2ljYWxseSBtZWFucyB0aGF0IHNvbWUgYXNwZWN0IG9mIHRoZSBmaWxlDQo+ID4gPiA+IG1ldGFk
YXRhIG5lZWRzIHJldmFsaWRhdGlvbiwgYW5kIHNvIGEgY2FsbCB0byBuZnNfcmV2YWxpZGF0ZV9p
bm9kZSgpDQo+ID4gPiA+IHdpbGwgYWx3YXlzIHJlc3VsdCBpbiBhIEdFVEFUVFIgcnBjIGNhbGwu
DQo+ID4gPiA+IA0KPiA+ID4gPiBORlNfSU5PX0lOVkFMSURfQUNDRVNTIG1lYW5zIHRoYXQgdGhl
IGZpbGUgYWNjZXNzIGNhY2hlIG5lZWRzDQo+ID4gPiA+IHJldmFsaWRhdGlvbi4gQSBjYWxsIHRv
IG5mc19kb19hY2Nlc3MoKSB3aWxsIGFsd2F5cyByZXN1bHQgaW4gYW4gQUNDRVNTDQo+ID4gPiA+
IHJwYyBjYWxsLg0KPiA+ID4gPiANCj4gPiA+ID4gTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgbWVh
bnMgdGhhdCB0aGUgZmlsZSBwYWdlIGNhY2hlIGRhdGEgbmVlZHMNCj4gPiA+ID4gcmV2YWxpZGF0
aW9uLiBBIGNhbGwgdG8gbmZzX3JldmFsaWRhdGVfZmlsZV9zaXplKCkgb3INCj4gPiA+ID4gbmZz
X3JldmFsaWRhdGVfbWFwcGluZygpIHdpbGwgcmVzdWx0IGluIGEgcmV2YWxpZGF0aW9uIG9mIHRo
ZSBwYWdlDQo+ID4gPiA+IGNhY2hlLiBJbiBwcmFjdGljZSB0aGF0IG1lYW5zIGEgR0VUQVRUUiBy
cGMgY2FsbCwgYnV0IHRoYXQgbWF5IGNoYW5nZSBpZg0KPiA+ID4gPiBmdXR1cmUgdmVyc2lvbnMg
b2YgTkZTIGFsbG93IGZvciBkaWZmZXJlbnQgbWV0aG9kcyBvZiByZXZhbGlkYXRpbmcgdGhlDQo+
ID4gPiA+IHBhZ2UgY2FjaGU7IGNvbnNpZGVyLCBmb3IgaW5zdGFuY2UsIHRoZSBieXRlLXJhbmdl
IGRlbGVnYXRpb24gUkZDIHRoYXQNCj4gPiA+ID4gQnJ1Y2UgYW5kIEkgcHVibGlzaGVkIGEgZmV3
IHllYXJzIGJhY2suDQo+ID4gPiA+IA0KPiA+ID4gDQo+ID4gPiBPaywgdGhhbmtzLiBBbm90aGVy
IHF1ZXN0aW9uIHRob3VnaCAtLSB3aGF0IGRvZXMgTkZTX0lOT19JTlZBTElEX0RBVEENCj4gPiA+
IG1lYW4sIGFuZCB3aGF0IGRpc3Rpbmd1aXNoZXMgaXQgZnJvbSBORlNfSU5PX1JFVkFMX1BBR0VD
QUNIRT8NCj4gPiANCj4gPiBTZWUgYWJvdmUuIFRoZXkgYWZmZWN0IF9kaWZmZXJlbnRfIHJldmFs
aWRhdGlvbiBzaXR1YXRpb25zIChyZXByZXNlbnRlZA0KPiA+IGJ5IGRpZmZlcmVudCByZXZhbGlk
YXRpb24gZnVuY3Rpb25zKS4NCj4gPiANCj4gPiA+ID4gPiA+IFNvIHRoZSBmaXggaGVyZSBzaG91
bGQgYmUgdG8gc2V0IE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFIHdoZW4gdGhlIGNoYW5nZQ0KPiA+
ID4gPiA+ID4gYXR0cmlidXRlIGFuZC9vciBzaXplIGNoYW5nZSBpcyBub3RpY2VkLiBUaGUgb25s
eSBmdW5jdGlvbiBJIGNhbiBzZWUNCj4gPiA+ID4gPiA+IHRoYXQgYXBwZWFycyB0byBnZXQgdGhp
cyB3cm9uZyBpcyBuZnNfd2NjX3VwZGF0ZV9pbm9kZSgpLiBEb2VzIGZpeGluZw0KPiA+ID4gPiA+
ID4gdGhhdCBsYXkgdGhlIGJ1ZyB0byByZXN0Pw0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gDQo+
ID4gPiA+ID4gTm8sIHRoYXQgZG9lc24ndCBmaXggaXQuIFRoZXJlJ3MgYW5vdGhlciBwbGFjZSB0
aGF0IG1pc3NlcyBzZXR0aW5nIHRoYXQNCj4gPiA+ID4gPiBmbGFnIHRvbyBpbiBuZnNfdXBkYXRl
X2lub2RlKCkgd2hlcmUgdGhlIHNpemUgY2hhbmdlcy4gSSBhZGRlZCBpdCB0aGVyZQ0KPiA+ID4g
PiA+IHRvbyBhbmQgdGhhdCBhbHNvIGRpZG4ndCBmaXggaXQuIEhlcmUncyB3aHkgKEkgdGhpbmsp
Og0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IFdoZW4gdGhlIHNpemUgY2hhbmdlIGlzIG5vdGljZWQg
dmlhIHRoZSBDVE8gR0VUQVRUUiBjYWxsLCB0aGVuIG15DQo+ID4gPiA+ID4gcGF0Y2hlZCBrZXJu
ZWwgbm93IHNldHMgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgaW4gbmZzX3VwZGF0ZV9pbm9kZS4N
Cj4gPiA+ID4gPiBOb3cgY2FjaGVfdmFsaWRpdHkgaXMgc2V0IHRvIDB4MmEuIFRoZSBrZXJuZWwg
dGhlbiBtYWtlcyBhbiBBQ0NFU1MgY2FsbA0KPiA+ID4gPiA+IHRoYXQgYWxzbyB1bHRpbWF0ZWx5
IGVuZHMgdXAgaW4gbmZzX3VwZGF0ZV9pbm9kZS4NCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBuZnNf
dXBkYXRlX2lub2RlIHNhdmVzIG9mZiB0aGUgY2FjaGVfdmFsaWRpdHkgZmxhZ3MgYW5kIGNsZWFy
cyBhbGwgb2YNCj4gPiA+ID4gPiB0aGVtIGV4Y2VwdCBmb3IgTkZTX0lOT19JTlZBTElEX0RBVEEu
IEFzIGJlc3QgSSBjYW4gdGVsbCwgdGhlDQo+ID4gPiA+ID4gc2F2ZWRfY2FjaGVfdmFsaWRpdHkg
aXMgdGhlbiBlbmRpbmcgdXAgZGlzY2FyZGVkIGFuZCBvbiBleGl0IHRoZQ0KPiA+ID4gPiA+IGNh
Y2hlX3ZhbGlkaXR5IGlzIGVuZGluZyB1cCBzZXQgdG8ganVzdCBORlNfSU5PX0lOVkFMSURfREFU
QS4NCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBJdCBzZWVtcyBsaWtlIHdlIG91Z2h0IHRvIGJlIHJl
c3RvcmluZyB0aGUgc2F2ZV9jYWNoZV92YWxpZGl0eSBmbGFncw0KPiA+ID4gPiA+IGJlZm9yZSBl
eGl0aW5nIHRoYXQgZnVuY3Rpb24sIGJ1dCBJIGNvbmZlc3MgdGhhdCBJIGRvbid0IHF1aXRlIGdy
YXNwDQo+ID4gPiA+ID4gdGhlIGxvZ2ljIGluIG5mc191cGRhdGVfaW5vZGUuIFRob3VnaHRzPw0K
PiA+ID4gPiANCj4gPiA+ID4gSWYgdGhlIEFDQ0VTUyBjYWxsIGlzbid0IHJldmFsaWRhdGluZyB0
aGUgZmlsZSBzaXplLCB0aGVuIGl0IHNob3VsZCBub3QNCj4gPiA+ID4gYmUgY2xlYXJpbmcgTkZT
X0lOT19SRVZBTF9QQUdFQ0FDSEU7IGl0IGRvZXMgdGhpcyBieSByZXN0b3JpbmcgdGhhdCBiaXQN
Cj4gPiA+ID4gZnJvbSB0aGUgInNhdmVfY2FjaGVfdmFsaWRpdHkiIHZhcmlhYmxlIGFuZCBzdG9y
aW5nIGl0IGluICJpbnZhbGlkIi4gQXMNCj4gPiA+ID4gZmFyIGFzIEkgY2FuIHRlbGwsIHRoYXQg
Y29kZSBpcyBjb3JyZWN0IGluIHVwc3RyZWFtLg0KPiA+ID4gPiANCj4gPiA+IA0KPiA+ID4gRldJ
VywgSSdtIHdvcmtpbmcgb24gYSAzLjctcmM4IGtlcm5lbCBoZXJlLi4uDQo+ID4gPiANCj4gPiA+
IFRoZSBBQ0NFU1MgY2FsbCBlbmRzIHVwIHdpdGggYW4gZmF0dHIgdGhhdCBoYXMgTkZTX0FUVFJf
RkFUVFJfU0laRSBzZXQsDQo+ID4gPiBpdCBqdXN0IGRvZXNuJ3QgZGV0ZWN0IGFueSBjaGFuZ2Ug
aW4gc2l6ZSBkdWUgdG8gdGhlIGZhY3QgdGhhdCB0aGUgQ1RPDQo+ID4gPiBHRVRBVFRSIGNhbGwg
aGFzIGFscmVhZHkgdXBkYXRlZCBpdC4NCj4gPiA+IA0KPiA+ID4gLS0tLS0tLS0tLS0tLS0tLS0t
W3NuaXBdLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+ID4gPiAgICAgICAgIC8qIENoZWNrIGlm
IG91ciBjYWNoZWQgZmlsZSBzaXplIGlzIHN0YWxlICovDQo+ID4gPiAgICAgICAgIGlmIChmYXR0
ci0+dmFsaWQgJiBORlNfQVRUUl9GQVRUUl9TSVpFKSB7DQo+ID4gPiAgICAgICAgICAgICAgICAg
bmV3X2lzaXplID0gbmZzX3NpemVfdG9fbG9mZl90KGZhdHRyLT5zaXplKTsNCj4gPiA+ICAgICAg
ICAgICAgICAgICBjdXJfaXNpemUgPSBpX3NpemVfcmVhZChpbm9kZSk7DQo+ID4gPiAgICAgICAg
ICAgICAgICAgaWYgKG5ld19pc2l6ZSAhPSBjdXJfaXNpemUpIHsNCj4gPiA+ICAgICAgICAgICAg
ICAgICAgICAgICAgIC8qIERvIHdlIHBlcmhhcHMgaGF2ZSBhbnkgb3V0c3RhbmRpbmcgd3JpdGVz
LCBvciBoYXMNCj4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAqIHRoZSBmaWxlIGdyb3du
IGJleW9uZCBvdXIgbGFzdCB3cml0ZT8gKi8NCj4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAg
IGlmICgobmZzaS0+bnBhZ2VzID09IDAgJiYgIXRlc3RfYml0KE5GU19JTk9fTEFZT1VUQ09NTUlU
LCAmbmZzaS0+ZmxhZ3MpKSB8fA0KPiA+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICBu
ZXdfaXNpemUgPiBjdXJfaXNpemUpIHsNCj4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgaV9zaXplX3dyaXRlKGlub2RlLCBuZXdfaXNpemUpOw0KPiA+ID4gICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBpbnZhbGlkIHw9IE5GU19JTk9fSU5WQUxJRF9BVFRSfE5GU19J
Tk9fSU5WQUxJRF9EQVRBOw0KPiA+ID4gICAgICAgICAgICAgICAgICAgICAgICAgfQ0KPiA+ID4g
ICAgICAgICAgICAgICAgICAgICAgICAgZHByaW50aygiTkZTOiBpc2l6ZSBjaGFuZ2Ugb24gc2Vy
dmVyIGZvciBmaWxlICVzLyVsZCAiDQo+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIiglTGQgdG8gJUxkKVxuIiwNCj4gPiA+ICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBpbm9kZS0+aV9zYi0+c19pZCwNCj4gPiA+ICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBpbm9kZS0+aV9pbm8sDQo+ID4gPiAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKGxvbmcgbG9uZyljdXJfaXNpemUsDQo+
ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKGxvbmcgbG9uZylu
ZXdfaXNpemUpOw0KPiA+ID4gICAgICAgICAgICAgICAgIH0NCj4gPiA+ICAgICAgICAgfSBlbHNl
DQo+ID4gPiAgICAgICAgICAgICAgICAgaW52YWxpZCB8PSBzYXZlX2NhY2hlX3ZhbGlkaXR5ICYg
KE5GU19JTk9fSU5WQUxJRF9BVFRSDQo+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgIHwgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUNCj4gPiA+ICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgfCBORlNfSU5PX1JFVkFMX0ZPUkNFRCk7DQo+ID4gPiAtLS0tLS0tLS0tLS0t
LS0tLS1bc25pcF0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gPiA+IA0KPiA+ID4gU28gaGVy
ZSwgd2Ugb25seSByZXN0b3JlIHNldCB0aGUgUkVWQUxfUEFHRUNBQ0hFIGZsYWcgaW4gaW52YWxp
ZCBpZiB0aGUNCj4gPiA+IHNpemUgY2hhbmdlZCwgb3IgdGhlcmUgd2FzIG5vIHNpemUgYXR0cmli
dXRlIHByZXNlbnQgaW4gdGhlIHJlc3BvbnNlLg0KPiA+ID4gVGhlIHNpemUgaGFzbid0IGNoYW5n
ZWQgc2luY2UgdGhlIEdFVEFUVFIgY2FsbCwgc28gUkVWQUxfUEFHRUNBQ0hFIGlzDQo+ID4gPiBu
b3QgcmVzdG9yZWQuDQo+ID4gDQo+ID4gSSBkb24ndCB1bmRlcnN0YW5kLiBXaHkgZG9lcyBpdCBu
ZWVkIHRvIGJlIHJlc3RvcmVkPyBUaGUNCj4gPiBORlNfQVRUUl9GQVRUUl9TSVpFIGlzIHNldCwg
c28gdGhlIHNlcnZlciBoYXMgc2VudCB1cyBhbiB1cGRhdGVkIHZhbHVlDQo+ID4gZm9yIHRoZSBz
aXplLiBJcyBpdCBseWluZyB0byB1cz8NCj4gPiANCj4gDQo+IE5vLCB0aGlzIGlzIE5GU3YzIGFu
ZCBJSVVDLCBORlNfQVRUUl9GQVRUUl9TSVpFIGp1c3QgbWVhbnMgdGhhdA0KPiBmYXR0ci0+c2l6
ZSBpcyBwb3B1bGF0ZWQgd2l0aCBhIHZhbHVlIHNlbnQgYnkgdGhlIHNlcnZlci4gSW4gZmFjdCwN
Cj4gbG9va2luZyBhdCBkZWNvZGVfZmF0dHIzKCk6DQo+IA0KPiAgICAgICAgIGZhdHRyLT52YWxp
ZCB8PSBORlNfQVRUUl9GQVRUUl9WMzsNCj4gDQo+IC4uLmFuZCBORlNfQVRUUl9GQVRUUl9WMyBj
b250YWlucyBORlNfQVRUUl9GQVRUUl9TSVpFLg0KDQpSaWdodC4gVGhlIE5GU3YzIHNlcnZlciBp
cyBhbHdheXMgc3VwcG9zZWQgdG8gc2VuZCB1cyB0aGUgY29ycmVjdCB2YWx1ZQ0KZm9yIHRoZSBz
aXplLg0KDQo+IE1heWJlIHRoaXMgZmxhZyBkb2Vzbid0IG5lZWQgdG8gYmUgcmVzdG9yZWQuIEkg
cmVhbGx5IGRvbid0IHVuZGVyc3RhbmQNCj4gdGhlIGN1cnJlbnQgbG9naWMgaW4gbmZzX3VwZGF0
ZV9pbm9kZSBhdCBhbGwgc28gSSdsbCBoYXZlIHRvIGRlZmVyIHRvDQo+IHlvdXIganVkZ2VtZW50
IGhlcmUuDQo+IA0KPiBBbGwgSSBrbm93IGlzIHRoYXQgd2hlbiB0aGUgQ1RPIEdFVEFUVFIgY2Fs
bCBpcyBkb25lLCB0aGUga2VybmVsIHNlZXMNCj4gdGhhdCB0aGUgc2l6ZSBoYXMgY2hhbmdlZC4g
UHJlc3VtYWJseSwgaXQgc2V0cyBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRQ0KPiB0byByZWZsZWN0
IHRoYXQgZmFjdC4NCg0KSXQgc2hvdWxkIGFsc28gc2V0IE5GU19JTk9fSU5WQUxJRF9EQVRBLCB3
aGljaCBpcyBfbm90XyBjbGVhcmVkIGJ5DQpuZnNfdXBkYXRlX2lub2RlKCkNCg0KPiAgVGhlbiwg
b24gdGhlIGZvbGxvd2luZyBBQ0NFU1MgY2FsbCB0aGF0IGdldHMgYmFjaw0KPiBhdHRyaWJ1dGVz
LCBpdCBjbGVhcnMgdGhhdCBmbGFnIHdpdGhvdXQgaW52YWxpZGF0aW5nIHRoZSBjYWNoZS4NCg0K
QWguLi4gSSBzZWUgdGhlIGJ1ZzoNCg0KbmZzX3dyaXRlX3BhZ2V1cHRvZGF0ZSgpIG5lZWRzIHRv
IGNoZWNrIGZvciBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRXwNCk5GU19JTk9fSU5WQUxJRF9EQVRB
Li4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIN
Cg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 18:00 ` Myklebust, Trond
@ 2012-12-10 18:08 ` Jeff Layton
2012-12-10 18:55 ` Myklebust, Trond
2012-12-10 18:28 ` Jeff Layton
1 sibling, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2012-12-10 18:08 UTC (permalink / raw)
To: Myklebust, Trond; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
On Mon, 10 Dec 2012 18:00:06 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> On Mon, 2012-12-10 at 12:20 -0500, Jeff Layton wrote:
> > On Mon, 10 Dec 2012 17:02:00 +0000
> > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> >
> > > On Mon, 2012-12-10 at 11:53 -0500, Jeff Layton wrote:
> > > > On Mon, 10 Dec 2012 16:28:47 +0000
> > > > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > > >
> > > > > On Mon, 2012-12-10 at 11:05 -0500, Jeff Layton wrote:
> > > > > > On Mon, 10 Dec 2012 14:42:23 +0000
> > > > > > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > > > > >
> > > > > > > On Mon, 2012-12-10 at 09:25 -0500, Jeff Layton wrote:
> > > > > > > > Jian reported that the following sequence would leave "testfile" with
> > > > > > > > corrupt data:
> > > > > > > >
> > > > > > > > # mount localhost:/export /mnt/nfs/ -o vers=3
> > > > > > > > # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
> > > > > > > > # cat -v /export/testfile
> > > > > > > > abc
> > > > > > > > ^@^@^@^@ghi
> > > > > > > >
> > > > > > > > While there's no locking involved here, the operations are serialized,
> > > > > > > > so CTO should prevent corruption.
> > > > > > > >
> > > > > > > > The first write to the file is fine and writes 4 bytes. The file is then
> > > > > > > > extended on the server. When it's reopened a GETATTR is issued and the
> > > > > > > > size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
> > > > > > > > the file. Because the file is opened for write only,
> > > > > > > > nfs_want_read_modify_write() returns 0 to nfs_write_begin().
> > > > > > > > nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
> > > > > > > > extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
> > > > > > > > still set on the file at that point, but that flag is ignored and
> > > > > > > > nfs_pageuptodate erroneously extends the write to cover the whole page,
> > > > > > > > with the write done on the server side filled in with zeroes.
> > > > > > > >
> > > > > > > > This patch just has that function check for NFS_INO_INVALID_DATA in
> > > > > > > > addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
> > > > > > > > over the code, I wonder if we might have a similar bug in
> > > > > > > > nfs_revalidate_size(). The difference between those two flags is very
> > > > > > > > subtle, so it seems like we ought to be checking for
> > > > > > > > NFS_INO_INVALID_DATA in most of the places that we look for
> > > > > > > > NFS_INO_REVAL_PAGECACHE.
> > > > > > > >
> > > > > > > > I believe this is regression introduced by commit 8d197a568. The code
> > > > > > > > did check for NFS_INO_INVALID_DATA prior to that patch.
> > > > > > > >
> > > > > > > > Original bug report is here:
> > > > > > > >
> > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=885743
> > > > > > >
> > > > > > > Hi Jeff,
> > > > > > >
> > > > > > > The point of NFS_INO_REVAL_PAGECACHE is to be able to distinguish
> > > > > > > between situations where the file size or change attribute is in doubt
> > > > > > > (and so the page cache validity is in doubt), and situations where other
> > > > > > > attributes are in doubt (such as ACCESS caches, file owner, nlinks,...).
> > > > > > > This is why we're only checking for NFS_INO_REVAL_PAGECACHE and not
> > > > > > > NFS_INO_INVALID_DATA in the functions you mention above.
> > > > > > >
> > > > > >
> > > > > > Ok, now I'm really confused... I thought that:
> > > > > >
> > > > > > NFS_INO_INVALID_DATA == pagecache data was known to be wrong
> > > > > > NFS_INO_REVAL_PAGECACHE == pagecache data is questionable
> > > > >
> > > > > There is no "known to be wrong" state. Both mean "needs revalidation".
> > > > >
> > > > > > ...but if I understand what you're saying above, the "data" that
> > > > > > NFS_INO_INVALID_DATA refers to is in peripheral caches, like the ACCESS
> > > > > > cache? I guess then I don't quite understand what NFS_INO_INVALID_ATTR
> > > > > > and NFS_INO_INVALID_ACCESS are for. Aren't they supposed to indicate that
> > > > > > the attrs and access caches are invalid?
> > > > >
> > > > > NFS_INO_INVALID_ATTR basically means that some aspect of the file
> > > > > metadata needs revalidation, and so a call to nfs_revalidate_inode()
> > > > > will always result in a GETATTR rpc call.
> > > > >
> > > > > NFS_INO_INVALID_ACCESS means that the file access cache needs
> > > > > revalidation. A call to nfs_do_access() will always result in an ACCESS
> > > > > rpc call.
> > > > >
> > > > > NFS_INO_REVAL_PAGECACHE means that the file page cache data needs
> > > > > revalidation. A call to nfs_revalidate_file_size() or
> > > > > nfs_revalidate_mapping() will result in a revalidation of the page
> > > > > cache. In practice that means a GETATTR rpc call, but that may change if
> > > > > future versions of NFS allow for different methods of revalidating the
> > > > > page cache; consider, for instance, the byte-range delegation RFC that
> > > > > Bruce and I published a few years back.
> > > > >
> > > >
> > > > Ok, thanks. Another question though -- what does NFS_INO_INVALID_DATA
> > > > mean, and what distinguishes it from NFS_INO_REVAL_PAGECACHE?
> > >
> > > See above. They affect _different_ revalidation situations (represented
> > > by different revalidation functions).
> > >
> > > > > > > So the fix here should be to set NFS_INO_REVAL_PAGECACHE when the change
> > > > > > > attribute and/or size change is noticed. The only function I can see
> > > > > > > that appears to get this wrong is nfs_wcc_update_inode(). Does fixing
> > > > > > > that lay the bug to rest?
> > > > > > >
> > > > > >
> > > > > > No, that doesn't fix it. There's another place that misses setting that
> > > > > > flag too in nfs_update_inode() where the size changes. I added it there
> > > > > > too and that also didn't fix it. Here's why (I think):
> > > > > >
> > > > > > When the size change is noticed via the CTO GETATTR call, then my
> > > > > > patched kernel now sets NFS_INO_REVAL_PAGECACHE in nfs_update_inode.
> > > > > > Now cache_validity is set to 0x2a. The kernel then makes an ACCESS call
> > > > > > that also ultimately ends up in nfs_update_inode.
> > > > > >
> > > > > > nfs_update_inode saves off the cache_validity flags and clears all of
> > > > > > them except for NFS_INO_INVALID_DATA. As best I can tell, the
> > > > > > saved_cache_validity is then ending up discarded and on exit the
> > > > > > cache_validity is ending up set to just NFS_INO_INVALID_DATA.
> > > > > >
> > > > > > It seems like we ought to be restoring the save_cache_validity flags
> > > > > > before exiting that function, but I confess that I don't quite grasp
> > > > > > the logic in nfs_update_inode. Thoughts?
> > > > >
> > > > > If the ACCESS call isn't revalidating the file size, then it should not
> > > > > be clearing NFS_INO_REVAL_PAGECACHE; it does this by restoring that bit
> > > > > from the "save_cache_validity" variable and storing it in "invalid". As
> > > > > far as I can tell, that code is correct in upstream.
> > > > >
> > > >
> > > > FWIW, I'm working on a 3.7-rc8 kernel here...
> > > >
> > > > The ACCESS call ends up with an fattr that has NFS_ATTR_FATTR_SIZE set,
> > > > it just doesn't detect any change in size due to the fact that the CTO
> > > > GETATTR call has already updated it.
> > > >
> > > > ------------------[snip]------------------------
> > > > /* Check if our cached file size is stale */
> > > > if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
> > > > new_isize = nfs_size_to_loff_t(fattr->size);
> > > > cur_isize = i_size_read(inode);
> > > > if (new_isize != cur_isize) {
> > > > /* Do we perhaps have any outstanding writes, or has
> > > > * the file grown beyond our last write? */
> > > > if ((nfsi->npages == 0 && !test_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->flags)) ||
> > > > new_isize > cur_isize) {
> > > > i_size_write(inode, new_isize);
> > > > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
> > > > }
> > > > dprintk("NFS: isize change on server for file %s/%ld "
> > > > "(%Ld to %Ld)\n",
> > > > inode->i_sb->s_id,
> > > > inode->i_ino,
> > > > (long long)cur_isize,
> > > > (long long)new_isize);
> > > > }
> > > > } else
> > > > invalid |= save_cache_validity & (NFS_INO_INVALID_ATTR
> > > > | NFS_INO_REVAL_PAGECACHE
> > > > | NFS_INO_REVAL_FORCED);
> > > > ------------------[snip]------------------------
> > > >
> > > > So here, we only restore set the REVAL_PAGECACHE flag in invalid if the
> > > > size changed, or there was no size attribute present in the response.
> > > > The size hasn't changed since the GETATTR call, so REVAL_PAGECACHE is
> > > > not restored.
> > >
> > > I don't understand. Why does it need to be restored? The
> > > NFS_ATTR_FATTR_SIZE is set, so the server has sent us an updated value
> > > for the size. Is it lying to us?
> > >
> >
> > No, this is NFSv3 and IIUC, NFS_ATTR_FATTR_SIZE just means that
> > fattr->size is populated with a value sent by the server. In fact,
> > looking at decode_fattr3():
> >
> > fattr->valid |= NFS_ATTR_FATTR_V3;
> >
> > ...and NFS_ATTR_FATTR_V3 contains NFS_ATTR_FATTR_SIZE.
>
> Right. The NFSv3 server is always supposed to send us the correct value
> for the size.
>
> > Maybe this flag doesn't need to be restored. I really don't understand
> > the current logic in nfs_update_inode at all so I'll have to defer to
> > your judgement here.
> >
> > All I know is that when the CTO GETATTR call is done, the kernel sees
> > that the size has changed. Presumably, it sets NFS_INO_REVAL_PAGECACHE
> > to reflect that fact.
>
> It should also set NFS_INO_INVALID_DATA, which is _not_ cleared by
> nfs_update_inode()
>
> > Then, on the following ACCESS call that gets back
> > attributes, it clears that flag without invalidating the cache.
>
> Ah... I see the bug:
>
> nfs_write_pageuptodate() needs to check for NFS_INO_REVAL_PAGECACHE|
> NFS_INO_INVALID_DATA...
>
That's exactly what the original patch I sent did :)
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 18:00 ` Myklebust, Trond
2012-12-10 18:08 ` Jeff Layton
@ 2012-12-10 18:28 ` Jeff Layton
2012-12-10 19:07 ` Myklebust, Trond
1 sibling, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2012-12-10 18:28 UTC (permalink / raw)
To: Myklebust, Trond; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
On Mon, 10 Dec 2012 18:00:06 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> On Mon, 2012-12-10 at 12:20 -0500, Jeff Layton wrote:
> > On Mon, 10 Dec 2012 17:02:00 +0000
> > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> >
> > > On Mon, 2012-12-10 at 11:53 -0500, Jeff Layton wrote:
> > > > On Mon, 10 Dec 2012 16:28:47 +0000
> > > > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > > >
> > > > > On Mon, 2012-12-10 at 11:05 -0500, Jeff Layton wrote:
> > > > > > On Mon, 10 Dec 2012 14:42:23 +0000
> > > > > > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > > > > >
> > > > > > > On Mon, 2012-12-10 at 09:25 -0500, Jeff Layton wrote:
> > > > > > > > Jian reported that the following sequence would leave "testfile" with
> > > > > > > > corrupt data:
> > > > > > > >
> > > > > > > > # mount localhost:/export /mnt/nfs/ -o vers=3
> > > > > > > > # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
> > > > > > > > # cat -v /export/testfile
> > > > > > > > abc
> > > > > > > > ^@^@^@^@ghi
> > > > > > > >
> > > > > > > > While there's no locking involved here, the operations are serialized,
> > > > > > > > so CTO should prevent corruption.
> > > > > > > >
> > > > > > > > The first write to the file is fine and writes 4 bytes. The file is then
> > > > > > > > extended on the server. When it's reopened a GETATTR is issued and the
> > > > > > > > size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
> > > > > > > > the file. Because the file is opened for write only,
> > > > > > > > nfs_want_read_modify_write() returns 0 to nfs_write_begin().
> > > > > > > > nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
> > > > > > > > extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
> > > > > > > > still set on the file at that point, but that flag is ignored and
> > > > > > > > nfs_pageuptodate erroneously extends the write to cover the whole page,
> > > > > > > > with the write done on the server side filled in with zeroes.
> > > > > > > >
> > > > > > > > This patch just has that function check for NFS_INO_INVALID_DATA in
> > > > > > > > addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
> > > > > > > > over the code, I wonder if we might have a similar bug in
> > > > > > > > nfs_revalidate_size(). The difference between those two flags is very
> > > > > > > > subtle, so it seems like we ought to be checking for
> > > > > > > > NFS_INO_INVALID_DATA in most of the places that we look for
> > > > > > > > NFS_INO_REVAL_PAGECACHE.
> > > > > > > >
> > > > > > > > I believe this is regression introduced by commit 8d197a568. The code
> > > > > > > > did check for NFS_INO_INVALID_DATA prior to that patch.
> > > > > > > >
> > > > > > > > Original bug report is here:
> > > > > > > >
> > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=885743
> > > > > > >
> > > > > > > Hi Jeff,
> > > > > > >
> > > > > > > The point of NFS_INO_REVAL_PAGECACHE is to be able to distinguish
> > > > > > > between situations where the file size or change attribute is in doubt
> > > > > > > (and so the page cache validity is in doubt), and situations where other
> > > > > > > attributes are in doubt (such as ACCESS caches, file owner, nlinks,...).
> > > > > > > This is why we're only checking for NFS_INO_REVAL_PAGECACHE and not
> > > > > > > NFS_INO_INVALID_DATA in the functions you mention above.
> > > > > > >
> > > > > >
> > > > > > Ok, now I'm really confused... I thought that:
> > > > > >
> > > > > > NFS_INO_INVALID_DATA == pagecache data was known to be wrong
> > > > > > NFS_INO_REVAL_PAGECACHE == pagecache data is questionable
> > > > >
> > > > > There is no "known to be wrong" state. Both mean "needs revalidation".
> > > > >
> > > > > > ...but if I understand what you're saying above, the "data" that
> > > > > > NFS_INO_INVALID_DATA refers to is in peripheral caches, like the ACCESS
> > > > > > cache? I guess then I don't quite understand what NFS_INO_INVALID_ATTR
> > > > > > and NFS_INO_INVALID_ACCESS are for. Aren't they supposed to indicate that
> > > > > > the attrs and access caches are invalid?
> > > > >
> > > > > NFS_INO_INVALID_ATTR basically means that some aspect of the file
> > > > > metadata needs revalidation, and so a call to nfs_revalidate_inode()
> > > > > will always result in a GETATTR rpc call.
> > > > >
> > > > > NFS_INO_INVALID_ACCESS means that the file access cache needs
> > > > > revalidation. A call to nfs_do_access() will always result in an ACCESS
> > > > > rpc call.
> > > > >
> > > > > NFS_INO_REVAL_PAGECACHE means that the file page cache data needs
> > > > > revalidation. A call to nfs_revalidate_file_size() or
> > > > > nfs_revalidate_mapping() will result in a revalidation of the page
> > > > > cache. In practice that means a GETATTR rpc call, but that may change if
> > > > > future versions of NFS allow for different methods of revalidating the
> > > > > page cache; consider, for instance, the byte-range delegation RFC that
> > > > > Bruce and I published a few years back.
> > > > >
> > > >
> > > > Ok, thanks. Another question though -- what does NFS_INO_INVALID_DATA
> > > > mean, and what distinguishes it from NFS_INO_REVAL_PAGECACHE?
> > >
> > > See above. They affect _different_ revalidation situations (represented
> > > by different revalidation functions).
> > >
> > > > > > > So the fix here should be to set NFS_INO_REVAL_PAGECACHE when the change
> > > > > > > attribute and/or size change is noticed. The only function I can see
> > > > > > > that appears to get this wrong is nfs_wcc_update_inode(). Does fixing
> > > > > > > that lay the bug to rest?
> > > > > > >
> > > > > >
> > > > > > No, that doesn't fix it. There's another place that misses setting that
> > > > > > flag too in nfs_update_inode() where the size changes. I added it there
> > > > > > too and that also didn't fix it. Here's why (I think):
> > > > > >
> > > > > > When the size change is noticed via the CTO GETATTR call, then my
> > > > > > patched kernel now sets NFS_INO_REVAL_PAGECACHE in nfs_update_inode.
> > > > > > Now cache_validity is set to 0x2a. The kernel then makes an ACCESS call
> > > > > > that also ultimately ends up in nfs_update_inode.
> > > > > >
> > > > > > nfs_update_inode saves off the cache_validity flags and clears all of
> > > > > > them except for NFS_INO_INVALID_DATA. As best I can tell, the
> > > > > > saved_cache_validity is then ending up discarded and on exit the
> > > > > > cache_validity is ending up set to just NFS_INO_INVALID_DATA.
> > > > > >
> > > > > > It seems like we ought to be restoring the save_cache_validity flags
> > > > > > before exiting that function, but I confess that I don't quite grasp
> > > > > > the logic in nfs_update_inode. Thoughts?
> > > > >
> > > > > If the ACCESS call isn't revalidating the file size, then it should not
> > > > > be clearing NFS_INO_REVAL_PAGECACHE; it does this by restoring that bit
> > > > > from the "save_cache_validity" variable and storing it in "invalid". As
> > > > > far as I can tell, that code is correct in upstream.
> > > > >
> > > >
> > > > FWIW, I'm working on a 3.7-rc8 kernel here...
> > > >
> > > > The ACCESS call ends up with an fattr that has NFS_ATTR_FATTR_SIZE set,
> > > > it just doesn't detect any change in size due to the fact that the CTO
> > > > GETATTR call has already updated it.
> > > >
> > > > ------------------[snip]------------------------
> > > > /* Check if our cached file size is stale */
> > > > if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
> > > > new_isize = nfs_size_to_loff_t(fattr->size);
> > > > cur_isize = i_size_read(inode);
> > > > if (new_isize != cur_isize) {
> > > > /* Do we perhaps have any outstanding writes, or has
> > > > * the file grown beyond our last write? */
> > > > if ((nfsi->npages == 0 && !test_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->flags)) ||
> > > > new_isize > cur_isize) {
> > > > i_size_write(inode, new_isize);
> > > > invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
> > > > }
> > > > dprintk("NFS: isize change on server for file %s/%ld "
> > > > "(%Ld to %Ld)\n",
> > > > inode->i_sb->s_id,
> > > > inode->i_ino,
> > > > (long long)cur_isize,
> > > > (long long)new_isize);
> > > > }
> > > > } else
> > > > invalid |= save_cache_validity & (NFS_INO_INVALID_ATTR
> > > > | NFS_INO_REVAL_PAGECACHE
> > > > | NFS_INO_REVAL_FORCED);
> > > > ------------------[snip]------------------------
> > > >
> > > > So here, we only restore set the REVAL_PAGECACHE flag in invalid if the
> > > > size changed, or there was no size attribute present in the response.
> > > > The size hasn't changed since the GETATTR call, so REVAL_PAGECACHE is
> > > > not restored.
> > >
> > > I don't understand. Why does it need to be restored? The
> > > NFS_ATTR_FATTR_SIZE is set, so the server has sent us an updated value
> > > for the size. Is it lying to us?
> > >
> >
> > No, this is NFSv3 and IIUC, NFS_ATTR_FATTR_SIZE just means that
> > fattr->size is populated with a value sent by the server. In fact,
> > looking at decode_fattr3():
> >
> > fattr->valid |= NFS_ATTR_FATTR_V3;
> >
> > ...and NFS_ATTR_FATTR_V3 contains NFS_ATTR_FATTR_SIZE.
>
> Right. The NFSv3 server is always supposed to send us the correct value
> for the size.
>
> > Maybe this flag doesn't need to be restored. I really don't understand
> > the current logic in nfs_update_inode at all so I'll have to defer to
> > your judgement here.
> >
> > All I know is that when the CTO GETATTR call is done, the kernel sees
> > that the size has changed. Presumably, it sets NFS_INO_REVAL_PAGECACHE
> > to reflect that fact.
>
> It should also set NFS_INO_INVALID_DATA, which is _not_ cleared by
> nfs_update_inode()
>
> > Then, on the following ACCESS call that gets back
> > attributes, it clears that flag without invalidating the cache.
>
> Ah... I see the bug:
>
> nfs_write_pageuptodate() needs to check for NFS_INO_REVAL_PAGECACHE|
> NFS_INO_INVALID_DATA...
>
One more question though...
I don't have a reproducer for it, but it looks like
nfs_revalidate_file_size() might have a similar bug. Should we also be
checking for NFS_INO_INVALID_DATA there too?
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 18:08 ` Jeff Layton
@ 2012-12-10 18:55 ` Myklebust, Trond
0 siblings, 0 replies; 12+ messages in thread
From: Myklebust, Trond @ 2012-12-10 18:55 UTC (permalink / raw)
To: Jeff Layton; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
T24gTW9uLCAyMDEyLTEyLTEwIGF0IDEzOjA4IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxODowMDowNiArMDAwMA0KPiAiTXlrbGVidXN0LCBUcm9uZCIg
PFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+ID4gT24gTW9uLCAyMDEy
LTEyLTEwIGF0IDEyOjIwIC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4gPiA+IE9uIE1vbiwg
MTAgRGVjIDIwMTIgMTc6MDI6MDAgKzAwMDANCj4gPiA+ICJNeWtsZWJ1c3QsIFRyb25kIiA8VHJv
bmQuTXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiA+ID4gDQo+ID4gPiA+IE9uIE1vbiwg
MjAxMi0xMi0xMCBhdCAxMTo1MyAtMDUwMCwgSmVmZiBMYXl0b24gd3JvdGU6DQo+ID4gPiA+ID4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxNjoyODo0NyArMDAwMA0KPiA+ID4gPiA+ICJNeWtsZWJ1c3Qs
IFRyb25kIiA8VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiA+ID4gPiA+IA0K
PiA+ID4gPiA+ID4gT24gTW9uLCAyMDEyLTEyLTEwIGF0IDExOjA1IC0wNTAwLCBKZWZmIExheXRv
biB3cm90ZToNCj4gPiA+ID4gPiA+ID4gT24gTW9uLCAxMCBEZWMgMjAxMiAxNDo0MjoyMyArMDAw
MA0KPiA+ID4gPiA+ID4gPiAiTXlrbGVidXN0LCBUcm9uZCIgPFRyb25kLk15a2xlYnVzdEBuZXRh
cHAuY29tPiB3cm90ZToNCj4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+ID4gT24gTW9uLCAy
MDEyLTEyLTEwIGF0IDA5OjI1IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4gPiA+ID4gPiA+
ID4gPiA+IEppYW4gcmVwb3J0ZWQgdGhhdCB0aGUgZm9sbG93aW5nIHNlcXVlbmNlIHdvdWxkIGxl
YXZlICJ0ZXN0ZmlsZSIgd2l0aA0KPiA+ID4gPiA+ID4gPiA+ID4gY29ycnVwdCBkYXRhOg0KPiA+
ID4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+ID4gPiAgICAgIyBtb3VudCBsb2NhbGhvc3Q6
L2V4cG9ydCAvbW50L25mcy8gLW8gdmVycz0zDQo+ID4gPiA+ID4gPiA+ID4gPiAgICAgIyBlY2hv
IGFiYyA+IC9tbnQvbmZzL3Rlc3RmaWxlOyBlY2hvIGRlZiA+PiAvZXhwb3J0L3Rlc3RmaWxlOyBl
Y2hvIGdoaSA+PiAvbW50L25mcy90ZXN0ZmlsZQ0KPiA+ID4gPiA+ID4gPiA+ID4gICAgICMgY2F0
IC12IC9leHBvcnQvdGVzdGZpbGUNCj4gPiA+ID4gPiA+ID4gPiA+ICAgICBhYmMNCj4gPiA+ID4g
PiA+ID4gPiA+ICAgICBeQF5AXkBeQGdoaQ0KPiA+ID4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4g
PiA+ID4gPiBXaGlsZSB0aGVyZSdzIG5vIGxvY2tpbmcgaW52b2x2ZWQgaGVyZSwgdGhlIG9wZXJh
dGlvbnMgYXJlIHNlcmlhbGl6ZWQsDQo+ID4gPiA+ID4gPiA+ID4gPiBzbyBDVE8gc2hvdWxkIHBy
ZXZlbnQgY29ycnVwdGlvbi4NCj4gPiA+ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiA+ID4g
VGhlIGZpcnN0IHdyaXRlIHRvIHRoZSBmaWxlIGlzIGZpbmUgYW5kIHdyaXRlcyA0IGJ5dGVzLiBU
aGUgZmlsZSBpcyB0aGVuDQo+ID4gPiA+ID4gPiA+ID4gPiBleHRlbmRlZCBvbiB0aGUgc2VydmVy
LiBXaGVuIGl0J3MgcmVvcGVuZWQgYSBHRVRBVFRSIGlzIGlzc3VlZCBhbmQgdGhlDQo+ID4gPiA+
ID4gPiA+ID4gPiBzaXplIGNoYW5nZSBpcyBub3RpY2VkLiBUaGlzIGNhdXNlcyBORlNfSU5PX0lO
VkFMSURfREFUQSB0byBiZSBzZXQgb24NCj4gPiA+ID4gPiA+ID4gPiA+IHRoZSBmaWxlLiBCZWNh
dXNlIHRoZSBmaWxlIGlzIG9wZW5lZCBmb3Igd3JpdGUgb25seSwNCj4gPiA+ID4gPiA+ID4gPiA+
IG5mc193YW50X3JlYWRfbW9kaWZ5X3dyaXRlKCkgcmV0dXJucyAwIHRvIG5mc193cml0ZV9iZWdp
bigpLg0KPiA+ID4gPiA+ID4gPiA+ID4gbmZzX3VwZGF0ZXBhZ2UgdGhlbiBjYWxscyBuZnNfd3Jp
dGVfcGFnZXVwdG9kYXRlKCkgdG8gc2VlIGlmIGl0IHNob3VsZA0KPiA+ID4gPiA+ID4gPiA+ID4g
ZXh0ZW5kIHRoZSBuZnNfcGFnZSB0byBjb3ZlciB0aGUgd2hvbGUgcGFnZS4gTkZTX0lOT19JTlZB
TElEX0RBVEEgaXMNCj4gPiA+ID4gPiA+ID4gPiA+IHN0aWxsIHNldCBvbiB0aGUgZmlsZSBhdCB0
aGF0IHBvaW50LCBidXQgdGhhdCBmbGFnIGlzIGlnbm9yZWQgYW5kDQo+ID4gPiA+ID4gPiA+ID4g
PiBuZnNfcGFnZXVwdG9kYXRlIGVycm9uZW91c2x5IGV4dGVuZHMgdGhlIHdyaXRlIHRvIGNvdmVy
IHRoZSB3aG9sZSBwYWdlLA0KPiA+ID4gPiA+ID4gPiA+ID4gd2l0aCB0aGUgd3JpdGUgZG9uZSBv
biB0aGUgc2VydmVyIHNpZGUgZmlsbGVkIGluIHdpdGggemVyb2VzLg0KPiA+ID4gPiA+ID4gPiA+
ID4gDQo+ID4gPiA+ID4gPiA+ID4gPiBUaGlzIHBhdGNoIGp1c3QgaGFzIHRoYXQgZnVuY3Rpb24g
Y2hlY2sgZm9yIE5GU19JTk9fSU5WQUxJRF9EQVRBIGluDQo+ID4gPiA+ID4gPiA+ID4gPiBhZGRp
dGlvbiB0byBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRS4gVGhpcyBmaXhlcyB0aGUgYnVnLCBidXQg
bG9va2luZw0KPiA+ID4gPiA+ID4gPiA+ID4gb3ZlciB0aGUgY29kZSwgSSB3b25kZXIgaWYgd2Ug
bWlnaHQgaGF2ZSBhIHNpbWlsYXIgYnVnIGluDQo+ID4gPiA+ID4gPiA+ID4gPiBuZnNfcmV2YWxp
ZGF0ZV9zaXplKCkuIFRoZSBkaWZmZXJlbmNlIGJldHdlZW4gdGhvc2UgdHdvIGZsYWdzIGlzIHZl
cnkNCj4gPiA+ID4gPiA+ID4gPiA+IHN1YnRsZSwgc28gaXQgc2VlbXMgbGlrZSB3ZSBvdWdodCB0
byBiZSBjaGVja2luZyBmb3INCj4gPiA+ID4gPiA+ID4gPiA+IE5GU19JTk9fSU5WQUxJRF9EQVRB
IGluIG1vc3Qgb2YgdGhlIHBsYWNlcyB0aGF0IHdlIGxvb2sgZm9yDQo+ID4gPiA+ID4gPiA+ID4g
PiBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRS4NCj4gPiA+ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+
ID4gPiA+ID4gSSBiZWxpZXZlIHRoaXMgaXMgcmVncmVzc2lvbiBpbnRyb2R1Y2VkIGJ5IGNvbW1p
dCA4ZDE5N2E1NjguIFRoZSBjb2RlDQo+ID4gPiA+ID4gPiA+ID4gPiBkaWQgY2hlY2sgZm9yIE5G
U19JTk9fSU5WQUxJRF9EQVRBIHByaW9yIHRvIHRoYXQgcGF0Y2guDQo+ID4gPiA+ID4gPiA+ID4g
PiANCj4gPiA+ID4gPiA+ID4gPiA+IE9yaWdpbmFsIGJ1ZyByZXBvcnQgaXMgaGVyZToNCj4gPiA+
ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiA+ID4gICAgIGh0dHBzOi8vYnVnemlsbGEucmVk
aGF0LmNvbS9zaG93X2J1Zy5jZ2k/aWQ9ODg1NzQzDQo+ID4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+
ID4gPiA+ID4gSGkgSmVmZiwNCj4gPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gPiBUaGUg
cG9pbnQgb2YgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgaXMgdG8gYmUgYWJsZSB0byBkaXN0aW5n
dWlzaA0KPiA+ID4gPiA+ID4gPiA+IGJldHdlZW4gc2l0dWF0aW9ucyB3aGVyZSB0aGUgZmlsZSBz
aXplIG9yIGNoYW5nZSBhdHRyaWJ1dGUgaXMgaW4gZG91YnQNCj4gPiA+ID4gPiA+ID4gPiAoYW5k
IHNvIHRoZSBwYWdlIGNhY2hlIHZhbGlkaXR5IGlzIGluIGRvdWJ0KSwgYW5kIHNpdHVhdGlvbnMg
d2hlcmUgb3RoZXINCj4gPiA+ID4gPiA+ID4gPiBhdHRyaWJ1dGVzIGFyZSBpbiBkb3VidCAoc3Vj
aCBhcyBBQ0NFU1MgY2FjaGVzLCBmaWxlIG93bmVyLCBubGlua3MsLi4uKS4NCj4gPiA+ID4gPiA+
ID4gPiBUaGlzIGlzIHdoeSB3ZSdyZSBvbmx5IGNoZWNraW5nIGZvciBORlNfSU5PX1JFVkFMX1BB
R0VDQUNIRSBhbmQgbm90DQo+ID4gPiA+ID4gPiA+ID4gTkZTX0lOT19JTlZBTElEX0RBVEEgaW4g
dGhlIGZ1bmN0aW9ucyB5b3UgbWVudGlvbiBhYm92ZS4NCj4gPiA+ID4gPiA+ID4gPiANCj4gPiA+
ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+IE9rLCBub3cgSSdtIHJlYWxseSBjb25mdXNlZC4uLiBJ
IHRob3VnaHQgdGhhdDoNCj4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+IE5GU19JTk9fSU5W
QUxJRF9EQVRBID09IHBhZ2VjYWNoZSBkYXRhIHdhcyBrbm93biB0byBiZSB3cm9uZw0KPiA+ID4g
PiA+ID4gPiBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRSA9PSBwYWdlY2FjaGUgZGF0YSBpcyBxdWVz
dGlvbmFibGUNCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gVGhlcmUgaXMgbm8gImtub3duIHRv
IGJlIHdyb25nIiBzdGF0ZS4gQm90aCBtZWFuICJuZWVkcyByZXZhbGlkYXRpb24iLg0KPiA+ID4g
PiA+ID4gDQo+ID4gPiA+ID4gPiA+IC4uLmJ1dCBpZiBJIHVuZGVyc3RhbmQgd2hhdCB5b3UncmUg
c2F5aW5nIGFib3ZlLCB0aGUgImRhdGEiIHRoYXQNCj4gPiA+ID4gPiA+ID4gTkZTX0lOT19JTlZB
TElEX0RBVEEgcmVmZXJzIHRvIGlzIGluIHBlcmlwaGVyYWwgY2FjaGVzLCBsaWtlIHRoZSBBQ0NF
U1MNCj4gPiA+ID4gPiA+ID4gY2FjaGU/IEkgZ3Vlc3MgdGhlbiBJIGRvbid0IHF1aXRlIHVuZGVy
c3RhbmQgd2hhdCBORlNfSU5PX0lOVkFMSURfQVRUUg0KPiA+ID4gPiA+ID4gPiBhbmQgTkZTX0lO
T19JTlZBTElEX0FDQ0VTUyBhcmUgZm9yLiBBcmVuJ3QgdGhleSBzdXBwb3NlZCB0byBpbmRpY2F0
ZSB0aGF0DQo+ID4gPiA+ID4gPiA+IHRoZSBhdHRycyBhbmQgYWNjZXNzIGNhY2hlcyBhcmUgaW52
YWxpZD8NCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gTkZTX0lOT19JTlZBTElEX0FUVFIgYmFz
aWNhbGx5IG1lYW5zIHRoYXQgc29tZSBhc3BlY3Qgb2YgdGhlIGZpbGUNCj4gPiA+ID4gPiA+IG1l
dGFkYXRhIG5lZWRzIHJldmFsaWRhdGlvbiwgYW5kIHNvIGEgY2FsbCB0byBuZnNfcmV2YWxpZGF0
ZV9pbm9kZSgpDQo+ID4gPiA+ID4gPiB3aWxsIGFsd2F5cyByZXN1bHQgaW4gYSBHRVRBVFRSIHJw
YyBjYWxsLg0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiBORlNfSU5PX0lOVkFMSURfQUNDRVNT
IG1lYW5zIHRoYXQgdGhlIGZpbGUgYWNjZXNzIGNhY2hlIG5lZWRzDQo+ID4gPiA+ID4gPiByZXZh
bGlkYXRpb24uIEEgY2FsbCB0byBuZnNfZG9fYWNjZXNzKCkgd2lsbCBhbHdheXMgcmVzdWx0IGlu
IGFuIEFDQ0VTUw0KPiA+ID4gPiA+ID4gcnBjIGNhbGwuDQo+ID4gPiA+ID4gPiANCj4gPiA+ID4g
PiA+IE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFIG1lYW5zIHRoYXQgdGhlIGZpbGUgcGFnZSBjYWNo
ZSBkYXRhIG5lZWRzDQo+ID4gPiA+ID4gPiByZXZhbGlkYXRpb24uIEEgY2FsbCB0byBuZnNfcmV2
YWxpZGF0ZV9maWxlX3NpemUoKSBvcg0KPiA+ID4gPiA+ID4gbmZzX3JldmFsaWRhdGVfbWFwcGlu
ZygpIHdpbGwgcmVzdWx0IGluIGEgcmV2YWxpZGF0aW9uIG9mIHRoZSBwYWdlDQo+ID4gPiA+ID4g
PiBjYWNoZS4gSW4gcHJhY3RpY2UgdGhhdCBtZWFucyBhIEdFVEFUVFIgcnBjIGNhbGwsIGJ1dCB0
aGF0IG1heSBjaGFuZ2UgaWYNCj4gPiA+ID4gPiA+IGZ1dHVyZSB2ZXJzaW9ucyBvZiBORlMgYWxs
b3cgZm9yIGRpZmZlcmVudCBtZXRob2RzIG9mIHJldmFsaWRhdGluZyB0aGUNCj4gPiA+ID4gPiA+
IHBhZ2UgY2FjaGU7IGNvbnNpZGVyLCBmb3IgaW5zdGFuY2UsIHRoZSBieXRlLXJhbmdlIGRlbGVn
YXRpb24gUkZDIHRoYXQNCj4gPiA+ID4gPiA+IEJydWNlIGFuZCBJIHB1Ymxpc2hlZCBhIGZldyB5
ZWFycyBiYWNrLg0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gT2ssIHRoYW5r
cy4gQW5vdGhlciBxdWVzdGlvbiB0aG91Z2ggLS0gd2hhdCBkb2VzIE5GU19JTk9fSU5WQUxJRF9E
QVRBDQo+ID4gPiA+ID4gbWVhbiwgYW5kIHdoYXQgZGlzdGluZ3Vpc2hlcyBpdCBmcm9tIE5GU19J
Tk9fUkVWQUxfUEFHRUNBQ0hFPw0KPiA+ID4gPiANCj4gPiA+ID4gU2VlIGFib3ZlLiBUaGV5IGFm
ZmVjdCBfZGlmZmVyZW50XyByZXZhbGlkYXRpb24gc2l0dWF0aW9ucyAocmVwcmVzZW50ZWQNCj4g
PiA+ID4gYnkgZGlmZmVyZW50IHJldmFsaWRhdGlvbiBmdW5jdGlvbnMpLg0KPiA+ID4gPiANCj4g
PiA+ID4gPiA+ID4gPiBTbyB0aGUgZml4IGhlcmUgc2hvdWxkIGJlIHRvIHNldCBORlNfSU5PX1JF
VkFMX1BBR0VDQUNIRSB3aGVuIHRoZSBjaGFuZ2UNCj4gPiA+ID4gPiA+ID4gPiBhdHRyaWJ1dGUg
YW5kL29yIHNpemUgY2hhbmdlIGlzIG5vdGljZWQuIFRoZSBvbmx5IGZ1bmN0aW9uIEkgY2FuIHNl
ZQ0KPiA+ID4gPiA+ID4gPiA+IHRoYXQgYXBwZWFycyB0byBnZXQgdGhpcyB3cm9uZyBpcyBuZnNf
d2NjX3VwZGF0ZV9pbm9kZSgpLiBEb2VzIGZpeGluZw0KPiA+ID4gPiA+ID4gPiA+IHRoYXQgbGF5
IHRoZSBidWcgdG8gcmVzdD8NCj4gPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gDQo+ID4g
PiA+ID4gPiA+IE5vLCB0aGF0IGRvZXNuJ3QgZml4IGl0LiBUaGVyZSdzIGFub3RoZXIgcGxhY2Ug
dGhhdCBtaXNzZXMgc2V0dGluZyB0aGF0DQo+ID4gPiA+ID4gPiA+IGZsYWcgdG9vIGluIG5mc191
cGRhdGVfaW5vZGUoKSB3aGVyZSB0aGUgc2l6ZSBjaGFuZ2VzLiBJIGFkZGVkIGl0IHRoZXJlDQo+
ID4gPiA+ID4gPiA+IHRvbyBhbmQgdGhhdCBhbHNvIGRpZG4ndCBmaXggaXQuIEhlcmUncyB3aHkg
KEkgdGhpbmspOg0KPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gV2hlbiB0aGUgc2l6ZSBj
aGFuZ2UgaXMgbm90aWNlZCB2aWEgdGhlIENUTyBHRVRBVFRSIGNhbGwsIHRoZW4gbXkNCj4gPiA+
ID4gPiA+ID4gcGF0Y2hlZCBrZXJuZWwgbm93IHNldHMgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUg
aW4gbmZzX3VwZGF0ZV9pbm9kZS4NCj4gPiA+ID4gPiA+ID4gTm93IGNhY2hlX3ZhbGlkaXR5IGlz
IHNldCB0byAweDJhLiBUaGUga2VybmVsIHRoZW4gbWFrZXMgYW4gQUNDRVNTIGNhbGwNCj4gPiA+
ID4gPiA+ID4gdGhhdCBhbHNvIHVsdGltYXRlbHkgZW5kcyB1cCBpbiBuZnNfdXBkYXRlX2lub2Rl
Lg0KPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gbmZzX3VwZGF0ZV9pbm9kZSBzYXZlcyBv
ZmYgdGhlIGNhY2hlX3ZhbGlkaXR5IGZsYWdzIGFuZCBjbGVhcnMgYWxsIG9mDQo+ID4gPiA+ID4g
PiA+IHRoZW0gZXhjZXB0IGZvciBORlNfSU5PX0lOVkFMSURfREFUQS4gQXMgYmVzdCBJIGNhbiB0
ZWxsLCB0aGUNCj4gPiA+ID4gPiA+ID4gc2F2ZWRfY2FjaGVfdmFsaWRpdHkgaXMgdGhlbiBlbmRp
bmcgdXAgZGlzY2FyZGVkIGFuZCBvbiBleGl0IHRoZQ0KPiA+ID4gPiA+ID4gPiBjYWNoZV92YWxp
ZGl0eSBpcyBlbmRpbmcgdXAgc2V0IHRvIGp1c3QgTkZTX0lOT19JTlZBTElEX0RBVEEuDQo+ID4g
PiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiBJdCBzZWVtcyBsaWtlIHdlIG91Z2h0IHRvIGJlIHJl
c3RvcmluZyB0aGUgc2F2ZV9jYWNoZV92YWxpZGl0eSBmbGFncw0KPiA+ID4gPiA+ID4gPiBiZWZv
cmUgZXhpdGluZyB0aGF0IGZ1bmN0aW9uLCBidXQgSSBjb25mZXNzIHRoYXQgSSBkb24ndCBxdWl0
ZSBncmFzcA0KPiA+ID4gPiA+ID4gPiB0aGUgbG9naWMgaW4gbmZzX3VwZGF0ZV9pbm9kZS4gVGhv
dWdodHM/DQo+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+IElmIHRoZSBBQ0NFU1MgY2FsbCBpc24n
dCByZXZhbGlkYXRpbmcgdGhlIGZpbGUgc2l6ZSwgdGhlbiBpdCBzaG91bGQgbm90DQo+ID4gPiA+
ID4gPiBiZSBjbGVhcmluZyBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRTsgaXQgZG9lcyB0aGlzIGJ5
IHJlc3RvcmluZyB0aGF0IGJpdA0KPiA+ID4gPiA+ID4gZnJvbSB0aGUgInNhdmVfY2FjaGVfdmFs
aWRpdHkiIHZhcmlhYmxlIGFuZCBzdG9yaW5nIGl0IGluICJpbnZhbGlkIi4gQXMNCj4gPiA+ID4g
PiA+IGZhciBhcyBJIGNhbiB0ZWxsLCB0aGF0IGNvZGUgaXMgY29ycmVjdCBpbiB1cHN0cmVhbS4N
Cj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IEZXSVcsIEknbSB3b3JraW5nIG9u
IGEgMy43LXJjOCBrZXJuZWwgaGVyZS4uLg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IFRoZSBBQ0NF
U1MgY2FsbCBlbmRzIHVwIHdpdGggYW4gZmF0dHIgdGhhdCBoYXMgTkZTX0FUVFJfRkFUVFJfU0la
RSBzZXQsDQo+ID4gPiA+ID4gaXQganVzdCBkb2Vzbid0IGRldGVjdCBhbnkgY2hhbmdlIGluIHNp
emUgZHVlIHRvIHRoZSBmYWN0IHRoYXQgdGhlIENUTw0KPiA+ID4gPiA+IEdFVEFUVFIgY2FsbCBo
YXMgYWxyZWFkeSB1cGRhdGVkIGl0Lg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IC0tLS0tLS0tLS0t
LS0tLS0tLVtzbmlwXS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPiA+ID4gPiA+ICAgICAgICAg
LyogQ2hlY2sgaWYgb3VyIGNhY2hlZCBmaWxlIHNpemUgaXMgc3RhbGUgKi8NCj4gPiA+ID4gPiAg
ICAgICAgIGlmIChmYXR0ci0+dmFsaWQgJiBORlNfQVRUUl9GQVRUUl9TSVpFKSB7DQo+ID4gPiA+
ID4gICAgICAgICAgICAgICAgIG5ld19pc2l6ZSA9IG5mc19zaXplX3RvX2xvZmZfdChmYXR0ci0+
c2l6ZSk7DQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgIGN1cl9pc2l6ZSA9IGlfc2l6ZV9yZWFk
KGlub2RlKTsNCj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgaWYgKG5ld19pc2l6ZSAhPSBjdXJf
aXNpemUpIHsNCj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAvKiBEbyB3ZSBwZXJo
YXBzIGhhdmUgYW55IG91dHN0YW5kaW5nIHdyaXRlcywgb3IgaGFzDQo+ID4gPiA+ID4gICAgICAg
ICAgICAgICAgICAgICAgICAgICogdGhlIGZpbGUgZ3Jvd24gYmV5b25kIG91ciBsYXN0IHdyaXRl
PyAqLw0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIGlmICgobmZzaS0+bnBhZ2Vz
ID09IDAgJiYgIXRlc3RfYml0KE5GU19JTk9fTEFZT1VUQ09NTUlULCAmbmZzaS0+ZmxhZ3MpKSB8
fA0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgbmV3X2lzaXplID4gY3Vy
X2lzaXplKSB7DQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBpX3Np
emVfd3JpdGUoaW5vZGUsIG5ld19pc2l6ZSk7DQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBpbnZhbGlkIHw9IE5GU19JTk9fSU5WQUxJRF9BVFRSfE5GU19JTk9fSU5W
QUxJRF9EQVRBOw0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIH0NCj4gPiA+ID4g
PiAgICAgICAgICAgICAgICAgICAgICAgICBkcHJpbnRrKCJORlM6IGlzaXplIGNoYW5nZSBvbiBz
ZXJ2ZXIgZm9yIGZpbGUgJXMvJWxkICINCj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgIiglTGQgdG8gJUxkKVxuIiwNCj4gPiA+ID4gPiAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaW5vZGUtPmlfc2ItPnNfaWQsDQo+ID4gPiA+
ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGlub2RlLT5pX2lubywN
Cj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKGxvbmcg
bG9uZyljdXJfaXNpemUsDQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgIChsb25nIGxvbmcpbmV3X2lzaXplKTsNCj4gPiA+ID4gPiAgICAgICAgICAgICAg
ICAgfQ0KPiA+ID4gPiA+ICAgICAgICAgfSBlbHNlDQo+ID4gPiA+ID4gICAgICAgICAgICAgICAg
IGludmFsaWQgfD0gc2F2ZV9jYWNoZV92YWxpZGl0eSAmIChORlNfSU5PX0lOVkFMSURfQVRUUg0K
PiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCBORlNfSU5PX1JFVkFM
X1BBR0VDQUNIRQ0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCBO
RlNfSU5PX1JFVkFMX0ZPUkNFRCk7DQo+ID4gPiA+ID4gLS0tLS0tLS0tLS0tLS0tLS0tW3NuaXBd
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gU28gaGVyZSwg
d2Ugb25seSByZXN0b3JlIHNldCB0aGUgUkVWQUxfUEFHRUNBQ0hFIGZsYWcgaW4gaW52YWxpZCBp
ZiB0aGUNCj4gPiA+ID4gPiBzaXplIGNoYW5nZWQsIG9yIHRoZXJlIHdhcyBubyBzaXplIGF0dHJp
YnV0ZSBwcmVzZW50IGluIHRoZSByZXNwb25zZS4NCj4gPiA+ID4gPiBUaGUgc2l6ZSBoYXNuJ3Qg
Y2hhbmdlZCBzaW5jZSB0aGUgR0VUQVRUUiBjYWxsLCBzbyBSRVZBTF9QQUdFQ0FDSEUgaXMNCj4g
PiA+ID4gPiBub3QgcmVzdG9yZWQuDQo+ID4gPiA+IA0KPiA+ID4gPiBJIGRvbid0IHVuZGVyc3Rh
bmQuIFdoeSBkb2VzIGl0IG5lZWQgdG8gYmUgcmVzdG9yZWQ/IFRoZQ0KPiA+ID4gPiBORlNfQVRU
Ul9GQVRUUl9TSVpFIGlzIHNldCwgc28gdGhlIHNlcnZlciBoYXMgc2VudCB1cyBhbiB1cGRhdGVk
IHZhbHVlDQo+ID4gPiA+IGZvciB0aGUgc2l6ZS4gSXMgaXQgbHlpbmcgdG8gdXM/DQo+ID4gPiA+
IA0KPiA+ID4gDQo+ID4gPiBObywgdGhpcyBpcyBORlN2MyBhbmQgSUlVQywgTkZTX0FUVFJfRkFU
VFJfU0laRSBqdXN0IG1lYW5zIHRoYXQNCj4gPiA+IGZhdHRyLT5zaXplIGlzIHBvcHVsYXRlZCB3
aXRoIGEgdmFsdWUgc2VudCBieSB0aGUgc2VydmVyLiBJbiBmYWN0LA0KPiA+ID4gbG9va2luZyBh
dCBkZWNvZGVfZmF0dHIzKCk6DQo+ID4gPiANCj4gPiA+ICAgICAgICAgZmF0dHItPnZhbGlkIHw9
IE5GU19BVFRSX0ZBVFRSX1YzOw0KPiA+ID4gDQo+ID4gPiAuLi5hbmQgTkZTX0FUVFJfRkFUVFJf
VjMgY29udGFpbnMgTkZTX0FUVFJfRkFUVFJfU0laRS4NCj4gPiANCj4gPiBSaWdodC4gVGhlIE5G
U3YzIHNlcnZlciBpcyBhbHdheXMgc3VwcG9zZWQgdG8gc2VuZCB1cyB0aGUgY29ycmVjdCB2YWx1
ZQ0KPiA+IGZvciB0aGUgc2l6ZS4NCj4gPiANCj4gPiA+IE1heWJlIHRoaXMgZmxhZyBkb2Vzbid0
IG5lZWQgdG8gYmUgcmVzdG9yZWQuIEkgcmVhbGx5IGRvbid0IHVuZGVyc3RhbmQNCj4gPiA+IHRo
ZSBjdXJyZW50IGxvZ2ljIGluIG5mc191cGRhdGVfaW5vZGUgYXQgYWxsIHNvIEknbGwgaGF2ZSB0
byBkZWZlciB0bw0KPiA+ID4geW91ciBqdWRnZW1lbnQgaGVyZS4NCj4gPiA+IA0KPiA+ID4gQWxs
IEkga25vdyBpcyB0aGF0IHdoZW4gdGhlIENUTyBHRVRBVFRSIGNhbGwgaXMgZG9uZSwgdGhlIGtl
cm5lbCBzZWVzDQo+ID4gPiB0aGF0IHRoZSBzaXplIGhhcyBjaGFuZ2VkLiBQcmVzdW1hYmx5LCBp
dCBzZXRzIE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFDQo+ID4gPiB0byByZWZsZWN0IHRoYXQgZmFj
dC4NCj4gPiANCj4gPiBJdCBzaG91bGQgYWxzbyBzZXQgTkZTX0lOT19JTlZBTElEX0RBVEEsIHdo
aWNoIGlzIF9ub3RfIGNsZWFyZWQgYnkNCj4gPiBuZnNfdXBkYXRlX2lub2RlKCkNCj4gPiANCj4g
PiA+ICBUaGVuLCBvbiB0aGUgZm9sbG93aW5nIEFDQ0VTUyBjYWxsIHRoYXQgZ2V0cyBiYWNrDQo+
ID4gPiBhdHRyaWJ1dGVzLCBpdCBjbGVhcnMgdGhhdCBmbGFnIHdpdGhvdXQgaW52YWxpZGF0aW5n
IHRoZSBjYWNoZS4NCj4gPiANCj4gPiBBaC4uLiBJIHNlZSB0aGUgYnVnOg0KPiA+IA0KPiA+IG5m
c193cml0ZV9wYWdldXB0b2RhdGUoKSBuZWVkcyB0byBjaGVjayBmb3IgTkZTX0lOT19SRVZBTF9Q
QUdFQ0FDSEV8DQo+ID4gTkZTX0lOT19JTlZBTElEX0RBVEEuLi4NCj4gPiANCj4gDQo+IFRoYXQn
cyBleGFjdGx5IHdoYXQgdGhlIG9yaWdpbmFsIHBhdGNoIEkgc2VudCBkaWQgOikNCg0KRG9oISBJ
IHdhcyByZWFkaW5nIGl0IGFzIGNoYW5naW5nIHRoZSBzdHVmZiBpbiBmaWxlLmMuDQoNClBsZWFz
ZSBibGFtZSBvbiBhY3V0ZSBqZXRsYWcuIEkganVzdCBnb3QgaG9tZSBmcm9tIGEgaGFsZi13ZWVr
IGxvbmcgdHJpcA0KdG8gTm9yd2F5Li4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBO
RlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNv
bQ0Kd3d3Lm5ldGFwcC5jb20NCg==
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid
2012-12-10 18:28 ` Jeff Layton
@ 2012-12-10 19:07 ` Myklebust, Trond
0 siblings, 0 replies; 12+ messages in thread
From: Myklebust, Trond @ 2012-12-10 19:07 UTC (permalink / raw)
To: Jeff Layton; +Cc: jiali@redhat.com, linux-nfs@vger.kernel.org
T24gTW9uLCAyMDEyLTEyLTEwIGF0IDEzOjI4IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxODowMDowNiArMDAwMA0KPiAiTXlrbGVidXN0LCBUcm9uZCIg
PFRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tPiB3cm90ZToNCj4gDQo+ID4gT24gTW9uLCAyMDEy
LTEyLTEwIGF0IDEyOjIwIC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4gPiA+IE9uIE1vbiwg
MTAgRGVjIDIwMTIgMTc6MDI6MDAgKzAwMDANCj4gPiA+ICJNeWtsZWJ1c3QsIFRyb25kIiA8VHJv
bmQuTXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiA+ID4gDQo+ID4gPiA+IE9uIE1vbiwg
MjAxMi0xMi0xMCBhdCAxMTo1MyAtMDUwMCwgSmVmZiBMYXl0b24gd3JvdGU6DQo+ID4gPiA+ID4g
T24gTW9uLCAxMCBEZWMgMjAxMiAxNjoyODo0NyArMDAwMA0KPiA+ID4gPiA+ICJNeWtsZWJ1c3Qs
IFRyb25kIiA8VHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiA+ID4gPiA+IA0K
PiA+ID4gPiA+ID4gT24gTW9uLCAyMDEyLTEyLTEwIGF0IDExOjA1IC0wNTAwLCBKZWZmIExheXRv
biB3cm90ZToNCj4gPiA+ID4gPiA+ID4gT24gTW9uLCAxMCBEZWMgMjAxMiAxNDo0MjoyMyArMDAw
MA0KPiA+ID4gPiA+ID4gPiAiTXlrbGVidXN0LCBUcm9uZCIgPFRyb25kLk15a2xlYnVzdEBuZXRh
cHAuY29tPiB3cm90ZToNCj4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+ID4gT24gTW9uLCAy
MDEyLTEyLTEwIGF0IDA5OjI1IC0wNTAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4gPiA+ID4gPiA+
ID4gPiA+IEppYW4gcmVwb3J0ZWQgdGhhdCB0aGUgZm9sbG93aW5nIHNlcXVlbmNlIHdvdWxkIGxl
YXZlICJ0ZXN0ZmlsZSIgd2l0aA0KPiA+ID4gPiA+ID4gPiA+ID4gY29ycnVwdCBkYXRhOg0KPiA+
ID4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+ID4gPiAgICAgIyBtb3VudCBsb2NhbGhvc3Q6
L2V4cG9ydCAvbW50L25mcy8gLW8gdmVycz0zDQo+ID4gPiA+ID4gPiA+ID4gPiAgICAgIyBlY2hv
IGFiYyA+IC9tbnQvbmZzL3Rlc3RmaWxlOyBlY2hvIGRlZiA+PiAvZXhwb3J0L3Rlc3RmaWxlOyBl
Y2hvIGdoaSA+PiAvbW50L25mcy90ZXN0ZmlsZQ0KPiA+ID4gPiA+ID4gPiA+ID4gICAgICMgY2F0
IC12IC9leHBvcnQvdGVzdGZpbGUNCj4gPiA+ID4gPiA+ID4gPiA+ICAgICBhYmMNCj4gPiA+ID4g
PiA+ID4gPiA+ICAgICBeQF5AXkBeQGdoaQ0KPiA+ID4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4g
PiA+ID4gPiBXaGlsZSB0aGVyZSdzIG5vIGxvY2tpbmcgaW52b2x2ZWQgaGVyZSwgdGhlIG9wZXJh
dGlvbnMgYXJlIHNlcmlhbGl6ZWQsDQo+ID4gPiA+ID4gPiA+ID4gPiBzbyBDVE8gc2hvdWxkIHBy
ZXZlbnQgY29ycnVwdGlvbi4NCj4gPiA+ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiA+ID4g
VGhlIGZpcnN0IHdyaXRlIHRvIHRoZSBmaWxlIGlzIGZpbmUgYW5kIHdyaXRlcyA0IGJ5dGVzLiBU
aGUgZmlsZSBpcyB0aGVuDQo+ID4gPiA+ID4gPiA+ID4gPiBleHRlbmRlZCBvbiB0aGUgc2VydmVy
LiBXaGVuIGl0J3MgcmVvcGVuZWQgYSBHRVRBVFRSIGlzIGlzc3VlZCBhbmQgdGhlDQo+ID4gPiA+
ID4gPiA+ID4gPiBzaXplIGNoYW5nZSBpcyBub3RpY2VkLiBUaGlzIGNhdXNlcyBORlNfSU5PX0lO
VkFMSURfREFUQSB0byBiZSBzZXQgb24NCj4gPiA+ID4gPiA+ID4gPiA+IHRoZSBmaWxlLiBCZWNh
dXNlIHRoZSBmaWxlIGlzIG9wZW5lZCBmb3Igd3JpdGUgb25seSwNCj4gPiA+ID4gPiA+ID4gPiA+
IG5mc193YW50X3JlYWRfbW9kaWZ5X3dyaXRlKCkgcmV0dXJucyAwIHRvIG5mc193cml0ZV9iZWdp
bigpLg0KPiA+ID4gPiA+ID4gPiA+ID4gbmZzX3VwZGF0ZXBhZ2UgdGhlbiBjYWxscyBuZnNfd3Jp
dGVfcGFnZXVwdG9kYXRlKCkgdG8gc2VlIGlmIGl0IHNob3VsZA0KPiA+ID4gPiA+ID4gPiA+ID4g
ZXh0ZW5kIHRoZSBuZnNfcGFnZSB0byBjb3ZlciB0aGUgd2hvbGUgcGFnZS4gTkZTX0lOT19JTlZB
TElEX0RBVEEgaXMNCj4gPiA+ID4gPiA+ID4gPiA+IHN0aWxsIHNldCBvbiB0aGUgZmlsZSBhdCB0
aGF0IHBvaW50LCBidXQgdGhhdCBmbGFnIGlzIGlnbm9yZWQgYW5kDQo+ID4gPiA+ID4gPiA+ID4g
PiBuZnNfcGFnZXVwdG9kYXRlIGVycm9uZW91c2x5IGV4dGVuZHMgdGhlIHdyaXRlIHRvIGNvdmVy
IHRoZSB3aG9sZSBwYWdlLA0KPiA+ID4gPiA+ID4gPiA+ID4gd2l0aCB0aGUgd3JpdGUgZG9uZSBv
biB0aGUgc2VydmVyIHNpZGUgZmlsbGVkIGluIHdpdGggemVyb2VzLg0KPiA+ID4gPiA+ID4gPiA+
ID4gDQo+ID4gPiA+ID4gPiA+ID4gPiBUaGlzIHBhdGNoIGp1c3QgaGFzIHRoYXQgZnVuY3Rpb24g
Y2hlY2sgZm9yIE5GU19JTk9fSU5WQUxJRF9EQVRBIGluDQo+ID4gPiA+ID4gPiA+ID4gPiBhZGRp
dGlvbiB0byBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRS4gVGhpcyBmaXhlcyB0aGUgYnVnLCBidXQg
bG9va2luZw0KPiA+ID4gPiA+ID4gPiA+ID4gb3ZlciB0aGUgY29kZSwgSSB3b25kZXIgaWYgd2Ug
bWlnaHQgaGF2ZSBhIHNpbWlsYXIgYnVnIGluDQo+ID4gPiA+ID4gPiA+ID4gPiBuZnNfcmV2YWxp
ZGF0ZV9zaXplKCkuIFRoZSBkaWZmZXJlbmNlIGJldHdlZW4gdGhvc2UgdHdvIGZsYWdzIGlzIHZl
cnkNCj4gPiA+ID4gPiA+ID4gPiA+IHN1YnRsZSwgc28gaXQgc2VlbXMgbGlrZSB3ZSBvdWdodCB0
byBiZSBjaGVja2luZyBmb3INCj4gPiA+ID4gPiA+ID4gPiA+IE5GU19JTk9fSU5WQUxJRF9EQVRB
IGluIG1vc3Qgb2YgdGhlIHBsYWNlcyB0aGF0IHdlIGxvb2sgZm9yDQo+ID4gPiA+ID4gPiA+ID4g
PiBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRS4NCj4gPiA+ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+
ID4gPiA+ID4gSSBiZWxpZXZlIHRoaXMgaXMgcmVncmVzc2lvbiBpbnRyb2R1Y2VkIGJ5IGNvbW1p
dCA4ZDE5N2E1NjguIFRoZSBjb2RlDQo+ID4gPiA+ID4gPiA+ID4gPiBkaWQgY2hlY2sgZm9yIE5G
U19JTk9fSU5WQUxJRF9EQVRBIHByaW9yIHRvIHRoYXQgcGF0Y2guDQo+ID4gPiA+ID4gPiA+ID4g
PiANCj4gPiA+ID4gPiA+ID4gPiA+IE9yaWdpbmFsIGJ1ZyByZXBvcnQgaXMgaGVyZToNCj4gPiA+
ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiA+ID4gICAgIGh0dHBzOi8vYnVnemlsbGEucmVk
aGF0LmNvbS9zaG93X2J1Zy5jZ2k/aWQ9ODg1NzQzDQo+ID4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+
ID4gPiA+ID4gSGkgSmVmZiwNCj4gPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gPiBUaGUg
cG9pbnQgb2YgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUgaXMgdG8gYmUgYWJsZSB0byBkaXN0aW5n
dWlzaA0KPiA+ID4gPiA+ID4gPiA+IGJldHdlZW4gc2l0dWF0aW9ucyB3aGVyZSB0aGUgZmlsZSBz
aXplIG9yIGNoYW5nZSBhdHRyaWJ1dGUgaXMgaW4gZG91YnQNCj4gPiA+ID4gPiA+ID4gPiAoYW5k
IHNvIHRoZSBwYWdlIGNhY2hlIHZhbGlkaXR5IGlzIGluIGRvdWJ0KSwgYW5kIHNpdHVhdGlvbnMg
d2hlcmUgb3RoZXINCj4gPiA+ID4gPiA+ID4gPiBhdHRyaWJ1dGVzIGFyZSBpbiBkb3VidCAoc3Vj
aCBhcyBBQ0NFU1MgY2FjaGVzLCBmaWxlIG93bmVyLCBubGlua3MsLi4uKS4NCj4gPiA+ID4gPiA+
ID4gPiBUaGlzIGlzIHdoeSB3ZSdyZSBvbmx5IGNoZWNraW5nIGZvciBORlNfSU5PX1JFVkFMX1BB
R0VDQUNIRSBhbmQgbm90DQo+ID4gPiA+ID4gPiA+ID4gTkZTX0lOT19JTlZBTElEX0RBVEEgaW4g
dGhlIGZ1bmN0aW9ucyB5b3UgbWVudGlvbiBhYm92ZS4NCj4gPiA+ID4gPiA+ID4gPiANCj4gPiA+
ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+IE9rLCBub3cgSSdtIHJlYWxseSBjb25mdXNlZC4uLiBJ
IHRob3VnaHQgdGhhdDoNCj4gPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiA+IE5GU19JTk9fSU5W
QUxJRF9EQVRBID09IHBhZ2VjYWNoZSBkYXRhIHdhcyBrbm93biB0byBiZSB3cm9uZw0KPiA+ID4g
PiA+ID4gPiBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRSA9PSBwYWdlY2FjaGUgZGF0YSBpcyBxdWVz
dGlvbmFibGUNCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gVGhlcmUgaXMgbm8gImtub3duIHRv
IGJlIHdyb25nIiBzdGF0ZS4gQm90aCBtZWFuICJuZWVkcyByZXZhbGlkYXRpb24iLg0KPiA+ID4g
PiA+ID4gDQo+ID4gPiA+ID4gPiA+IC4uLmJ1dCBpZiBJIHVuZGVyc3RhbmQgd2hhdCB5b3UncmUg
c2F5aW5nIGFib3ZlLCB0aGUgImRhdGEiIHRoYXQNCj4gPiA+ID4gPiA+ID4gTkZTX0lOT19JTlZB
TElEX0RBVEEgcmVmZXJzIHRvIGlzIGluIHBlcmlwaGVyYWwgY2FjaGVzLCBsaWtlIHRoZSBBQ0NF
U1MNCj4gPiA+ID4gPiA+ID4gY2FjaGU/IEkgZ3Vlc3MgdGhlbiBJIGRvbid0IHF1aXRlIHVuZGVy
c3RhbmQgd2hhdCBORlNfSU5PX0lOVkFMSURfQVRUUg0KPiA+ID4gPiA+ID4gPiBhbmQgTkZTX0lO
T19JTlZBTElEX0FDQ0VTUyBhcmUgZm9yLiBBcmVuJ3QgdGhleSBzdXBwb3NlZCB0byBpbmRpY2F0
ZSB0aGF0DQo+ID4gPiA+ID4gPiA+IHRoZSBhdHRycyBhbmQgYWNjZXNzIGNhY2hlcyBhcmUgaW52
YWxpZD8NCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gTkZTX0lOT19JTlZBTElEX0FUVFIgYmFz
aWNhbGx5IG1lYW5zIHRoYXQgc29tZSBhc3BlY3Qgb2YgdGhlIGZpbGUNCj4gPiA+ID4gPiA+IG1l
dGFkYXRhIG5lZWRzIHJldmFsaWRhdGlvbiwgYW5kIHNvIGEgY2FsbCB0byBuZnNfcmV2YWxpZGF0
ZV9pbm9kZSgpDQo+ID4gPiA+ID4gPiB3aWxsIGFsd2F5cyByZXN1bHQgaW4gYSBHRVRBVFRSIHJw
YyBjYWxsLg0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiBORlNfSU5PX0lOVkFMSURfQUNDRVNT
IG1lYW5zIHRoYXQgdGhlIGZpbGUgYWNjZXNzIGNhY2hlIG5lZWRzDQo+ID4gPiA+ID4gPiByZXZh
bGlkYXRpb24uIEEgY2FsbCB0byBuZnNfZG9fYWNjZXNzKCkgd2lsbCBhbHdheXMgcmVzdWx0IGlu
IGFuIEFDQ0VTUw0KPiA+ID4gPiA+ID4gcnBjIGNhbGwuDQo+ID4gPiA+ID4gPiANCj4gPiA+ID4g
PiA+IE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFIG1lYW5zIHRoYXQgdGhlIGZpbGUgcGFnZSBjYWNo
ZSBkYXRhIG5lZWRzDQo+ID4gPiA+ID4gPiByZXZhbGlkYXRpb24uIEEgY2FsbCB0byBuZnNfcmV2
YWxpZGF0ZV9maWxlX3NpemUoKSBvcg0KPiA+ID4gPiA+ID4gbmZzX3JldmFsaWRhdGVfbWFwcGlu
ZygpIHdpbGwgcmVzdWx0IGluIGEgcmV2YWxpZGF0aW9uIG9mIHRoZSBwYWdlDQo+ID4gPiA+ID4g
PiBjYWNoZS4gSW4gcHJhY3RpY2UgdGhhdCBtZWFucyBhIEdFVEFUVFIgcnBjIGNhbGwsIGJ1dCB0
aGF0IG1heSBjaGFuZ2UgaWYNCj4gPiA+ID4gPiA+IGZ1dHVyZSB2ZXJzaW9ucyBvZiBORlMgYWxs
b3cgZm9yIGRpZmZlcmVudCBtZXRob2RzIG9mIHJldmFsaWRhdGluZyB0aGUNCj4gPiA+ID4gPiA+
IHBhZ2UgY2FjaGU7IGNvbnNpZGVyLCBmb3IgaW5zdGFuY2UsIHRoZSBieXRlLXJhbmdlIGRlbGVn
YXRpb24gUkZDIHRoYXQNCj4gPiA+ID4gPiA+IEJydWNlIGFuZCBJIHB1Ymxpc2hlZCBhIGZldyB5
ZWFycyBiYWNrLg0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gT2ssIHRoYW5r
cy4gQW5vdGhlciBxdWVzdGlvbiB0aG91Z2ggLS0gd2hhdCBkb2VzIE5GU19JTk9fSU5WQUxJRF9E
QVRBDQo+ID4gPiA+ID4gbWVhbiwgYW5kIHdoYXQgZGlzdGluZ3Vpc2hlcyBpdCBmcm9tIE5GU19J
Tk9fUkVWQUxfUEFHRUNBQ0hFPw0KPiA+ID4gPiANCj4gPiA+ID4gU2VlIGFib3ZlLiBUaGV5IGFm
ZmVjdCBfZGlmZmVyZW50XyByZXZhbGlkYXRpb24gc2l0dWF0aW9ucyAocmVwcmVzZW50ZWQNCj4g
PiA+ID4gYnkgZGlmZmVyZW50IHJldmFsaWRhdGlvbiBmdW5jdGlvbnMpLg0KPiA+ID4gPiANCj4g
PiA+ID4gPiA+ID4gPiBTbyB0aGUgZml4IGhlcmUgc2hvdWxkIGJlIHRvIHNldCBORlNfSU5PX1JF
VkFMX1BBR0VDQUNIRSB3aGVuIHRoZSBjaGFuZ2UNCj4gPiA+ID4gPiA+ID4gPiBhdHRyaWJ1dGUg
YW5kL29yIHNpemUgY2hhbmdlIGlzIG5vdGljZWQuIFRoZSBvbmx5IGZ1bmN0aW9uIEkgY2FuIHNl
ZQ0KPiA+ID4gPiA+ID4gPiA+IHRoYXQgYXBwZWFycyB0byBnZXQgdGhpcyB3cm9uZyBpcyBuZnNf
d2NjX3VwZGF0ZV9pbm9kZSgpLiBEb2VzIGZpeGluZw0KPiA+ID4gPiA+ID4gPiA+IHRoYXQgbGF5
IHRoZSBidWcgdG8gcmVzdD8NCj4gPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gDQo+ID4g
PiA+ID4gPiA+IE5vLCB0aGF0IGRvZXNuJ3QgZml4IGl0LiBUaGVyZSdzIGFub3RoZXIgcGxhY2Ug
dGhhdCBtaXNzZXMgc2V0dGluZyB0aGF0DQo+ID4gPiA+ID4gPiA+IGZsYWcgdG9vIGluIG5mc191
cGRhdGVfaW5vZGUoKSB3aGVyZSB0aGUgc2l6ZSBjaGFuZ2VzLiBJIGFkZGVkIGl0IHRoZXJlDQo+
ID4gPiA+ID4gPiA+IHRvbyBhbmQgdGhhdCBhbHNvIGRpZG4ndCBmaXggaXQuIEhlcmUncyB3aHkg
KEkgdGhpbmspOg0KPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gV2hlbiB0aGUgc2l6ZSBj
aGFuZ2UgaXMgbm90aWNlZCB2aWEgdGhlIENUTyBHRVRBVFRSIGNhbGwsIHRoZW4gbXkNCj4gPiA+
ID4gPiA+ID4gcGF0Y2hlZCBrZXJuZWwgbm93IHNldHMgTkZTX0lOT19SRVZBTF9QQUdFQ0FDSEUg
aW4gbmZzX3VwZGF0ZV9pbm9kZS4NCj4gPiA+ID4gPiA+ID4gTm93IGNhY2hlX3ZhbGlkaXR5IGlz
IHNldCB0byAweDJhLiBUaGUga2VybmVsIHRoZW4gbWFrZXMgYW4gQUNDRVNTIGNhbGwNCj4gPiA+
ID4gPiA+ID4gdGhhdCBhbHNvIHVsdGltYXRlbHkgZW5kcyB1cCBpbiBuZnNfdXBkYXRlX2lub2Rl
Lg0KPiA+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+ID4gbmZzX3VwZGF0ZV9pbm9kZSBzYXZlcyBv
ZmYgdGhlIGNhY2hlX3ZhbGlkaXR5IGZsYWdzIGFuZCBjbGVhcnMgYWxsIG9mDQo+ID4gPiA+ID4g
PiA+IHRoZW0gZXhjZXB0IGZvciBORlNfSU5PX0lOVkFMSURfREFUQS4gQXMgYmVzdCBJIGNhbiB0
ZWxsLCB0aGUNCj4gPiA+ID4gPiA+ID4gc2F2ZWRfY2FjaGVfdmFsaWRpdHkgaXMgdGhlbiBlbmRp
bmcgdXAgZGlzY2FyZGVkIGFuZCBvbiBleGl0IHRoZQ0KPiA+ID4gPiA+ID4gPiBjYWNoZV92YWxp
ZGl0eSBpcyBlbmRpbmcgdXAgc2V0IHRvIGp1c3QgTkZTX0lOT19JTlZBTElEX0RBVEEuDQo+ID4g
PiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiBJdCBzZWVtcyBsaWtlIHdlIG91Z2h0IHRvIGJlIHJl
c3RvcmluZyB0aGUgc2F2ZV9jYWNoZV92YWxpZGl0eSBmbGFncw0KPiA+ID4gPiA+ID4gPiBiZWZv
cmUgZXhpdGluZyB0aGF0IGZ1bmN0aW9uLCBidXQgSSBjb25mZXNzIHRoYXQgSSBkb24ndCBxdWl0
ZSBncmFzcA0KPiA+ID4gPiA+ID4gPiB0aGUgbG9naWMgaW4gbmZzX3VwZGF0ZV9pbm9kZS4gVGhv
dWdodHM/DQo+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+IElmIHRoZSBBQ0NFU1MgY2FsbCBpc24n
dCByZXZhbGlkYXRpbmcgdGhlIGZpbGUgc2l6ZSwgdGhlbiBpdCBzaG91bGQgbm90DQo+ID4gPiA+
ID4gPiBiZSBjbGVhcmluZyBORlNfSU5PX1JFVkFMX1BBR0VDQUNIRTsgaXQgZG9lcyB0aGlzIGJ5
IHJlc3RvcmluZyB0aGF0IGJpdA0KPiA+ID4gPiA+ID4gZnJvbSB0aGUgInNhdmVfY2FjaGVfdmFs
aWRpdHkiIHZhcmlhYmxlIGFuZCBzdG9yaW5nIGl0IGluICJpbnZhbGlkIi4gQXMNCj4gPiA+ID4g
PiA+IGZhciBhcyBJIGNhbiB0ZWxsLCB0aGF0IGNvZGUgaXMgY29ycmVjdCBpbiB1cHN0cmVhbS4N
Cj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IEZXSVcsIEknbSB3b3JraW5nIG9u
IGEgMy43LXJjOCBrZXJuZWwgaGVyZS4uLg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IFRoZSBBQ0NF
U1MgY2FsbCBlbmRzIHVwIHdpdGggYW4gZmF0dHIgdGhhdCBoYXMgTkZTX0FUVFJfRkFUVFJfU0la
RSBzZXQsDQo+ID4gPiA+ID4gaXQganVzdCBkb2Vzbid0IGRldGVjdCBhbnkgY2hhbmdlIGluIHNp
emUgZHVlIHRvIHRoZSBmYWN0IHRoYXQgdGhlIENUTw0KPiA+ID4gPiA+IEdFVEFUVFIgY2FsbCBo
YXMgYWxyZWFkeSB1cGRhdGVkIGl0Lg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IC0tLS0tLS0tLS0t
LS0tLS0tLVtzbmlwXS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KPiA+ID4gPiA+ICAgICAgICAg
LyogQ2hlY2sgaWYgb3VyIGNhY2hlZCBmaWxlIHNpemUgaXMgc3RhbGUgKi8NCj4gPiA+ID4gPiAg
ICAgICAgIGlmIChmYXR0ci0+dmFsaWQgJiBORlNfQVRUUl9GQVRUUl9TSVpFKSB7DQo+ID4gPiA+
ID4gICAgICAgICAgICAgICAgIG5ld19pc2l6ZSA9IG5mc19zaXplX3RvX2xvZmZfdChmYXR0ci0+
c2l6ZSk7DQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgIGN1cl9pc2l6ZSA9IGlfc2l6ZV9yZWFk
KGlub2RlKTsNCj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgaWYgKG5ld19pc2l6ZSAhPSBjdXJf
aXNpemUpIHsNCj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAvKiBEbyB3ZSBwZXJo
YXBzIGhhdmUgYW55IG91dHN0YW5kaW5nIHdyaXRlcywgb3IgaGFzDQo+ID4gPiA+ID4gICAgICAg
ICAgICAgICAgICAgICAgICAgICogdGhlIGZpbGUgZ3Jvd24gYmV5b25kIG91ciBsYXN0IHdyaXRl
PyAqLw0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIGlmICgobmZzaS0+bnBhZ2Vz
ID09IDAgJiYgIXRlc3RfYml0KE5GU19JTk9fTEFZT1VUQ09NTUlULCAmbmZzaS0+ZmxhZ3MpKSB8
fA0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgbmV3X2lzaXplID4gY3Vy
X2lzaXplKSB7DQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBpX3Np
emVfd3JpdGUoaW5vZGUsIG5ld19pc2l6ZSk7DQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICBpbnZhbGlkIHw9IE5GU19JTk9fSU5WQUxJRF9BVFRSfE5GU19JTk9fSU5W
QUxJRF9EQVRBOw0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIH0NCj4gPiA+ID4g
PiAgICAgICAgICAgICAgICAgICAgICAgICBkcHJpbnRrKCJORlM6IGlzaXplIGNoYW5nZSBvbiBz
ZXJ2ZXIgZm9yIGZpbGUgJXMvJWxkICINCj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgIiglTGQgdG8gJUxkKVxuIiwNCj4gPiA+ID4gPiAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaW5vZGUtPmlfc2ItPnNfaWQsDQo+ID4gPiA+
ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGlub2RlLT5pX2lubywN
Cj4gPiA+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKGxvbmcg
bG9uZyljdXJfaXNpemUsDQo+ID4gPiA+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgIChsb25nIGxvbmcpbmV3X2lzaXplKTsNCj4gPiA+ID4gPiAgICAgICAgICAgICAg
ICAgfQ0KPiA+ID4gPiA+ICAgICAgICAgfSBlbHNlDQo+ID4gPiA+ID4gICAgICAgICAgICAgICAg
IGludmFsaWQgfD0gc2F2ZV9jYWNoZV92YWxpZGl0eSAmIChORlNfSU5PX0lOVkFMSURfQVRUUg0K
PiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCBORlNfSU5PX1JFVkFM
X1BBR0VDQUNIRQ0KPiA+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCBO
RlNfSU5PX1JFVkFMX0ZPUkNFRCk7DQo+ID4gPiA+ID4gLS0tLS0tLS0tLS0tLS0tLS0tW3NuaXBd
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gU28gaGVyZSwg
d2Ugb25seSByZXN0b3JlIHNldCB0aGUgUkVWQUxfUEFHRUNBQ0hFIGZsYWcgaW4gaW52YWxpZCBp
ZiB0aGUNCj4gPiA+ID4gPiBzaXplIGNoYW5nZWQsIG9yIHRoZXJlIHdhcyBubyBzaXplIGF0dHJp
YnV0ZSBwcmVzZW50IGluIHRoZSByZXNwb25zZS4NCj4gPiA+ID4gPiBUaGUgc2l6ZSBoYXNuJ3Qg
Y2hhbmdlZCBzaW5jZSB0aGUgR0VUQVRUUiBjYWxsLCBzbyBSRVZBTF9QQUdFQ0FDSEUgaXMNCj4g
PiA+ID4gPiBub3QgcmVzdG9yZWQuDQo+ID4gPiA+IA0KPiA+ID4gPiBJIGRvbid0IHVuZGVyc3Rh
bmQuIFdoeSBkb2VzIGl0IG5lZWQgdG8gYmUgcmVzdG9yZWQ/IFRoZQ0KPiA+ID4gPiBORlNfQVRU
Ul9GQVRUUl9TSVpFIGlzIHNldCwgc28gdGhlIHNlcnZlciBoYXMgc2VudCB1cyBhbiB1cGRhdGVk
IHZhbHVlDQo+ID4gPiA+IGZvciB0aGUgc2l6ZS4gSXMgaXQgbHlpbmcgdG8gdXM/DQo+ID4gPiA+
IA0KPiA+ID4gDQo+ID4gPiBObywgdGhpcyBpcyBORlN2MyBhbmQgSUlVQywgTkZTX0FUVFJfRkFU
VFJfU0laRSBqdXN0IG1lYW5zIHRoYXQNCj4gPiA+IGZhdHRyLT5zaXplIGlzIHBvcHVsYXRlZCB3
aXRoIGEgdmFsdWUgc2VudCBieSB0aGUgc2VydmVyLiBJbiBmYWN0LA0KPiA+ID4gbG9va2luZyBh
dCBkZWNvZGVfZmF0dHIzKCk6DQo+ID4gPiANCj4gPiA+ICAgICAgICAgZmF0dHItPnZhbGlkIHw9
IE5GU19BVFRSX0ZBVFRSX1YzOw0KPiA+ID4gDQo+ID4gPiAuLi5hbmQgTkZTX0FUVFJfRkFUVFJf
VjMgY29udGFpbnMgTkZTX0FUVFJfRkFUVFJfU0laRS4NCj4gPiANCj4gPiBSaWdodC4gVGhlIE5G
U3YzIHNlcnZlciBpcyBhbHdheXMgc3VwcG9zZWQgdG8gc2VuZCB1cyB0aGUgY29ycmVjdCB2YWx1
ZQ0KPiA+IGZvciB0aGUgc2l6ZS4NCj4gPiANCj4gPiA+IE1heWJlIHRoaXMgZmxhZyBkb2Vzbid0
IG5lZWQgdG8gYmUgcmVzdG9yZWQuIEkgcmVhbGx5IGRvbid0IHVuZGVyc3RhbmQNCj4gPiA+IHRo
ZSBjdXJyZW50IGxvZ2ljIGluIG5mc191cGRhdGVfaW5vZGUgYXQgYWxsIHNvIEknbGwgaGF2ZSB0
byBkZWZlciB0bw0KPiA+ID4geW91ciBqdWRnZW1lbnQgaGVyZS4NCj4gPiA+IA0KPiA+ID4gQWxs
IEkga25vdyBpcyB0aGF0IHdoZW4gdGhlIENUTyBHRVRBVFRSIGNhbGwgaXMgZG9uZSwgdGhlIGtl
cm5lbCBzZWVzDQo+ID4gPiB0aGF0IHRoZSBzaXplIGhhcyBjaGFuZ2VkLiBQcmVzdW1hYmx5LCBp
dCBzZXRzIE5GU19JTk9fUkVWQUxfUEFHRUNBQ0hFDQo+ID4gPiB0byByZWZsZWN0IHRoYXQgZmFj
dC4NCj4gPiANCj4gPiBJdCBzaG91bGQgYWxzbyBzZXQgTkZTX0lOT19JTlZBTElEX0RBVEEsIHdo
aWNoIGlzIF9ub3RfIGNsZWFyZWQgYnkNCj4gPiBuZnNfdXBkYXRlX2lub2RlKCkNCj4gPiANCj4g
PiA+ICBUaGVuLCBvbiB0aGUgZm9sbG93aW5nIEFDQ0VTUyBjYWxsIHRoYXQgZ2V0cyBiYWNrDQo+
ID4gPiBhdHRyaWJ1dGVzLCBpdCBjbGVhcnMgdGhhdCBmbGFnIHdpdGhvdXQgaW52YWxpZGF0aW5n
IHRoZSBjYWNoZS4NCj4gPiANCj4gPiBBaC4uLiBJIHNlZSB0aGUgYnVnOg0KPiA+IA0KPiA+IG5m
c193cml0ZV9wYWdldXB0b2RhdGUoKSBuZWVkcyB0byBjaGVjayBmb3IgTkZTX0lOT19SRVZBTF9Q
QUdFQ0FDSEV8DQo+ID4gTkZTX0lOT19JTlZBTElEX0RBVEEuLi4NCj4gPiANCj4gDQo+IE9uZSBt
b3JlIHF1ZXN0aW9uIHRob3VnaC4uLg0KPiANCj4gSSBkb24ndCBoYXZlIGEgcmVwcm9kdWNlciBm
b3IgaXQsIGJ1dCBpdCBsb29rcyBsaWtlDQo+IG5mc19yZXZhbGlkYXRlX2ZpbGVfc2l6ZSgpIG1p
Z2h0IGhhdmUgYSBzaW1pbGFyIGJ1Zy4gU2hvdWxkIHdlIGFsc28gYmUNCj4gY2hlY2tpbmcgZm9y
IE5GU19JTk9fSU5WQUxJRF9EQVRBIHRoZXJlIHRvbz8NCg0KTm8uIG5mc19yZXZhbGlkYXRlX2Zp
bGVfc2l6ZSgpIGlzIGxpdGVyYWxseSBvbmx5IGZvciB0aGUgZmlsZSBzaXplLiBJdA0KZXhpc3Rz
IG9ubHkgZm9yIHRoZSBiZW5lZml0IG9mIGxzZWVrKFNFRUtfRU5EKSBhbmQgZm9yIGFwcGVuZCB3
cml0ZXMuDQoNCkluIHNpdHVhdGlvbnMgd2hlcmUgd2UgbmVlZCBkYXRhIGNhY2hlIGNvbnNpc3Rl
bmN5LCB3ZSBzaG91bGQgaW5zdGVhZCBiZQ0KY2FsbGluZyBuZnNfcmV2YWxpZGF0ZV9tYXBwaW5n
KCkuDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIN
Cg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-12-10 19:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-10 14:25 [PATCH] nfs: don't extend writes to cover entire page if pagecache is invalid Jeff Layton
2012-12-10 14:42 ` Myklebust, Trond
2012-12-10 16:05 ` Jeff Layton
2012-12-10 16:28 ` Myklebust, Trond
2012-12-10 16:53 ` Jeff Layton
2012-12-10 17:02 ` Myklebust, Trond
2012-12-10 17:20 ` Jeff Layton
2012-12-10 18:00 ` Myklebust, Trond
2012-12-10 18:08 ` Jeff Layton
2012-12-10 18:55 ` Myklebust, Trond
2012-12-10 18:28 ` Jeff Layton
2012-12-10 19:07 ` Myklebust, Trond
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox