* [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: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: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: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