Re: [PATCH 4/6] xstat: NFS: Return extended attributes

T24gVGh1LCAyMDEyLTA0LTE5IGF0IDE1OjA2ICswMTAwLCBEYXZpZCBIb3dlbGxzIHdyb3RlOg0KPiBSZXR1cm4gZXh0ZW5kZWQgYXR0cmlidXRlcyBmcm9tIHRoZSBORlMgZmlsZXN5c3RlbS4gIFRoaXMgaW5jbHVkZXMgdGhlDQo+IGZvbGxvd2luZzoNCj4gDQo+ICAoMSkgVGhlIGNoYW5nZSBhdHRyaWJ1dGUgYXMgc3RfZGF0YV92ZXJzaW9uIGlmIE5GU3Y0Lg0KPiANCj4gICgyKSBYU1RBVF9JTkZPX0FVVE9NT1VOVCBhbmQgWFNUQVRfSU5GT19GQUJSSUNBVEVEIGFyZSBzZXQgb24gcmVmZXJyYWwgb3INCj4gICAgICBzdWJtb3VudCBkaXJlY3RvcmllcyB0aGF0IGFyZSBhdXRvbW91bnRlZCB1cG9uLiAgTkZTIHNob3dzIG9uZSBkaXJlY3RvcnkNCj4gICAgICB3aXRoIGEgZGlmZmVyZW50IEZTSUQsIGJ1dCB0aGUgbG9jYWwgZmlsZXN5c3RlbSBoYXMgdHdvOiB0aGUgbW91bnRwb2ludA0KPiAgICAgIGRpcmVjdG9yeSBhbmQgdGhlIHJvb3Qgb2YgdGhlIGZpbGVzeXN0ZW0gbW91bnRlZCB1cG9uIGl0Lg0KPiANCj4gICgzKSBYU1RBVF9JTkZPX1JFTU9URSBpcyBzZXQgb24gZmlsZXMgYWNxdWlyZWQgb3ZlciBORlMuDQo+IA0KPiBGdXJ0aGVybW9yZSwgd2hhdCBuZnNfZ2V0YXR0cigpIGRvZXMgY2FuIGJlIGNvbnRyb2xsZWQgYXMgZm9sbG93czoNCj4gDQo+ICAoMSkgSWYgQVRfRk9SQ0VfQVRUUl9TWU5DIGlzIGluZGljYXRlZCwgb3IgbXRpbWUsIGN0aW1lIG9yIGRhdGFfdmVyc2lvbiAoTkZTdjQNCj4gICAgICBvbmx5KSBhcmUgcmVxdWVzdGVkIHRoZW4gdGhlIG91dHN0YW5kaW5nIHdyaXRlcyB3aWxsIGJlIHdyaXR0ZW4gdG8gdGhlDQo+ICAgICAgc2VydmVyIGZpcnN0Lg0KPiANCj4gICgyKSBUaGUgaW5vZGUncyBhdHRyaWJ1dGVzIG1heSBiZSBzeW5jaHJvbmlzZWQgd2l0aCB0aGUgc2VydmVyOg0KPiANCj4gICAgICAoYSkgSWYgQVRfRk9SQ0VfQVRUUl9TWU5DIGlzIGluZGljYXRlZCBvciBpZiBhdGltZSBpcyByZXF1ZXN0ZWQgKGFuZCBhdGltZQ0KPiAgICAgIAkgdXBkYXRpbmcgaXMgbm90IHN1cHByZXNzZWQgYnkgYSBtb3VudCBmbGFnKSB0aGVuIHRoZSBhdHRyaWJ1dGVzIHdpbGwNCj4gICAgICAJIGJlIHJlcmVhZCB1bmNvbmRpdGlvbmFsbHkuDQo+IA0KPiAgICAgIChiKSBJZiB0aGUgZGF0YSB2ZXJzaW9uIG9yIGFueSBvZiBiYXNpYyBzdGF0cyBhcmUgcmVxdWVzdGVkIHRoZW4gdGhlDQo+ICAgICAgCSBhdHRyaWJ1dGVzIHdpbGwgYmUgcmVyZWFkIGlmIHRoZSBjYWNoZWQgYXR0cmlidXRlcyBoYXZlIGV4cGlyZWQuDQo+IA0KPiAgICAgIChjKSBPdGhlcndpc2UgdGhlIGNhY2hlZCBhdHRyaWJ1dGVzIHdpbGwgYmUgdXNlZCAtIGV2ZW4gaWYgZXhwaXJlZCAtDQo+ICAgICAgCSB3aXRob3V0IHJlZmVyZW5jZSB0byB0aGUgc2VydmVyLg0KDQpIbW0uLi4gQXMgZmFyIGFzIEkgY2FuIHNlZSB5b3UgYXJlIHN0aWxsIGRvaW5nIGFuIG5mc19yZXZhbGlkYXRlX2lub2RlKCkNCmluIHRoZSBub24tZm9yY2VkIGNhc2UuIFRoYXQgd2lsbCBjYXVzZSBleHBpcmVkIGF0dHJpYnV0ZXMgdG8gYmUNCnJldHJpZXZlZCBmcm9tIHRoZSBzZXJ2ZXIuDQoNCj4gU2lnbmVkLW9mZi1ieTogRGF2aWQgSG93ZWxscyA8ZGhvd2VsbHNAcmVkaGF0LmNvbT4NCj4gLS0tDQo+IA0KPiAgZnMvbmZzL2lub2RlLmMgfCAgIDQ5ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKystLS0tLS0tLS0tLS0NCj4gIGZzL25mcy9zdXBlci5jIHwgICAgMSArDQo+ICAyIGZpbGVzIGNoYW5nZWQsIDM4IGluc2VydGlvbnMoKyksIDEyIGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9pbm9kZS5jIGIvZnMvbmZzL2lub2RlLmMNCj4gaW5kZXggZThiYmZhNS4uNDYwZmNmMyAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL2lub2RlLmMNCj4gKysrIGIvZnMvbmZzL2lub2RlLmMNCj4gQEAgLTUwOSwxMSArNTA5LDE4IEBAIHZvaWQgbmZzX3NldGF0dHJfdXBkYXRlX2lub2RlKHN0cnVjdCBpbm9kZSAqaW5vZGUsIHN0cnVjdCBpYXR0ciAqYXR0cikNCj4gIGludCBuZnNfZ2V0YXR0cihzdHJ1Y3QgdmZzbW91bnQgKm1udCwgc3RydWN0IGRlbnRyeSAqZGVudHJ5LCBzdHJ1Y3Qga3N0YXQgKnN0YXQpDQo+ICB7DQo+ICAJc3RydWN0IGlub2RlICppbm9kZSA9IGRlbnRyeS0+ZF9pbm9kZTsNCj4gKwl1bnNpZ25lZCBmb3JjZSA9IHN0YXQtPnF1ZXJ5X2ZsYWdzICYgQVRfRk9SQ0VfQVRUUl9TWU5DOw0KPiAgCWludCBuZWVkX2F0aW1lID0gTkZTX0koaW5vZGUpLT5jYWNoZV92YWxpZGl0eSAmIE5GU19JTk9fSU5WQUxJRF9BVElNRTsNCj4gIAlpbnQgZXJyOw0KPiAgDQo+IC0JLyogRmx1c2ggb3V0IHdyaXRlcyB0byB0aGUgc2VydmVyIGluIG9yZGVyIHRvIHVwZGF0ZSBjL210aW1lLiAgKi8NCj4gLQlpZiAoU19JU1JFRyhpbm9kZS0+aV9tb2RlKSkgew0KPiArCWlmIChORlNfU0VSVkVSKGlub2RlKS0+bmZzX2NsaWVudC0+cnBjX29wcy0+dmVyc2lvbiA8IDQpDQo+ICsJCXN0YXQtPnJlcXVlc3RfbWFzayAmPSB+WFNUQVRfVkVSU0lPTjsNCj4gKw0KPiArCS8qIEZsdXNoIG91dCB3cml0ZXMgdG8gdGhlIHNlcnZlciBpbiBvcmRlciB0byB1cGRhdGUgYy9tdGltZQ0KPiArCSAqIG9yIGRhdGEgdmVyc2lvbiBpZiB0aGUgdXNlciB3YW50cyB0aGVtICovDQo+ICsJaWYgKChmb3JjZSB8fCAoc3RhdC0+cmVxdWVzdF9tYXNrICYNCj4gKwkJICAgICAgIChYU1RBVF9NVElNRSB8IFhTVEFUX0NUSU1FIHwgWFNUQVRfVkVSU0lPTikpKSAmJg0KPiArCSAgICBTX0lTUkVHKGlub2RlLT5pX21vZGUpKSB7DQo+ICAJCWVyciA9IGZpbGVtYXBfd3JpdGVfYW5kX3dhaXQoaW5vZGUtPmlfbWFwcGluZyk7DQoNCldlIGNhbiBnZXQgcmlkIG9mIHRoZSBmaWxlbWFwX3dyaXRlX2FuZF93YWl0KCkgaWYgdGhlIGNhbGxlciBhbGxvd3MgdXMNCnRvIGFwcHJveGltYXRlIG0vY3RpbWUgdmFsdWVzLiBUaGF0IHdvdWxkIGdpdmUgYSBtYWpvciBzcGVlZC11cCBmb3IgbW9zdA0Kc3RhdCgpIHdvcmtsb2Fkcy4NCg0KPiAgCQlpZiAoZXJyKQ0KPiAgCQkJZ290byBvdXQ7DQo+IEBAIC01MjgsMTggKzUzNSwzNiBAQCBpbnQgbmZzX2dldGF0dHIoc3RydWN0IHZmc21vdW50ICptbnQsIHN0cnVjdCBkZW50cnkgKmRlbnRyeSwgc3RydWN0IGtzdGF0ICpzdGF0KQ0KPiAgCSAqICAtIE5GUyBuZXZlciBzZXRzIE1TX05PQVRJTUUgb3IgTVNfTk9ESVJBVElNRSBzbyB0aGVyZSBpcw0KPiAgCSAqICAgIG5vIHBvaW50IGluIGNoZWNraW5nIHRob3NlLg0KPiAgCSAqLw0KPiAtIAlpZiAoKG1udC0+bW50X2ZsYWdzICYgTU5UX05PQVRJTUUpIHx8DQo+IC0gCSAgICAoKG1udC0+bW50X2ZsYWdzICYgTU5UX05PRElSQVRJTUUpICYmIFNfSVNESVIoaW5vZGUtPmlfbW9kZSkpKQ0KPiArCWlmIChtbnQtPm1udF9mbGFncyAmIE1OVF9OT0FUSU1FIHx8DQo+ICsJICAgIChtbnQtPm1udF9mbGFncyAmIE1OVF9OT0RJUkFUSU1FICYmIFNfSVNESVIoaW5vZGUtPmlfbW9kZSkpKSB7DQo+ICsJCXN0YXQtPmlvY19mbGFncyB8PSBGU19OT0FUSU1FX0ZMOw0KPiArCQluZWVkX2F0aW1lID0gMDsNCj4gKwl9IGVsc2UgaWYgKCEoc3RhdC0+cmVxdWVzdF9tYXNrICYgWFNUQVRfQVRJTUUpKSB7DQo+ICAJCW5lZWRfYXRpbWUgPSAwOw0KPiArCX0NCj4gIA0KPiAtCWlmIChuZWVkX2F0aW1lKQ0KPiAtCQllcnIgPSBfX25mc19yZXZhbGlkYXRlX2lub2RlKE5GU19TRVJWRVIoaW5vZGUpLCBpbm9kZSk7DQo+IC0JZWxzZQ0KPiAtCQllcnIgPSBuZnNfcmV2YWxpZGF0ZV9pbm9kZShORlNfU0VSVkVSKGlub2RlKSwgaW5vZGUpOw0KPiAtCWlmICghZXJyKSB7DQo+IC0JCWdlbmVyaWNfZmlsbGF0dHIoaW5vZGUsIHN0YXQpOw0KPiAtCQlzdGF0LT5pbm8gPSBuZnNfY29tcGF0X3VzZXJfaW5vNjQoTkZTX0ZJTEVJRChpbm9kZSkpOw0KPiArCWlmIChmb3JjZSB8fCBzdGF0LT5yZXF1ZXN0X21hc2sgJiAoWFNUQVRfQkFTSUNfU1RBVFMgfCBYU1RBVF9WRVJTSU9OKSkgew0KPiArCQlpZiAoZm9yY2UgfHwgbmVlZF9hdGltZSkNCj4gKwkJCWVyciA9IF9fbmZzX3JldmFsaWRhdGVfaW5vZGUoTkZTX1NFUlZFUihpbm9kZSksIGlub2RlKTsNCj4gKwkJZWxzZQ0KPiArCQkJZXJyID0gbmZzX3JldmFsaWRhdGVfaW5vZGUoTkZTX1NFUlZFUihpbm9kZSksIGlub2RlKTsNCj4gKwkJaWYgKGVycikNCj4gKwkJCWdvdG8gb3V0Ow0KPiAgCX0NCj4gKw0KPiArCWdlbmVyaWNfZmlsbGF0dHIoaW5vZGUsIHN0YXQpOw0KPiArCXN0YXQtPmlubyA9IG5mc19jb21wYXRfdXNlcl9pbm82NChORlNfRklMRUlEKGlub2RlKSk7DQo+ICsNCj4gKwlpZiAoc3RhdC0+cmVxdWVzdF9tYXNrICYgWFNUQVRfVkVSU0lPTikgew0KPiArCQlzdGF0LT52ZXJzaW9uID0gaW5vZGUtPmlfdmVyc2lvbjsNCj4gKwkJc3RhdC0+cmVzdWx0X21hc2sgfD0gWFNUQVRfVkVSU0lPTjsNCj4gKwl9DQo+ICsNCj4gKwlpZiAoSVNfQVVUT01PVU5UKGlub2RlKSkNCj4gKwkJc3RhdC0+aW5mb3JtYXRpb24gfD0gWFNUQVRfSU5GT19GQUJSSUNBVEVEOw0KPiArDQo+ICsJc3RhdC0+aW5mb3JtYXRpb24gfD0gWFNUQVRfSU5GT19SRU1PVEU7DQo+ICsNCj4gIG91dDoNCj4gIAlyZXR1cm4gZXJyOw0KPiAgfQ0KPiBAQCAtODUyLDcgKzg3Nyw3IEBAIGludCBuZnNfcmV2YWxpZGF0ZV9pbm9kZShzdHJ1Y3QgbmZzX3NlcnZlciAqc2VydmVyLCBzdHJ1Y3QgaW5vZGUgKmlub2RlKQ0KPiAgc3RhdGljIGludCBuZnNfaW52YWxpZGF0ZV9tYXBwaW5nKHN0cnVjdCBpbm9kZSAqaW5vZGUsIHN0cnVjdCBhZGRyZXNzX3NwYWNlICptYXBwaW5nKQ0KPiAgew0KPiAgCXN0cnVjdCBuZnNfaW5vZGUgKm5mc2kgPSBORlNfSShpbm9kZSk7DQo+IC0JDQo+ICsNCj4gIAlpZiAobWFwcGluZy0+bnJwYWdlcyAhPSAwKSB7DQo+ICAJCWludCByZXQgPSBpbnZhbGlkYXRlX2lub2RlX3BhZ2VzMihtYXBwaW5nKTsNCj4gIAkJaWYgKHJldCA8IDApDQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvc3VwZXIuYyBiL2ZzL25mcy9zdXBlci5jDQo+IGluZGV4IDM3NDEyZjcuLmZhYTY1MmMgMTAwNjQ0DQo+IC0tLSBhL2ZzL25mcy9zdXBlci5jDQo+ICsrKyBiL2ZzL25mcy9zdXBlci5jDQo+IEBAIC0yMjIyLDYgKzIyMjIsNyBAQCBzdGF0aWMgaW50IG5mc19zZXRfc3VwZXIoc3RydWN0IHN1cGVyX2Jsb2NrICpzLCB2b2lkICpkYXRhKQ0KPiAgCXJldCA9IHNldF9hbm9uX3N1cGVyKHMsIHNlcnZlcik7DQo+ICAJaWYgKHJldCA9PSAwKQ0KPiAgCQlzZXJ2ZXItPnNfZGV2ID0gcy0+c19kZXY7DQo+ICsJbWVtY3B5KCZzLT5zX3ZvbHVtZV9pZCwgJnNlcnZlci0+ZnNpZCwgc2l6ZW9mKHMtPnNfdm9sdW1lX2lkKSk7DQo+ICAJcmV0dXJuIHJldDsNCj4gIH0NCj4gIA0KPiANCj4gLS0NCj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LW5mcyIgaW4NCj4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcNCj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K--To unsubscribe from this list: send the line "unsubscribe linux-nfs" inthe body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.orgMore majordomo info at  http://vger.kernel.org/majordomo-info.html

Myklebust, Trond [off-list ref] wrote:

Hmm... As far as I can see you are still doing an nfs_revalidate_inode()
in the non-forced case. That will cause expired attributes to be
retrieved from the server.

Revalidation is only done when you force it or explicitly ask for a basic stat
or the data version number:

-	if (need_atime)
-		err = __nfs_revalidate_inode(NFS_SERVER(inode), inode);
-	else
-		err = nfs_revalidate_inode(NFS_SERVER(inode), inode);
-	if (!err) {
-		generic_fillattr(inode, stat);
-		stat->ino = nfs_compat_user_ino64(NFS_FILEID(inode));
+	if (force || stat->request_mask & (XSTAT_BASIC_STATS | XSTAT_VERSION)) {
+		if (force || need_atime)
+			err = __nfs_revalidate_inode(NFS_SERVER(inode), inode);
+		else
+			err = nfs_revalidate_inode(NFS_SERVER(inode), inode);
+		if (err)
+			goto out;

Unfortunately, I think I have to revalidate if any of XSTAT_BASIC_STATS are
requested to maintain compatibility with stat() so that stat() can be done
with xstat().  On the other hand, stat() could be done by userspace with
xstat() and AT_FORCE_ATTR_SYNC, I suppose.

David