Hi Trond,
I would some help understanding attributes management.
Right now, any time a directory inode that was marked with
INVALID_ACCESS (say to a change_attribute changed) ends up triggering
sending a duplicate GETATTR. I don't think that's correct.
In nfs_execute_ok() we check if (nfs_check_cache_invalid(inode,
NFS_INO_INVALID_ACCESS)) and the call __nfs_revalidate_inode() which
will trigger GETATTR. I don't understand why after calling this
function the INVALID_ACCESS doesn't get cleared? Because that's what
causes the double GETATTR to be sent.
On the open path, the first time the getattr is sent is in
Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110 [nfsv4]
Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370 [nfs]
Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
Jul 19 15:39:52 ipa18 kernel: link_path_walk+0x29d/0x520
Jul 19 15:39:52 ipa18 kernel: path_openat+0xf6/0x1230
Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
Jul 19 15:39:52 ipa18 kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9
And then again during the open
Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110 [nfsv4]
Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370 [nfs]
Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
Jul 19 15:39:52 ipa18 kernel: path_openat+0x942/0x1230
Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
Jul 19 15:39:52 ipa18 kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9
Can't we remove INVALID_ACCESS after we revalidated the inode? This
removes the duplicated GETATTRs. Is this a valid way of fixing this
issue?
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 8f8e9e9..2b55a45 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2504,6 +2504,8 @@ static int nfs_execute_ok(struct inode *inode, int mask)
if (mask & MAY_NOT_BLOCK)
return -ECHILD;
ret = __nfs_revalidate_inode(server, inode);
+ if (!ret)
+ NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_ACCESS;
}
if (ret == 0 && !execute_ok(inode))
ret = -EACCES;
T24gRnJpLCAyMDE4LTA3LTIwIGF0IDEzOjI2IC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90
ZToNCj4gSGkgVHJvbmQsDQo+IA0KPiBJIHdvdWxkIHNvbWUgaGVscCB1bmRlcnN0YW5kaW5nIGF0
dHJpYnV0ZXMgbWFuYWdlbWVudC4NCj4gDQo+IFJpZ2h0IG5vdywgYW55IHRpbWUgYSBkaXJlY3Rv
cnkgaW5vZGUgdGhhdCB3YXMgbWFya2VkIHdpdGgNCj4gSU5WQUxJRF9BQ0NFU1MgKHNheSB0byBh
IGNoYW5nZV9hdHRyaWJ1dGUgY2hhbmdlZCkgZW5kcyB1cCB0cmlnZ2VyaW5nDQo+IHNlbmRpbmcg
YSBkdXBsaWNhdGUgR0VUQVRUUi4gSSBkb24ndCB0aGluayB0aGF0J3MgY29ycmVjdC4NCj4gDQo+
IEluIG5mc19leGVjdXRlX29rKCkgd2UgY2hlY2sgaWYgKG5mc19jaGVja19jYWNoZV9pbnZhbGlk
KGlub2RlLA0KPiBORlNfSU5PX0lOVkFMSURfQUNDRVNTKSkgYW5kIHRoZSBjYWxsIF9fbmZzX3Jl
dmFsaWRhdGVfaW5vZGUoKSB3aGljaA0KPiB3aWxsIHRyaWdnZXIgR0VUQVRUUi4gSSBkb24ndCB1
bmRlcnN0YW5kIHdoeSBhZnRlciBjYWxsaW5nIHRoaXMNCj4gZnVuY3Rpb24gdGhlIElOVkFMSURf
QUNDRVNTIGRvZXNuJ3QgZ2V0IGNsZWFyZWQ/IEJlY2F1c2UgdGhhdCdzIHdoYXQNCj4gY2F1c2Vz
IHRoZSBkb3VibGUgR0VUQVRUUiB0byBiZSBzZW50Lg0KPiANCj4gT24gdGhlIG9wZW4gcGF0aCwg
dGhlIGZpcnN0IHRpbWUgdGhlIGdldGF0dHIgaXMgc2VudCBpcyBpbg0KPiANCj4gSnVsIDE5IDE1
OjM5OjUyIGlwYTE4IGtlcm5lbDogZHVtcF9zdGFjaysweDVhLzB4NzMNCj4gSnVsIDE5IDE1OjM5
OjUyIGlwYTE4IGtlcm5lbDogbmZzNF9wcm9jX2dldGF0dHIrMHg2NS8weDExMCBbbmZzdjRdDQo+
IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IF9fbmZzX3JldmFsaWRhdGVfaW5vZGUrMHhl
MS8weDM3MCBbbmZzXQ0KPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBuZnNfcGVybWlz
c2lvbisweDE2Yi8weDFmMCBbbmZzXQ0KPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBp
bm9kZV9wZXJtaXNzaW9uKzB4YWIvMHgxMzANCj4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5l
bDogbGlua19wYXRoX3dhbGsrMHgyOWQvMHg1MjANCj4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtl
cm5lbDogcGF0aF9vcGVuYXQrMHhmNi8weDEyMzANCj4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtl
cm5lbDogZG9fZmlscF9vcGVuKzB4OTEvMHgxMDANCj4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtl
cm5lbDogZG9fc3lzX29wZW4rMHgxMjYvMHgyMTANCj4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtl
cm5lbDogZG9fc3lzY2FsbF82NCsweDU1LzB4MTgwDQo+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBr
ZXJuZWw6DQo+IGVudHJ5X1NZU0NBTExfNjRfYWZ0ZXJfaHdmcmFtZSsweDQ0LzB4YTkNCj4gDQo+
IEFuZCB0aGVuIGFnYWluIGR1cmluZyB0aGUgb3Blbg0KPiANCj4gSnVsIDE5IDE1OjM5OjUyIGlw
YTE4IGtlcm5lbDogZHVtcF9zdGFjaysweDVhLzB4NzMNCj4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4
IGtlcm5lbDogbmZzNF9wcm9jX2dldGF0dHIrMHg2NS8weDExMCBbbmZzdjRdDQo+IEp1bCAxOSAx
NTozOTo1MiBpcGExOCBrZXJuZWw6IF9fbmZzX3JldmFsaWRhdGVfaW5vZGUrMHhlMS8weDM3MCBb
bmZzXQ0KPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBuZnNfcGVybWlzc2lvbisweDE2
Yi8weDFmMCBbbmZzXQ0KPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBpbm9kZV9wZXJt
aXNzaW9uKzB4YWIvMHgxMzANCj4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogcGF0aF9v
cGVuYXQrMHg5NDIvMHgxMjMwDQo+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IGRvX2Zp
bHBfb3BlbisweDkxLzB4MTAwDQo+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IGRvX3N5
c19vcGVuKzB4MTI2LzB4MjEwDQo+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IGRvX3N5
c2NhbGxfNjQrMHg1NS8weDE4MA0KPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOg0KPiBl
bnRyeV9TWVNDQUxMXzY0X2FmdGVyX2h3ZnJhbWUrMHg0NC8weGE5DQo+IA0KPiBDYW4ndCB3ZSBy
ZW1vdmUgSU5WQUxJRF9BQ0NFU1MgYWZ0ZXIgd2UgcmV2YWxpZGF0ZWQgdGhlIGlub2RlPyBUaGlz
DQo+IHJlbW92ZXMgdGhlIGR1cGxpY2F0ZWQgR0VUQVRUUnMuIElzIHRoaXMgYSB2YWxpZCB3YXkg
b2YgZml4aW5nIHRoaXMNCj4gaXNzdWU/DQo+IA0KPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2Rpci5j
IGIvZnMvbmZzL2Rpci5jDQo+IGluZGV4IDhmOGU5ZTkuLjJiNTVhNDUgMTAwNjQ0DQo+IC0tLSBh
L2ZzL25mcy9kaXIuYw0KPiArKysgYi9mcy9uZnMvZGlyLmMNCj4gQEAgLTI1MDQsNiArMjUwNCw4
IEBAIHN0YXRpYyBpbnQgbmZzX2V4ZWN1dGVfb2soc3RydWN0IGlub2RlICppbm9kZSwNCj4gaW50
IG1hc2spDQo+ICAgICAgICAgICAgICAgICBpZiAobWFzayAmIE1BWV9OT1RfQkxPQ0spDQo+ICAg
ICAgICAgICAgICAgICAgICAgICAgIHJldHVybiAtRUNISUxEOw0KPiAgICAgICAgICAgICAgICAg
cmV0ID0gX19uZnNfcmV2YWxpZGF0ZV9pbm9kZShzZXJ2ZXIsIGlub2RlKTsNCj4gKyAgICAgICAg
ICAgICAgIGlmICghcmV0KQ0KPiArICAgICAgICAgICAgICAgICAgICAgICBORlNfSShpbm9kZSkt
PmNhY2hlX3ZhbGlkaXR5ICY9DQo+IH5ORlNfSU5PX0lOVkFMSURfQUNDRVNTOw0KPiAgICAgICAg
IH0NCj4gICAgICAgICBpZiAocmV0ID09IDAgJiYgIWV4ZWN1dGVfb2soaW5vZGUpKQ0KPiAgICAg
ICAgICAgICAgICAgcmV0ID0gLUVBQ0NFUzsNCg0KDQpJIGRvbid0IHNlZSBob3cgdGhlIGFib3Zl
IG1ha2VzIHNlbnNlLg0KDQpFaXRoZXIgdGhlIGF0dHJpYnV0ZSByZXZhbGlkYXRpb24gY29uZmly
bWVkIHRoYXQgdGhlIGNoYW5nZSBhdHRyLCBtb2RlDQorIHVpZCArIGdpZCBhcmUgdW5jaGFuZ2Vk
LCBpbiB3aGljaCBjYXNlIHRoZSBjYWxsIHRvDQpuZnNfcmVmcmVzaF9pbm9kZSgpIGR1cmluZyBy
ZXZhbGlkYXRpb24gd2lsbCBmYWlsIHRvIHNldA0KTkZTX0lOT19JTlZBTElEX0FDQ0VTUywgb3Ig
aXQgY29uZmlybWVkIHRoYXQgYXQgbGVhc3Qgb25lIG9mIHRoZW0gaGFzDQpjaGFuZ2VkLCBpbiB3
aGljaCBjYXNlIHdlIGRvIHdhbnQgdG8gcmV2YWxpZGF0ZSB0aGUgYWNjZXNzIGNhY2hlLg0KDQot
LSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyLCBIYW1tZXJz
cGFjZQ0KdHJvbmQubXlrbGVidXN0QGhhbW1lcnNwYWNlLmNvbQ0KDQo=
On Fri, Jul 20, 2018 at 2:05 PM, Trond Myklebust
<[email protected]> wrote:
> On Fri, 2018-07-20 at 13:26 -0400, Olga Kornievskaia wrote:
>> Hi Trond,
>>
>> I would some help understanding attributes management.
>>
>> Right now, any time a directory inode that was marked with
>> INVALID_ACCESS (say to a change_attribute changed) ends up triggering
>> sending a duplicate GETATTR. I don't think that's correct.
>>
>> In nfs_execute_ok() we check if (nfs_check_cache_invalid(inode,
>> NFS_INO_INVALID_ACCESS)) and the call __nfs_revalidate_inode() which
>> will trigger GETATTR. I don't understand why after calling this
>> function the INVALID_ACCESS doesn't get cleared? Because that's what
>> causes the double GETATTR to be sent.
>>
>> On the open path, the first time the getattr is sent is in
>>
>> Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>> Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110 [nfsv4]
>> Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370 [nfs]
>> Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
>> Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>> Jul 19 15:39:52 ipa18 kernel: link_path_walk+0x29d/0x520
>> Jul 19 15:39:52 ipa18 kernel: path_openat+0xf6/0x1230
>> Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>> Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>> Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>> Jul 19 15:39:52 ipa18 kernel:
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> And then again during the open
>>
>> Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>> Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110 [nfsv4]
>> Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370 [nfs]
>> Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
>> Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>> Jul 19 15:39:52 ipa18 kernel: path_openat+0x942/0x1230
>> Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>> Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>> Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>> Jul 19 15:39:52 ipa18 kernel:
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> Can't we remove INVALID_ACCESS after we revalidated the inode? This
>> removes the duplicated GETATTRs. Is this a valid way of fixing this
>> issue?
>>
>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> index 8f8e9e9..2b55a45 100644
>> --- a/fs/nfs/dir.c
>> +++ b/fs/nfs/dir.c
>> @@ -2504,6 +2504,8 @@ static int nfs_execute_ok(struct inode *inode,
>> int mask)
>> if (mask & MAY_NOT_BLOCK)
>> return -ECHILD;
>> ret = __nfs_revalidate_inode(server, inode);
>> + if (!ret)
>> + NFS_I(inode)->cache_validity &=
>> ~NFS_INO_INVALID_ACCESS;
>> }
>> if (ret == 0 && !execute_ok(inode))
>> ret = -EACCES;
>
>
> I don't see how the above makes sense.
>
> Either the attribute revalidation confirmed that the change attr, mode
> + uid + gid are unchanged, in which case the call to
> nfs_refresh_inode() during revalidation will fail to set
> NFS_INO_INVALID_ACCESS, or it confirmed that at least one of them has
> changed, in which case we do want to revalidate the access cache.
I'm confused as to against what are we checking then?
We flagged a directory inode to have invalid attributes. So we sent a
GETATTR. I would think that after getting the result all our
attributes should be valid. Why would a client as the next operation
send another GETATTR for the same inode?
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> [email protected]
>
On Fri, Jul 20, 2018 at 2:17 PM, Olga Kornievskaia <[email protected]> wrote:
> On Fri, Jul 20, 2018 at 2:05 PM, Trond Myklebust
> <[email protected]> wrote:
>> On Fri, 2018-07-20 at 13:26 -0400, Olga Kornievskaia wrote:
>>> Hi Trond,
>>>
>>> I would some help understanding attributes management.
>>>
>>> Right now, any time a directory inode that was marked with
>>> INVALID_ACCESS (say to a change_attribute changed) ends up triggering
>>> sending a duplicate GETATTR. I don't think that's correct.
>>>
>>> In nfs_execute_ok() we check if (nfs_check_cache_invalid(inode,
>>> NFS_INO_INVALID_ACCESS)) and the call __nfs_revalidate_inode() which
>>> will trigger GETATTR. I don't understand why after calling this
>>> function the INVALID_ACCESS doesn't get cleared? Because that's what
>>> causes the double GETATTR to be sent.
>>>
>>> On the open path, the first time the getattr is sent is in
>>>
>>> Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>>> Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110 [nfsv4]
>>> Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370 [nfs]
>>> Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
>>> Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>>> Jul 19 15:39:52 ipa18 kernel: link_path_walk+0x29d/0x520
>>> Jul 19 15:39:52 ipa18 kernel: path_openat+0xf6/0x1230
>>> Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>>> Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>>> Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>>> Jul 19 15:39:52 ipa18 kernel:
>>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>>
>>> And then again during the open
>>>
>>> Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>>> Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110 [nfsv4]
>>> Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370 [nfs]
>>> Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
>>> Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>>> Jul 19 15:39:52 ipa18 kernel: path_openat+0x942/0x1230
>>> Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>>> Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>>> Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>>> Jul 19 15:39:52 ipa18 kernel:
>>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>>
>>> Can't we remove INVALID_ACCESS after we revalidated the inode? This
>>> removes the duplicated GETATTRs. Is this a valid way of fixing this
>>> issue?
>>>
>>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>>> index 8f8e9e9..2b55a45 100644
>>> --- a/fs/nfs/dir.c
>>> +++ b/fs/nfs/dir.c
>>> @@ -2504,6 +2504,8 @@ static int nfs_execute_ok(struct inode *inode,
>>> int mask)
>>> if (mask & MAY_NOT_BLOCK)
>>> return -ECHILD;
>>> ret = __nfs_revalidate_inode(server, inode);
>>> + if (!ret)
>>> + NFS_I(inode)->cache_validity &=
>>> ~NFS_INO_INVALID_ACCESS;
>>> }
>>> if (ret == 0 && !execute_ok(inode))
>>> ret = -EACCES;
>>
>>
>> I don't see how the above makes sense.
>>
>> Either the attribute revalidation confirmed that the change attr, mode
>> + uid + gid are unchanged, in which case the call to
>> nfs_refresh_inode() during revalidation will fail to set
>> NFS_INO_INVALID_ACCESS, or it confirmed that at least one of them has
>> changed, in which case we do want to revalidate the access cache.
>
> I'm confused as to against what are we checking then?
>
> We flagged a directory inode to have invalid attributes. So we sent a
> GETATTR. I would think that after getting the result all our
> attributes should be valid. Why would a client as the next operation
> send another GETATTR for the same inode?
>
I didn't answer your question but rather explained what I see client do.
Let's me see if I can try again:
in nfs_execute_ok() the check for the INVALID_CACHE is true so the
code calls __nfs_revalidate_inode() which ends up sending a GETATTR.
Could it be that what's happening is that GETATTRs reply for the
change_attr is different than what we have stored. BUT that's obvious,
we knew that to begin with since we are validating the attributes. So
we update it and then we send another GETATTR now the GETATTR sends
back the "same" change_attr and this time it matches what we have
stored. Why is this 2nd GETATTR necessary? The attribute should be
marked as valid after a getting a successful GETATTR.
>>
>> --
>> Trond Myklebust
>> Linux NFS client maintainer, Hammerspace
>> [email protected]
>>
T24gRnJpLCAyMDE4LTA3LTIwIGF0IDE0OjE3IC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90
ZToNCj4gT24gRnJpLCBKdWwgMjAsIDIwMTggYXQgMjowNSBQTSwgVHJvbmQgTXlrbGVidXN0DQo+
IDx0cm9uZG15QGhhbW1lcnNwYWNlLmNvbT4gd3JvdGU6DQo+ID4gT24gRnJpLCAyMDE4LTA3LTIw
IGF0IDEzOjI2IC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90ZToNCj4gPiA+IEhpIFRyb25k
LA0KPiA+ID4gDQo+ID4gPiBJIHdvdWxkIHNvbWUgaGVscCB1bmRlcnN0YW5kaW5nIGF0dHJpYnV0
ZXMgbWFuYWdlbWVudC4NCj4gPiA+IA0KPiA+ID4gUmlnaHQgbm93LCBhbnkgdGltZSBhIGRpcmVj
dG9yeSBpbm9kZSB0aGF0IHdhcyBtYXJrZWQgd2l0aA0KPiA+ID4gSU5WQUxJRF9BQ0NFU1MgKHNh
eSB0byBhIGNoYW5nZV9hdHRyaWJ1dGUgY2hhbmdlZCkgZW5kcyB1cA0KPiA+ID4gdHJpZ2dlcmlu
Zw0KPiA+ID4gc2VuZGluZyBhIGR1cGxpY2F0ZSBHRVRBVFRSLiBJIGRvbid0IHRoaW5rIHRoYXQn
cyBjb3JyZWN0Lg0KPiA+ID4gDQo+ID4gPiBJbiBuZnNfZXhlY3V0ZV9vaygpIHdlIGNoZWNrIGlm
IChuZnNfY2hlY2tfY2FjaGVfaW52YWxpZChpbm9kZSwNCj4gPiA+IE5GU19JTk9fSU5WQUxJRF9B
Q0NFU1MpKSBhbmQgdGhlIGNhbGwgX19uZnNfcmV2YWxpZGF0ZV9pbm9kZSgpDQo+ID4gPiB3aGlj
aA0KPiA+ID4gd2lsbCB0cmlnZ2VyIEdFVEFUVFIuIEkgZG9uJ3QgdW5kZXJzdGFuZCB3aHkgYWZ0
ZXIgY2FsbGluZyB0aGlzDQo+ID4gPiBmdW5jdGlvbiB0aGUgSU5WQUxJRF9BQ0NFU1MgZG9lc24n
dCBnZXQgY2xlYXJlZD8gQmVjYXVzZSB0aGF0J3MNCj4gPiA+IHdoYXQNCj4gPiA+IGNhdXNlcyB0
aGUgZG91YmxlIEdFVEFUVFIgdG8gYmUgc2VudC4NCj4gPiA+IA0KPiA+ID4gT24gdGhlIG9wZW4g
cGF0aCwgdGhlIGZpcnN0IHRpbWUgdGhlIGdldGF0dHIgaXMgc2VudCBpcyBpbg0KPiA+ID4gDQo+
ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBkdW1wX3N0YWNrKzB4NWEvMHg3Mw0K
PiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogbmZzNF9wcm9jX2dldGF0dHIrMHg2
NS8weDExMA0KPiA+ID4gW25mc3Y0XQ0KPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5l
bDogX19uZnNfcmV2YWxpZGF0ZV9pbm9kZSsweGUxLzB4MzcwDQo+ID4gPiBbbmZzXQ0KPiA+ID4g
SnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogbmZzX3Blcm1pc3Npb24rMHgxNmIvMHgxZjAg
W25mc10NCj4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IGlub2RlX3Blcm1pc3Np
b24rMHhhYi8weDEzMA0KPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogbGlua19w
YXRoX3dhbGsrMHgyOWQvMHg1MjANCj4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6
IHBhdGhfb3BlbmF0KzB4ZjYvMHgxMjMwDQo+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2Vy
bmVsOiBkb19maWxwX29wZW4rMHg5MS8weDEwMA0KPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4
IGtlcm5lbDogZG9fc3lzX29wZW4rMHgxMjYvMHgyMTANCj4gPiA+IEp1bCAxOSAxNTozOTo1MiBp
cGExOCBrZXJuZWw6IGRvX3N5c2NhbGxfNjQrMHg1NS8weDE4MA0KPiA+ID4gSnVsIDE5IDE1OjM5
OjUyIGlwYTE4IGtlcm5lbDoNCj4gPiA+IGVudHJ5X1NZU0NBTExfNjRfYWZ0ZXJfaHdmcmFtZSsw
eDQ0LzB4YTkNCj4gPiA+IA0KPiA+ID4gQW5kIHRoZW4gYWdhaW4gZHVyaW5nIHRoZSBvcGVuDQo+
ID4gPiANCj4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IGR1bXBfc3RhY2srMHg1
YS8weDczDQo+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBuZnM0X3Byb2NfZ2V0
YXR0cisweDY1LzB4MTEwDQo+ID4gPiBbbmZzdjRdDQo+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBh
MTgga2VybmVsOiBfX25mc19yZXZhbGlkYXRlX2lub2RlKzB4ZTEvMHgzNzANCj4gPiA+IFtuZnNd
DQo+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBuZnNfcGVybWlzc2lvbisweDE2
Yi8weDFmMCBbbmZzXQ0KPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogaW5vZGVf
cGVybWlzc2lvbisweGFiLzB4MTMwDQo+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVs
OiBwYXRoX29wZW5hdCsweDk0Mi8weDEyMzANCj4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBr
ZXJuZWw6IGRvX2ZpbHBfb3BlbisweDkxLzB4MTAwDQo+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBh
MTgga2VybmVsOiBkb19zeXNfb3BlbisweDEyNi8weDIxMA0KPiA+ID4gSnVsIDE5IDE1OjM5OjUy
IGlwYTE4IGtlcm5lbDogZG9fc3lzY2FsbF82NCsweDU1LzB4MTgwDQo+ID4gPiBKdWwgMTkgMTU6
Mzk6NTIgaXBhMTgga2VybmVsOg0KPiA+ID4gZW50cnlfU1lTQ0FMTF82NF9hZnRlcl9od2ZyYW1l
KzB4NDQvMHhhOQ0KPiA+ID4gDQo+ID4gPiBDYW4ndCB3ZSByZW1vdmUgSU5WQUxJRF9BQ0NFU1Mg
YWZ0ZXIgd2UgcmV2YWxpZGF0ZWQgdGhlIGlub2RlPw0KPiA+ID4gVGhpcw0KPiA+ID4gcmVtb3Zl
cyB0aGUgZHVwbGljYXRlZCBHRVRBVFRScy4gSXMgdGhpcyBhIHZhbGlkIHdheSBvZiBmaXhpbmcN
Cj4gPiA+IHRoaXMNCj4gPiA+IGlzc3VlPw0KPiA+ID4gDQo+ID4gPiBkaWZmIC0tZ2l0IGEvZnMv
bmZzL2Rpci5jIGIvZnMvbmZzL2Rpci5jDQo+ID4gPiBpbmRleCA4ZjhlOWU5Li4yYjU1YTQ1IDEw
MDY0NA0KPiA+ID4gLS0tIGEvZnMvbmZzL2Rpci5jDQo+ID4gPiArKysgYi9mcy9uZnMvZGlyLmMN
Cj4gPiA+IEBAIC0yNTA0LDYgKzI1MDQsOCBAQCBzdGF0aWMgaW50IG5mc19leGVjdXRlX29rKHN0
cnVjdCBpbm9kZQ0KPiA+ID4gKmlub2RlLA0KPiA+ID4gaW50IG1hc2spDQo+ID4gPiAgICAgICAg
ICAgICAgICAgaWYgKG1hc2sgJiBNQVlfTk9UX0JMT0NLKQ0KPiA+ID4gICAgICAgICAgICAgICAg
ICAgICAgICAgcmV0dXJuIC1FQ0hJTEQ7DQo+ID4gPiAgICAgICAgICAgICAgICAgcmV0ID0gX19u
ZnNfcmV2YWxpZGF0ZV9pbm9kZShzZXJ2ZXIsIGlub2RlKTsNCj4gPiA+ICsgICAgICAgICAgICAg
ICBpZiAoIXJldCkNCj4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgIE5GU19JKGlub2RlKS0+
Y2FjaGVfdmFsaWRpdHkgJj0NCj4gPiA+IH5ORlNfSU5PX0lOVkFMSURfQUNDRVNTOw0KPiA+ID4g
ICAgICAgICB9DQo+ID4gPiAgICAgICAgIGlmIChyZXQgPT0gMCAmJiAhZXhlY3V0ZV9vayhpbm9k
ZSkpDQo+ID4gPiAgICAgICAgICAgICAgICAgcmV0ID0gLUVBQ0NFUzsNCj4gPiANCj4gPiANCj4g
PiBJIGRvbid0IHNlZSBob3cgdGhlIGFib3ZlIG1ha2VzIHNlbnNlLg0KPiA+IA0KPiA+IEVpdGhl
ciB0aGUgYXR0cmlidXRlIHJldmFsaWRhdGlvbiBjb25maXJtZWQgdGhhdCB0aGUgY2hhbmdlIGF0
dHIsDQo+ID4gbW9kZQ0KPiA+ICsgdWlkICsgZ2lkIGFyZSB1bmNoYW5nZWQsIGluIHdoaWNoIGNh
c2UgdGhlIGNhbGwgdG8NCj4gPiBuZnNfcmVmcmVzaF9pbm9kZSgpIGR1cmluZyByZXZhbGlkYXRp
b24gd2lsbCBmYWlsIHRvIHNldA0KPiA+IE5GU19JTk9fSU5WQUxJRF9BQ0NFU1MsIG9yIGl0IGNv
bmZpcm1lZCB0aGF0IGF0IGxlYXN0IG9uZSBvZiB0aGVtDQo+ID4gaGFzDQo+ID4gY2hhbmdlZCwg
aW4gd2hpY2ggY2FzZSB3ZSBkbyB3YW50IHRvIHJldmFsaWRhdGUgdGhlIGFjY2VzcyBjYWNoZS4N
Cj4gDQo+IEknbSBjb25mdXNlZCBhcyB0byBhZ2FpbnN0IHdoYXQgYXJlIHdlIGNoZWNraW5nIHRo
ZW4/DQo+IA0KPiBXZSBmbGFnZ2VkIGEgZGlyZWN0b3J5IGlub2RlIHRvIGhhdmUgaW52YWxpZCBh
dHRyaWJ1dGVzLiBTbyB3ZSBzZW50IGENCj4gR0VUQVRUUi4gSSB3b3VsZCB0aGluayB0aGF0IGFm
dGVyIGdldHRpbmcgdGhlIHJlc3VsdCBhbGwgb3VyDQo+IGF0dHJpYnV0ZXMgc2hvdWxkIGJlIHZh
bGlkLiBXaHkgd291bGQgYSBjbGllbnQgYXMgdGhlIG5leHQgb3BlcmF0aW9uDQo+IHNlbmQgYW5v
dGhlciBHRVRBVFRSIGZvciB0aGUgc2FtZSBpbm9kZT8NCg0KVGhlIHN0YXRlIE5GU19JTk9fSU5W
QUxJRF9BQ0NFU1Mgc2hvdWxkIG5vdCBiZSB0cmlnZ2VyaW5nIGFueSBHRVRBVFRSDQpjYWxscy4g
VGhlIGludGVudGlvbiBpcyB0aGF0IGl0IHNob3VsZCBvbmx5IGNhdXNlIHRoZSBhY2Nlc3MgY2Fj
aGUgdG8NCmJlIHJldmFsaWRhdGVkLg0KDQpJT1c6IEl0IGNvdWxkIHRyaWdnZXIgZnVydGhlciBB
Q0NFU1MgY2FsbHMuDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpDVE8sIEhhbW1lcnNwYWNlIElu
Yw0KNDMwMCBFbCBDYW1pbm8gUmVhbCwgU3VpdGUgMTA1DQpMb3MgQWx0b3MsIENBIDk0MDIyDQp3
d3cuaGFtbWVyLnNwYWNlDQoNCg==
T24gRnJpLCAyMDE4LTA3LTIwIGF0IDE0OjQwIC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90
ZToNCj4gT24gRnJpLCBKdWwgMjAsIDIwMTggYXQgMjoxNyBQTSwgT2xnYSBLb3JuaWV2c2thaWEg
PGFnbG9AdW1pY2guZWR1Pg0KPiB3cm90ZToNCj4gPiBPbiBGcmksIEp1bCAyMCwgMjAxOCBhdCAy
OjA1IFBNLCBUcm9uZCBNeWtsZWJ1c3QNCj4gPiA8dHJvbmRteUBoYW1tZXJzcGFjZS5jb20+IHdy
b3RlOg0KPiA+ID4gT24gRnJpLCAyMDE4LTA3LTIwIGF0IDEzOjI2IC0wNDAwLCBPbGdhIEtvcm5p
ZXZza2FpYSB3cm90ZToNCj4gPiA+ID4gSGkgVHJvbmQsDQo+ID4gPiA+IA0KPiA+ID4gPiBJIHdv
dWxkIHNvbWUgaGVscCB1bmRlcnN0YW5kaW5nIGF0dHJpYnV0ZXMgbWFuYWdlbWVudC4NCj4gPiA+
ID4gDQo+ID4gPiA+IFJpZ2h0IG5vdywgYW55IHRpbWUgYSBkaXJlY3RvcnkgaW5vZGUgdGhhdCB3
YXMgbWFya2VkIHdpdGgNCj4gPiA+ID4gSU5WQUxJRF9BQ0NFU1MgKHNheSB0byBhIGNoYW5nZV9h
dHRyaWJ1dGUgY2hhbmdlZCkgZW5kcyB1cA0KPiA+ID4gPiB0cmlnZ2VyaW5nDQo+ID4gPiA+IHNl
bmRpbmcgYSBkdXBsaWNhdGUgR0VUQVRUUi4gSSBkb24ndCB0aGluayB0aGF0J3MgY29ycmVjdC4N
Cj4gPiA+ID4gDQo+ID4gPiA+IEluIG5mc19leGVjdXRlX29rKCkgd2UgY2hlY2sgaWYgKG5mc19j
aGVja19jYWNoZV9pbnZhbGlkKGlub2RlLA0KPiA+ID4gPiBORlNfSU5PX0lOVkFMSURfQUNDRVNT
KSkgYW5kIHRoZSBjYWxsIF9fbmZzX3JldmFsaWRhdGVfaW5vZGUoKQ0KPiA+ID4gPiB3aGljaA0K
PiA+ID4gPiB3aWxsIHRyaWdnZXIgR0VUQVRUUi4gSSBkb24ndCB1bmRlcnN0YW5kIHdoeSBhZnRl
ciBjYWxsaW5nIHRoaXMNCj4gPiA+ID4gZnVuY3Rpb24gdGhlIElOVkFMSURfQUNDRVNTIGRvZXNu
J3QgZ2V0IGNsZWFyZWQ/IEJlY2F1c2UgdGhhdCdzDQo+ID4gPiA+IHdoYXQNCj4gPiA+ID4gY2F1
c2VzIHRoZSBkb3VibGUgR0VUQVRUUiB0byBiZSBzZW50Lg0KPiA+ID4gPiANCj4gPiA+ID4gT24g
dGhlIG9wZW4gcGF0aCwgdGhlIGZpcnN0IHRpbWUgdGhlIGdldGF0dHIgaXMgc2VudCBpcyBpbg0K
PiA+ID4gPiANCj4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogZHVtcF9zdGFj
aysweDVhLzB4NzMNCj4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogbmZzNF9w
cm9jX2dldGF0dHIrMHg2NS8weDExMA0KPiA+ID4gPiBbbmZzdjRdDQo+ID4gPiA+IEp1bCAxOSAx
NTozOTo1MiBpcGExOCBrZXJuZWw6IF9fbmZzX3JldmFsaWRhdGVfaW5vZGUrMHhlMS8weDM3MA0K
PiA+ID4gPiBbbmZzXQ0KPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBuZnNf
cGVybWlzc2lvbisweDE2Yi8weDFmMCBbbmZzXQ0KPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBh
MTgga2VybmVsOiBpbm9kZV9wZXJtaXNzaW9uKzB4YWIvMHgxMzANCj4gPiA+ID4gSnVsIDE5IDE1
OjM5OjUyIGlwYTE4IGtlcm5lbDogbGlua19wYXRoX3dhbGsrMHgyOWQvMHg1MjANCj4gPiA+ID4g
SnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogcGF0aF9vcGVuYXQrMHhmNi8weDEyMzANCj4g
PiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogZG9fZmlscF9vcGVuKzB4OTEvMHgx
MDANCj4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogZG9fc3lzX29wZW4rMHgx
MjYvMHgyMTANCj4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogZG9fc3lzY2Fs
bF82NCsweDU1LzB4MTgwDQo+ID4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6DQo+
ID4gPiA+IGVudHJ5X1NZU0NBTExfNjRfYWZ0ZXJfaHdmcmFtZSsweDQ0LzB4YTkNCj4gPiA+ID4g
DQo+ID4gPiA+IEFuZCB0aGVuIGFnYWluIGR1cmluZyB0aGUgb3Blbg0KPiA+ID4gPiANCj4gPiA+
ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogZHVtcF9zdGFjaysweDVhLzB4NzMNCj4g
PiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogbmZzNF9wcm9jX2dldGF0dHIrMHg2
NS8weDExMA0KPiA+ID4gPiBbbmZzdjRdDQo+ID4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBr
ZXJuZWw6IF9fbmZzX3JldmFsaWRhdGVfaW5vZGUrMHhlMS8weDM3MA0KPiA+ID4gPiBbbmZzXQ0K
PiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBuZnNfcGVybWlzc2lvbisweDE2
Yi8weDFmMCBbbmZzXQ0KPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBpbm9k
ZV9wZXJtaXNzaW9uKzB4YWIvMHgxMzANCj4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtl
cm5lbDogcGF0aF9vcGVuYXQrMHg5NDIvMHgxMjMwDQo+ID4gPiA+IEp1bCAxOSAxNTozOTo1MiBp
cGExOCBrZXJuZWw6IGRvX2ZpbHBfb3BlbisweDkxLzB4MTAwDQo+ID4gPiA+IEp1bCAxOSAxNToz
OTo1MiBpcGExOCBrZXJuZWw6IGRvX3N5c19vcGVuKzB4MTI2LzB4MjEwDQo+ID4gPiA+IEp1bCAx
OSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IGRvX3N5c2NhbGxfNjQrMHg1NS8weDE4MA0KPiA+ID4g
PiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOg0KPiA+ID4gPiBlbnRyeV9TWVNDQUxMXzY0
X2FmdGVyX2h3ZnJhbWUrMHg0NC8weGE5DQo+ID4gPiA+IA0KPiA+ID4gPiBDYW4ndCB3ZSByZW1v
dmUgSU5WQUxJRF9BQ0NFU1MgYWZ0ZXIgd2UgcmV2YWxpZGF0ZWQgdGhlIGlub2RlPw0KPiA+ID4g
PiBUaGlzDQo+ID4gPiA+IHJlbW92ZXMgdGhlIGR1cGxpY2F0ZWQgR0VUQVRUUnMuIElzIHRoaXMg
YSB2YWxpZCB3YXkgb2YgZml4aW5nDQo+ID4gPiA+IHRoaXMNCj4gPiA+ID4gaXNzdWU/DQo+ID4g
PiA+IA0KPiA+ID4gPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2Rpci5jIGIvZnMvbmZzL2Rpci5jDQo+
ID4gPiA+IGluZGV4IDhmOGU5ZTkuLjJiNTVhNDUgMTAwNjQ0DQo+ID4gPiA+IC0tLSBhL2ZzL25m
cy9kaXIuYw0KPiA+ID4gPiArKysgYi9mcy9uZnMvZGlyLmMNCj4gPiA+ID4gQEAgLTI1MDQsNiAr
MjUwNCw4IEBAIHN0YXRpYyBpbnQgbmZzX2V4ZWN1dGVfb2soc3RydWN0IGlub2RlDQo+ID4gPiA+
ICppbm9kZSwNCj4gPiA+ID4gaW50IG1hc2spDQo+ID4gPiA+ICAgICAgICAgICAgICAgICBpZiAo
bWFzayAmIE1BWV9OT1RfQkxPQ0spDQo+ID4gPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIHJl
dHVybiAtRUNISUxEOw0KPiA+ID4gPiAgICAgICAgICAgICAgICAgcmV0ID0gX19uZnNfcmV2YWxp
ZGF0ZV9pbm9kZShzZXJ2ZXIsIGlub2RlKTsNCj4gPiA+ID4gKyAgICAgICAgICAgICAgIGlmICgh
cmV0KQ0KPiA+ID4gPiArICAgICAgICAgICAgICAgICAgICAgICBORlNfSShpbm9kZSktPmNhY2hl
X3ZhbGlkaXR5ICY9DQo+ID4gPiA+IH5ORlNfSU5PX0lOVkFMSURfQUNDRVNTOw0KPiA+ID4gPiAg
ICAgICAgIH0NCj4gPiA+ID4gICAgICAgICBpZiAocmV0ID09IDAgJiYgIWV4ZWN1dGVfb2soaW5v
ZGUpKQ0KPiA+ID4gPiAgICAgICAgICAgICAgICAgcmV0ID0gLUVBQ0NFUzsNCj4gPiA+IA0KPiA+
ID4gDQo+ID4gPiBJIGRvbid0IHNlZSBob3cgdGhlIGFib3ZlIG1ha2VzIHNlbnNlLg0KPiA+ID4g
DQo+ID4gPiBFaXRoZXIgdGhlIGF0dHJpYnV0ZSByZXZhbGlkYXRpb24gY29uZmlybWVkIHRoYXQg
dGhlIGNoYW5nZSBhdHRyLA0KPiA+ID4gbW9kZQ0KPiA+ID4gKyB1aWQgKyBnaWQgYXJlIHVuY2hh
bmdlZCwgaW4gd2hpY2ggY2FzZSB0aGUgY2FsbCB0bw0KPiA+ID4gbmZzX3JlZnJlc2hfaW5vZGUo
KSBkdXJpbmcgcmV2YWxpZGF0aW9uIHdpbGwgZmFpbCB0byBzZXQNCj4gPiA+IE5GU19JTk9fSU5W
QUxJRF9BQ0NFU1MsIG9yIGl0IGNvbmZpcm1lZCB0aGF0IGF0IGxlYXN0IG9uZSBvZiB0aGVtDQo+
ID4gPiBoYXMNCj4gPiA+IGNoYW5nZWQsIGluIHdoaWNoIGNhc2Ugd2UgZG8gd2FudCB0byByZXZh
bGlkYXRlIHRoZSBhY2Nlc3MgY2FjaGUuDQo+ID4gDQo+ID4gSSdtIGNvbmZ1c2VkIGFzIHRvIGFn
YWluc3Qgd2hhdCBhcmUgd2UgY2hlY2tpbmcgdGhlbj8NCj4gPiANCj4gPiBXZSBmbGFnZ2VkIGEg
ZGlyZWN0b3J5IGlub2RlIHRvIGhhdmUgaW52YWxpZCBhdHRyaWJ1dGVzLiBTbyB3ZSBzZW50DQo+
ID4gYQ0KPiA+IEdFVEFUVFIuIEkgd291bGQgdGhpbmsgdGhhdCBhZnRlciBnZXR0aW5nIHRoZSBy
ZXN1bHQgYWxsIG91cg0KPiA+IGF0dHJpYnV0ZXMgc2hvdWxkIGJlIHZhbGlkLiBXaHkgd291bGQg
YSBjbGllbnQgYXMgdGhlIG5leHQNCj4gPiBvcGVyYXRpb24NCj4gPiBzZW5kIGFub3RoZXIgR0VU
QVRUUiBmb3IgdGhlIHNhbWUgaW5vZGU/DQo+ID4gDQo+IA0KPiBJIGRpZG4ndCBhbnN3ZXIgeW91
ciBxdWVzdGlvbiBidXQgcmF0aGVyIGV4cGxhaW5lZCB3aGF0IEkgc2VlIGNsaWVudA0KPiBkby4N
Cj4gDQo+IExldCdzIG1lIHNlZSBpZiBJIGNhbiB0cnkgYWdhaW46DQo+IGluIG5mc19leGVjdXRl
X29rKCkgdGhlIGNoZWNrIGZvciB0aGUgSU5WQUxJRF9DQUNIRSBpcyB0cnVlIHNvIHRoZQ0KPiBj
b2RlIGNhbGxzIF9fbmZzX3JldmFsaWRhdGVfaW5vZGUoKSB3aGljaCBlbmRzIHVwIHNlbmRpbmcg
YSBHRVRBVFRSLg0KPiBDb3VsZCBpdCBiZSB0aGF0IHdoYXQncyBoYXBwZW5pbmcgaXMgdGhhdCBH
RVRBVFRScyByZXBseSBmb3IgdGhlDQo+IGNoYW5nZV9hdHRyIGlzIGRpZmZlcmVudCB0aGFuIHdo
YXQgd2UgaGF2ZSBzdG9yZWQuIEJVVCB0aGF0J3MNCj4gb2J2aW91cywNCj4gd2Uga25ldyB0aGF0
IHRvIGJlZ2luIHdpdGggc2luY2Ugd2UgYXJlIHZhbGlkYXRpbmcgdGhlIGF0dHJpYnV0ZXMuIFNv
DQo+IHdlIHVwZGF0ZSBpdCBhbmQgdGhlbiB3ZSBzZW5kIGFub3RoZXIgR0VUQVRUUiBub3cgdGhl
IEdFVEFUVFIgc2VuZHMNCj4gYmFjayB0aGUgInNhbWUiIGNoYW5nZV9hdHRyIGFuZCB0aGlzIHRp
bWUgaXQgbWF0Y2hlcyB3aGF0IHdlIGhhdmUNCj4gc3RvcmVkLiBXaHkgaXMgdGhpcyAybmQgR0VU
QVRUUiBuZWNlc3Nhcnk/IFRoZSBhdHRyaWJ1dGUgc2hvdWxkIGJlDQo+IG1hcmtlZCBhcyB2YWxp
ZCBhZnRlciBhIGdldHRpbmcgYSBzdWNjZXNzZnVsIEdFVEFUVFIuDQo+IA0KT2guLi4gSSBzZWUg
d2hhdCB5b3UgYXJlIHNheWluZy4gWWVzLCB0aGF0IGNoZWNrIGxvb2tzIGxpa2UgaXQgc2hvdWxk
DQpiZSBsb29raW5nIGZvciBORlNfSU5PX0lOVkFMSURfT1RIRVIuDQoNCi0tIA0KVHJvbmQgTXlr
bGVidXN0DQpDVE8sIEhhbW1lcnNwYWNlIEluYw0KNDMwMCBFbCBDYW1pbm8gUmVhbCwgU3VpdGUg
MTA1DQpMb3MgQWx0b3MsIENBIDk0MDIyDQp3d3cuaGFtbWVyLnNwYWNlDQo=
On Fri, Jul 20, 2018 at 2:46 PM, Trond Myklebust
<[email protected]> wrote:
> On Fri, 2018-07-20 at 14:40 -0400, Olga Kornievskaia wrote:
>> On Fri, Jul 20, 2018 at 2:17 PM, Olga Kornievskaia <[email protected]>
>> wrote:
>> > On Fri, Jul 20, 2018 at 2:05 PM, Trond Myklebust
>> > <[email protected]> wrote:
>> > > On Fri, 2018-07-20 at 13:26 -0400, Olga Kornievskaia wrote:
>> > > > Hi Trond,
>> > > >
>> > > > I would some help understanding attributes management.
>> > > >
>> > > > Right now, any time a directory inode that was marked with
>> > > > INVALID_ACCESS (say to a change_attribute changed) ends up
>> > > > triggering
>> > > > sending a duplicate GETATTR. I don't think that's correct.
>> > > >
>> > > > In nfs_execute_ok() we check if (nfs_check_cache_invalid(inode,
>> > > > NFS_INO_INVALID_ACCESS)) and the call __nfs_revalidate_inode()
>> > > > which
>> > > > will trigger GETATTR. I don't understand why after calling this
>> > > > function the INVALID_ACCESS doesn't get cleared? Because that's
>> > > > what
>> > > > causes the double GETATTR to be sent.
>> > > >
>> > > > On the open path, the first time the getattr is sent is in
>> > > >
>> > > > Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>> > > > Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110
>> > > > [nfsv4]
>> > > > Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370
>> > > > [nfs]
>> > > > Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
>> > > > Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>> > > > Jul 19 15:39:52 ipa18 kernel: link_path_walk+0x29d/0x520
>> > > > Jul 19 15:39:52 ipa18 kernel: path_openat+0xf6/0x1230
>> > > > Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>> > > > Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>> > > > Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>> > > > Jul 19 15:39:52 ipa18 kernel:
>> > > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> > > >
>> > > > And then again during the open
>> > > >
>> > > > Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>> > > > Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110
>> > > > [nfsv4]
>> > > > Jul 19 15:39:52 ipa18 kernel: __nfs_revalidate_inode+0xe1/0x370
>> > > > [nfs]
>> > > > Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0 [nfs]
>> > > > Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>> > > > Jul 19 15:39:52 ipa18 kernel: path_openat+0x942/0x1230
>> > > > Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>> > > > Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>> > > > Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>> > > > Jul 19 15:39:52 ipa18 kernel:
>> > > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> > > >
>> > > > Can't we remove INVALID_ACCESS after we revalidated the inode?
>> > > > This
>> > > > removes the duplicated GETATTRs. Is this a valid way of fixing
>> > > > this
>> > > > issue?
>> > > >
>> > > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> > > > index 8f8e9e9..2b55a45 100644
>> > > > --- a/fs/nfs/dir.c
>> > > > +++ b/fs/nfs/dir.c
>> > > > @@ -2504,6 +2504,8 @@ static int nfs_execute_ok(struct inode
>> > > > *inode,
>> > > > int mask)
>> > > > if (mask & MAY_NOT_BLOCK)
>> > > > return -ECHILD;
>> > > > ret = __nfs_revalidate_inode(server, inode);
>> > > > + if (!ret)
>> > > > + NFS_I(inode)->cache_validity &=
>> > > > ~NFS_INO_INVALID_ACCESS;
>> > > > }
>> > > > if (ret == 0 && !execute_ok(inode))
>> > > > ret = -EACCES;
>> > >
>> > >
>> > > I don't see how the above makes sense.
>> > >
>> > > Either the attribute revalidation confirmed that the change attr,
>> > > mode
>> > > + uid + gid are unchanged, in which case the call to
>> > > nfs_refresh_inode() during revalidation will fail to set
>> > > NFS_INO_INVALID_ACCESS, or it confirmed that at least one of them
>> > > has
>> > > changed, in which case we do want to revalidate the access cache.
>> >
>> > I'm confused as to against what are we checking then?
>> >
>> > We flagged a directory inode to have invalid attributes. So we sent
>> > a
>> > GETATTR. I would think that after getting the result all our
>> > attributes should be valid. Why would a client as the next
>> > operation
>> > send another GETATTR for the same inode?
>> >
>>
>> I didn't answer your question but rather explained what I see client
>> do.
>>
>> Let's me see if I can try again:
>> in nfs_execute_ok() the check for the INVALID_CACHE is true so the
>> code calls __nfs_revalidate_inode() which ends up sending a GETATTR.
>> Could it be that what's happening is that GETATTRs reply for the
>> change_attr is different than what we have stored. BUT that's
>> obvious,
>> we knew that to begin with since we are validating the attributes. So
>> we update it and then we send another GETATTR now the GETATTR sends
>> back the "same" change_attr and this time it matches what we have
>> stored. Why is this 2nd GETATTR necessary? The attribute should be
>> marked as valid after a getting a successful GETATTR.
>>
> Oh... I see what you are saying. Yes, that check looks like it should
> be looking for NFS_INO_INVALID_OTHER.
Whew. ok I'm glad something make sense. However, you lost me on
"NFS_INO_INVALID_OTHER". What does that flag means?
Are you talking about checking the check in nfs_execute_ok() to check
for INVALID_OTHER instead of INVALID_ACCESS?
>
> --
> Trond Myklebust
> CTO, Hammerspace Inc
> 4300 El Camino Real, Suite 105
> Los Altos, CA 94022
> http://www.hammer.space
T24gRnJpLCAyMDE4LTA3LTIwIGF0IDE0OjU4IC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90
ZToNCj4gT24gRnJpLCBKdWwgMjAsIDIwMTggYXQgMjo0NiBQTSwgVHJvbmQgTXlrbGVidXN0DQo+
IDx0cm9uZG15QGhhbW1lcnNwYWNlLmNvbT4gd3JvdGU6DQo+ID4gT24gRnJpLCAyMDE4LTA3LTIw
IGF0IDE0OjQwIC0wNDAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90ZToNCj4gPiA+IE9uIEZyaSwg
SnVsIDIwLCAyMDE4IGF0IDI6MTcgUE0sIE9sZ2EgS29ybmlldnNrYWlhIDxhZ2xvQHVtaWNoLmVk
DQo+ID4gPiB1Pg0KPiA+ID4gd3JvdGU6DQo+ID4gPiA+IE9uIEZyaSwgSnVsIDIwLCAyMDE4IGF0
IDI6MDUgUE0sIFRyb25kIE15a2xlYnVzdA0KPiA+ID4gPiA8dHJvbmRteUBoYW1tZXJzcGFjZS5j
b20+IHdyb3RlOg0KPiA+ID4gPiA+IE9uIEZyaSwgMjAxOC0wNy0yMCBhdCAxMzoyNiAtMDQwMCwg
T2xnYSBLb3JuaWV2c2thaWEgd3JvdGU6DQo+ID4gPiA+ID4gPiBIaSBUcm9uZCwNCj4gPiA+ID4g
PiA+IA0KPiA+ID4gPiA+ID4gSSB3b3VsZCBzb21lIGhlbHAgdW5kZXJzdGFuZGluZyBhdHRyaWJ1
dGVzIG1hbmFnZW1lbnQuDQo+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+IFJpZ2h0IG5vdywgYW55
IHRpbWUgYSBkaXJlY3RvcnkgaW5vZGUgdGhhdCB3YXMgbWFya2VkIHdpdGgNCj4gPiA+ID4gPiA+
IElOVkFMSURfQUNDRVNTIChzYXkgdG8gYSBjaGFuZ2VfYXR0cmlidXRlIGNoYW5nZWQpIGVuZHMg
dXANCj4gPiA+ID4gPiA+IHRyaWdnZXJpbmcNCj4gPiA+ID4gPiA+IHNlbmRpbmcgYSBkdXBsaWNh
dGUgR0VUQVRUUi4gSSBkb24ndCB0aGluayB0aGF0J3MgY29ycmVjdC4NCj4gPiA+ID4gPiA+IA0K
PiA+ID4gPiA+ID4gSW4gbmZzX2V4ZWN1dGVfb2soKSB3ZSBjaGVjayBpZg0KPiA+ID4gPiA+ID4g
KG5mc19jaGVja19jYWNoZV9pbnZhbGlkKGlub2RlLA0KPiA+ID4gPiA+ID4gTkZTX0lOT19JTlZB
TElEX0FDQ0VTUykpIGFuZCB0aGUgY2FsbA0KPiA+ID4gPiA+ID4gX19uZnNfcmV2YWxpZGF0ZV9p
bm9kZSgpDQo+ID4gPiA+ID4gPiB3aGljaA0KPiA+ID4gPiA+ID4gd2lsbCB0cmlnZ2VyIEdFVEFU
VFIuIEkgZG9uJ3QgdW5kZXJzdGFuZCB3aHkgYWZ0ZXIgY2FsbGluZw0KPiA+ID4gPiA+ID4gdGhp
cw0KPiA+ID4gPiA+ID4gZnVuY3Rpb24gdGhlIElOVkFMSURfQUNDRVNTIGRvZXNuJ3QgZ2V0IGNs
ZWFyZWQ/IEJlY2F1c2UNCj4gPiA+ID4gPiA+IHRoYXQncw0KPiA+ID4gPiA+ID4gd2hhdA0KPiA+
ID4gPiA+ID4gY2F1c2VzIHRoZSBkb3VibGUgR0VUQVRUUiB0byBiZSBzZW50Lg0KPiA+ID4gPiA+
ID4gDQo+ID4gPiA+ID4gPiBPbiB0aGUgb3BlbiBwYXRoLCB0aGUgZmlyc3QgdGltZSB0aGUgZ2V0
YXR0ciBpcyBzZW50IGlzIGluDQo+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+IEp1bCAxOSAxNToz
OTo1MiBpcGExOCBrZXJuZWw6IGR1bXBfc3RhY2srMHg1YS8weDczDQo+ID4gPiA+ID4gPiBKdWwg
MTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBuZnM0X3Byb2NfZ2V0YXR0cisweDY1LzB4MTEwDQo+
ID4gPiA+ID4gPiBbbmZzdjRdDQo+ID4gPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2Vy
bmVsOg0KPiA+ID4gPiA+ID4gX19uZnNfcmV2YWxpZGF0ZV9pbm9kZSsweGUxLzB4MzcwDQo+ID4g
PiA+ID4gPiBbbmZzXQ0KPiA+ID4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDog
bmZzX3Blcm1pc3Npb24rMHgxNmIvMHgxZjANCj4gPiA+ID4gPiA+IFtuZnNdDQo+ID4gPiA+ID4g
PiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBpbm9kZV9wZXJtaXNzaW9uKzB4YWIvMHgx
MzANCj4gPiA+ID4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6IGxpbmtfcGF0aF93
YWxrKzB4MjlkLzB4NTIwDQo+ID4gPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVs
OiBwYXRoX29wZW5hdCsweGY2LzB4MTIzMA0KPiA+ID4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlw
YTE4IGtlcm5lbDogZG9fZmlscF9vcGVuKzB4OTEvMHgxMDANCj4gPiA+ID4gPiA+IEp1bCAxOSAx
NTozOTo1MiBpcGExOCBrZXJuZWw6IGRvX3N5c19vcGVuKzB4MTI2LzB4MjEwDQo+ID4gPiA+ID4g
PiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBkb19zeXNjYWxsXzY0KzB4NTUvMHgxODAN
Cj4gPiA+ID4gPiA+IEp1bCAxOSAxNTozOTo1MiBpcGExOCBrZXJuZWw6DQo+ID4gPiA+ID4gPiBl
bnRyeV9TWVNDQUxMXzY0X2FmdGVyX2h3ZnJhbWUrMHg0NC8weGE5DQo+ID4gPiA+ID4gPiANCj4g
PiA+ID4gPiA+IEFuZCB0aGVuIGFnYWluIGR1cmluZyB0aGUgb3Blbg0KPiA+ID4gPiA+ID4gDQo+
ID4gPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBkdW1wX3N0YWNrKzB4NWEv
MHg3Mw0KPiA+ID4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogbmZzNF9wcm9j
X2dldGF0dHIrMHg2NS8weDExMA0KPiA+ID4gPiA+ID4gW25mc3Y0XQ0KPiA+ID4gPiA+ID4gSnVs
IDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDoNCj4gPiA+ID4gPiA+IF9fbmZzX3JldmFsaWRhdGVf
aW5vZGUrMHhlMS8weDM3MA0KPiA+ID4gPiA+ID4gW25mc10NCj4gPiA+ID4gPiA+IEp1bCAxOSAx
NTozOTo1MiBpcGExOCBrZXJuZWw6IG5mc19wZXJtaXNzaW9uKzB4MTZiLzB4MWYwDQo+ID4gPiA+
ID4gPiBbbmZzXQ0KPiA+ID4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogaW5v
ZGVfcGVybWlzc2lvbisweGFiLzB4MTMwDQo+ID4gPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBh
MTgga2VybmVsOiBwYXRoX29wZW5hdCsweDk0Mi8weDEyMzANCj4gPiA+ID4gPiA+IEp1bCAxOSAx
NTozOTo1MiBpcGExOCBrZXJuZWw6IGRvX2ZpbHBfb3BlbisweDkxLzB4MTAwDQo+ID4gPiA+ID4g
PiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOiBkb19zeXNfb3BlbisweDEyNi8weDIxMA0K
PiA+ID4gPiA+ID4gSnVsIDE5IDE1OjM5OjUyIGlwYTE4IGtlcm5lbDogZG9fc3lzY2FsbF82NCsw
eDU1LzB4MTgwDQo+ID4gPiA+ID4gPiBKdWwgMTkgMTU6Mzk6NTIgaXBhMTgga2VybmVsOg0KPiA+
ID4gPiA+ID4gZW50cnlfU1lTQ0FMTF82NF9hZnRlcl9od2ZyYW1lKzB4NDQvMHhhOQ0KPiA+ID4g
PiA+ID4gDQo+ID4gPiA+ID4gPiBDYW4ndCB3ZSByZW1vdmUgSU5WQUxJRF9BQ0NFU1MgYWZ0ZXIg
d2UgcmV2YWxpZGF0ZWQgdGhlDQo+ID4gPiA+ID4gPiBpbm9kZT8NCj4gPiA+ID4gPiA+IFRoaXMN
Cj4gPiA+ID4gPiA+IHJlbW92ZXMgdGhlIGR1cGxpY2F0ZWQgR0VUQVRUUnMuIElzIHRoaXMgYSB2
YWxpZCB3YXkgb2YNCj4gPiA+ID4gPiA+IGZpeGluZw0KPiA+ID4gPiA+ID4gdGhpcw0KPiA+ID4g
PiA+ID4gaXNzdWU/DQo+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+IGRpZmYgLS1naXQgYS9mcy9u
ZnMvZGlyLmMgYi9mcy9uZnMvZGlyLmMNCj4gPiA+ID4gPiA+IGluZGV4IDhmOGU5ZTkuLjJiNTVh
NDUgMTAwNjQ0DQo+ID4gPiA+ID4gPiAtLS0gYS9mcy9uZnMvZGlyLmMNCj4gPiA+ID4gPiA+ICsr
KyBiL2ZzL25mcy9kaXIuYw0KPiA+ID4gPiA+ID4gQEAgLTI1MDQsNiArMjUwNCw4IEBAIHN0YXRp
YyBpbnQgbmZzX2V4ZWN1dGVfb2soc3RydWN0DQo+ID4gPiA+ID4gPiBpbm9kZQ0KPiA+ID4gPiA+
ID4gKmlub2RlLA0KPiA+ID4gPiA+ID4gaW50IG1hc2spDQo+ID4gPiA+ID4gPiAgICAgICAgICAg
ICAgICAgaWYgKG1hc2sgJiBNQVlfTk9UX0JMT0NLKQ0KPiA+ID4gPiA+ID4gICAgICAgICAgICAg
ICAgICAgICAgICAgcmV0dXJuIC1FQ0hJTEQ7DQo+ID4gPiA+ID4gPiAgICAgICAgICAgICAgICAg
cmV0ID0gX19uZnNfcmV2YWxpZGF0ZV9pbm9kZShzZXJ2ZXIsDQo+ID4gPiA+ID4gPiBpbm9kZSk7
DQo+ID4gPiA+ID4gPiArICAgICAgICAgICAgICAgaWYgKCFyZXQpDQo+ID4gPiA+ID4gPiArICAg
ICAgICAgICAgICAgICAgICAgICBORlNfSShpbm9kZSktPmNhY2hlX3ZhbGlkaXR5ICY9DQo+ID4g
PiA+ID4gPiB+TkZTX0lOT19JTlZBTElEX0FDQ0VTUzsNCj4gPiA+ID4gPiA+ICAgICAgICAgfQ0K
PiA+ID4gPiA+ID4gICAgICAgICBpZiAocmV0ID09IDAgJiYgIWV4ZWN1dGVfb2soaW5vZGUpKQ0K
PiA+ID4gPiA+ID4gICAgICAgICAgICAgICAgIHJldCA9IC1FQUNDRVM7DQo+ID4gPiA+ID4gDQo+
ID4gPiA+ID4gDQo+ID4gPiA+ID4gSSBkb24ndCBzZWUgaG93IHRoZSBhYm92ZSBtYWtlcyBzZW5z
ZS4NCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBFaXRoZXIgdGhlIGF0dHJpYnV0ZSByZXZhbGlkYXRp
b24gY29uZmlybWVkIHRoYXQgdGhlIGNoYW5nZQ0KPiA+ID4gPiA+IGF0dHIsDQo+ID4gPiA+ID4g
bW9kZQ0KPiA+ID4gPiA+ICsgdWlkICsgZ2lkIGFyZSB1bmNoYW5nZWQsIGluIHdoaWNoIGNhc2Ug
dGhlIGNhbGwgdG8NCj4gPiA+ID4gPiBuZnNfcmVmcmVzaF9pbm9kZSgpIGR1cmluZyByZXZhbGlk
YXRpb24gd2lsbCBmYWlsIHRvIHNldA0KPiA+ID4gPiA+IE5GU19JTk9fSU5WQUxJRF9BQ0NFU1Ms
IG9yIGl0IGNvbmZpcm1lZCB0aGF0IGF0IGxlYXN0IG9uZSBvZg0KPiA+ID4gPiA+IHRoZW0NCj4g
PiA+ID4gPiBoYXMNCj4gPiA+ID4gPiBjaGFuZ2VkLCBpbiB3aGljaCBjYXNlIHdlIGRvIHdhbnQg
dG8gcmV2YWxpZGF0ZSB0aGUgYWNjZXNzDQo+ID4gPiA+ID4gY2FjaGUuDQo+ID4gPiA+IA0KPiA+
ID4gPiBJJ20gY29uZnVzZWQgYXMgdG8gYWdhaW5zdCB3aGF0IGFyZSB3ZSBjaGVja2luZyB0aGVu
Pw0KPiA+ID4gPiANCj4gPiA+ID4gV2UgZmxhZ2dlZCBhIGRpcmVjdG9yeSBpbm9kZSB0byBoYXZl
IGludmFsaWQgYXR0cmlidXRlcy4gU28gd2UNCj4gPiA+ID4gc2VudA0KPiA+ID4gPiBhDQo+ID4g
PiA+IEdFVEFUVFIuIEkgd291bGQgdGhpbmsgdGhhdCBhZnRlciBnZXR0aW5nIHRoZSByZXN1bHQg
YWxsIG91cg0KPiA+ID4gPiBhdHRyaWJ1dGVzIHNob3VsZCBiZSB2YWxpZC4gV2h5IHdvdWxkIGEg
Y2xpZW50IGFzIHRoZSBuZXh0DQo+ID4gPiA+IG9wZXJhdGlvbg0KPiA+ID4gPiBzZW5kIGFub3Ro
ZXIgR0VUQVRUUiBmb3IgdGhlIHNhbWUgaW5vZGU/DQo+ID4gPiA+IA0KPiA+ID4gDQo+ID4gPiBJ
IGRpZG4ndCBhbnN3ZXIgeW91ciBxdWVzdGlvbiBidXQgcmF0aGVyIGV4cGxhaW5lZCB3aGF0IEkg
c2VlDQo+ID4gPiBjbGllbnQNCj4gPiA+IGRvLg0KPiA+ID4gDQo+ID4gPiBMZXQncyBtZSBzZWUg
aWYgSSBjYW4gdHJ5IGFnYWluOg0KPiA+ID4gaW4gbmZzX2V4ZWN1dGVfb2soKSB0aGUgY2hlY2sg
Zm9yIHRoZSBJTlZBTElEX0NBQ0hFIGlzIHRydWUgc28NCj4gPiA+IHRoZQ0KPiA+ID4gY29kZSBj
YWxscyBfX25mc19yZXZhbGlkYXRlX2lub2RlKCkgd2hpY2ggZW5kcyB1cCBzZW5kaW5nIGENCj4g
PiA+IEdFVEFUVFIuDQo+ID4gPiBDb3VsZCBpdCBiZSB0aGF0IHdoYXQncyBoYXBwZW5pbmcgaXMg
dGhhdCBHRVRBVFRScyByZXBseSBmb3IgdGhlDQo+ID4gPiBjaGFuZ2VfYXR0ciBpcyBkaWZmZXJl
bnQgdGhhbiB3aGF0IHdlIGhhdmUgc3RvcmVkLiBCVVQgdGhhdCdzDQo+ID4gPiBvYnZpb3VzLA0K
PiA+ID4gd2Uga25ldyB0aGF0IHRvIGJlZ2luIHdpdGggc2luY2Ugd2UgYXJlIHZhbGlkYXRpbmcg
dGhlDQo+ID4gPiBhdHRyaWJ1dGVzLiBTbw0KPiA+ID4gd2UgdXBkYXRlIGl0IGFuZCB0aGVuIHdl
IHNlbmQgYW5vdGhlciBHRVRBVFRSIG5vdyB0aGUgR0VUQVRUUg0KPiA+ID4gc2VuZHMNCj4gPiA+
IGJhY2sgdGhlICJzYW1lIiBjaGFuZ2VfYXR0ciBhbmQgdGhpcyB0aW1lIGl0IG1hdGNoZXMgd2hh
dCB3ZSBoYXZlDQo+ID4gPiBzdG9yZWQuIFdoeSBpcyB0aGlzIDJuZCBHRVRBVFRSIG5lY2Vzc2Fy
eT8gVGhlIGF0dHJpYnV0ZSBzaG91bGQNCj4gPiA+IGJlDQo+ID4gPiBtYXJrZWQgYXMgdmFsaWQg
YWZ0ZXIgYSBnZXR0aW5nIGEgc3VjY2Vzc2Z1bCBHRVRBVFRSLg0KPiA+ID4gDQo+ID4gDQo+ID4g
T2guLi4gSSBzZWUgd2hhdCB5b3UgYXJlIHNheWluZy4gWWVzLCB0aGF0IGNoZWNrIGxvb2tzIGxp
a2UgaXQNCj4gPiBzaG91bGQNCj4gPiBiZSBsb29raW5nIGZvciBORlNfSU5PX0lOVkFMSURfT1RI
RVIuDQo+IA0KPiBXaGV3LiBvayBJJ20gZ2xhZCBzb21ldGhpbmcgbWFrZSBzZW5zZS4gSG93ZXZl
ciwgeW91IGxvc3QgbWUgb24NCj4gIk5GU19JTk9fSU5WQUxJRF9PVEhFUiIuIFdoYXQgZG9lcyB0
aGF0IGZsYWcgbWVhbnM/DQo+IEFyZSB5b3UgdGFsa2luZyBhYm91dCBjaGVja2luZyB0aGUgY2hl
Y2sgaW4gbmZzX2V4ZWN1dGVfb2soKSB0byBjaGVjaw0KPiBmb3IgSU5WQUxJRF9PVEhFUiBpbnN0
ZWFkIG9mIElOVkFMSURfQUNDRVNTPw0KPiANCg0KSSdtIHNheWluZyB0aGF0IGZ1bmN0aW9uIHNo
b3VsZCBwcm9iYWJseSByZWFkIGFzOg0KDQpzdGF0aWMgaW50IG5mc19leGVjdXRlX29rKHN0cnVj
dCBpbm9kZSAqaW5vZGUsIGludCBtYXNrKQ0Kew0KICAgICAgICBzdHJ1Y3QgbmZzX3NlcnZlciAq
c2VydmVyID0gTkZTX1NFUlZFUihpbm9kZSk7DQogICAgICAgIGludCByZXQgPSAwOw0KDQogICAg
ICAgIGlmIChuZnNfY2hlY2tfY2FjaGVfaW52YWxpZChpbm9kZSwgTkZTX0lOT19JTlZBTElEX09U
SEVSKSkgew0KICAgICAgICAgICAgICAgIGlmIChtYXNrICYgTUFZX05PVF9CTE9DSykNCiAgICAg
ICAgICAgICAgICAgICAgICAgIHJldHVybiAtRUNISUxEOw0KICAgICAgICAgICAgICAgIHJldCA9
IF9fbmZzX3JldmFsaWRhdGVfaW5vZGUoc2VydmVyLCBpbm9kZSk7DQogICAgICAgIH0NCiAgICAg
ICAgaWYgKHJldCA9PSAwICYmICFleGVjdXRlX29rKGlub2RlKSkNCiAgICAgICAgICAgICAgICBy
ZXQgPSAtRUFDQ0VTOw0KICAgICAgICByZXR1cm4gcmV0Ow0KfQ0KDQotLSANClRyb25kIE15a2xl
YnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyLCBIYW1tZXJzcGFjZQ0KdHJvbmQubXlr
bGVidXN0QGhhbW1lcnNwYWNlLmNvbQ0KDQo=
On Fri, Jul 20, 2018 at 3:19 PM, Trond Myklebust
<[email protected]> wrote:
> On Fri, 2018-07-20 at 14:58 -0400, Olga Kornievskaia wrote:
>> On Fri, Jul 20, 2018 at 2:46 PM, Trond Myklebust
>> <[email protected]> wrote:
>> > On Fri, 2018-07-20 at 14:40 -0400, Olga Kornievskaia wrote:
>> > > On Fri, Jul 20, 2018 at 2:17 PM, Olga Kornievskaia <[email protected]
>> > > u>
>> > > wrote:
>> > > > On Fri, Jul 20, 2018 at 2:05 PM, Trond Myklebust
>> > > > <[email protected]> wrote:
>> > > > > On Fri, 2018-07-20 at 13:26 -0400, Olga Kornievskaia wrote:
>> > > > > > Hi Trond,
>> > > > > >
>> > > > > > I would some help understanding attributes management.
>> > > > > >
>> > > > > > Right now, any time a directory inode that was marked with
>> > > > > > INVALID_ACCESS (say to a change_attribute changed) ends up
>> > > > > > triggering
>> > > > > > sending a duplicate GETATTR. I don't think that's correct.
>> > > > > >
>> > > > > > In nfs_execute_ok() we check if
>> > > > > > (nfs_check_cache_invalid(inode,
>> > > > > > NFS_INO_INVALID_ACCESS)) and the call
>> > > > > > __nfs_revalidate_inode()
>> > > > > > which
>> > > > > > will trigger GETATTR. I don't understand why after calling
>> > > > > > this
>> > > > > > function the INVALID_ACCESS doesn't get cleared? Because
>> > > > > > that's
>> > > > > > what
>> > > > > > causes the double GETATTR to be sent.
>> > > > > >
>> > > > > > On the open path, the first time the getattr is sent is in
>> > > > > >
>> > > > > > Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>> > > > > > Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110
>> > > > > > [nfsv4]
>> > > > > > Jul 19 15:39:52 ipa18 kernel:
>> > > > > > __nfs_revalidate_inode+0xe1/0x370
>> > > > > > [nfs]
>> > > > > > Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0
>> > > > > > [nfs]
>> > > > > > Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>> > > > > > Jul 19 15:39:52 ipa18 kernel: link_path_walk+0x29d/0x520
>> > > > > > Jul 19 15:39:52 ipa18 kernel: path_openat+0xf6/0x1230
>> > > > > > Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>> > > > > > Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>> > > > > > Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>> > > > > > Jul 19 15:39:52 ipa18 kernel:
>> > > > > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> > > > > >
>> > > > > > And then again during the open
>> > > > > >
>> > > > > > Jul 19 15:39:52 ipa18 kernel: dump_stack+0x5a/0x73
>> > > > > > Jul 19 15:39:52 ipa18 kernel: nfs4_proc_getattr+0x65/0x110
>> > > > > > [nfsv4]
>> > > > > > Jul 19 15:39:52 ipa18 kernel:
>> > > > > > __nfs_revalidate_inode+0xe1/0x370
>> > > > > > [nfs]
>> > > > > > Jul 19 15:39:52 ipa18 kernel: nfs_permission+0x16b/0x1f0
>> > > > > > [nfs]
>> > > > > > Jul 19 15:39:52 ipa18 kernel: inode_permission+0xab/0x130
>> > > > > > Jul 19 15:39:52 ipa18 kernel: path_openat+0x942/0x1230
>> > > > > > Jul 19 15:39:52 ipa18 kernel: do_filp_open+0x91/0x100
>> > > > > > Jul 19 15:39:52 ipa18 kernel: do_sys_open+0x126/0x210
>> > > > > > Jul 19 15:39:52 ipa18 kernel: do_syscall_64+0x55/0x180
>> > > > > > Jul 19 15:39:52 ipa18 kernel:
>> > > > > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> > > > > >
>> > > > > > Can't we remove INVALID_ACCESS after we revalidated the
>> > > > > > inode?
>> > > > > > This
>> > > > > > removes the duplicated GETATTRs. Is this a valid way of
>> > > > > > fixing
>> > > > > > this
>> > > > > > issue?
>> > > > > >
>> > > > > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> > > > > > index 8f8e9e9..2b55a45 100644
>> > > > > > --- a/fs/nfs/dir.c
>> > > > > > +++ b/fs/nfs/dir.c
>> > > > > > @@ -2504,6 +2504,8 @@ static int nfs_execute_ok(struct
>> > > > > > inode
>> > > > > > *inode,
>> > > > > > int mask)
>> > > > > > if (mask & MAY_NOT_BLOCK)
>> > > > > > return -ECHILD;
>> > > > > > ret = __nfs_revalidate_inode(server,
>> > > > > > inode);
>> > > > > > + if (!ret)
>> > > > > > + NFS_I(inode)->cache_validity &=
>> > > > > > ~NFS_INO_INVALID_ACCESS;
>> > > > > > }
>> > > > > > if (ret == 0 && !execute_ok(inode))
>> > > > > > ret = -EACCES;
>> > > > >
>> > > > >
>> > > > > I don't see how the above makes sense.
>> > > > >
>> > > > > Either the attribute revalidation confirmed that the change
>> > > > > attr,
>> > > > > mode
>> > > > > + uid + gid are unchanged, in which case the call to
>> > > > > nfs_refresh_inode() during revalidation will fail to set
>> > > > > NFS_INO_INVALID_ACCESS, or it confirmed that at least one of
>> > > > > them
>> > > > > has
>> > > > > changed, in which case we do want to revalidate the access
>> > > > > cache.
>> > > >
>> > > > I'm confused as to against what are we checking then?
>> > > >
>> > > > We flagged a directory inode to have invalid attributes. So we
>> > > > sent
>> > > > a
>> > > > GETATTR. I would think that after getting the result all our
>> > > > attributes should be valid. Why would a client as the next
>> > > > operation
>> > > > send another GETATTR for the same inode?
>> > > >
>> > >
>> > > I didn't answer your question but rather explained what I see
>> > > client
>> > > do.
>> > >
>> > > Let's me see if I can try again:
>> > > in nfs_execute_ok() the check for the INVALID_CACHE is true so
>> > > the
>> > > code calls __nfs_revalidate_inode() which ends up sending a
>> > > GETATTR.
>> > > Could it be that what's happening is that GETATTRs reply for the
>> > > change_attr is different than what we have stored. BUT that's
>> > > obvious,
>> > > we knew that to begin with since we are validating the
>> > > attributes. So
>> > > we update it and then we send another GETATTR now the GETATTR
>> > > sends
>> > > back the "same" change_attr and this time it matches what we have
>> > > stored. Why is this 2nd GETATTR necessary? The attribute should
>> > > be
>> > > marked as valid after a getting a successful GETATTR.
>> > >
>> >
>> > Oh... I see what you are saying. Yes, that check looks like it
>> > should
>> > be looking for NFS_INO_INVALID_OTHER.
>>
>> Whew. ok I'm glad something make sense. However, you lost me on
>> "NFS_INO_INVALID_OTHER". What does that flag means?
>> Are you talking about checking the check in nfs_execute_ok() to check
>> for INVALID_OTHER instead of INVALID_ACCESS?
>>
>
> I'm saying that function should probably read as:
>
> static int nfs_execute_ok(struct inode *inode, int mask)
> {
> struct nfs_server *server = NFS_SERVER(inode);
> int ret = 0;
>
> if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_OTHER)) {
> if (mask & MAY_NOT_BLOCK)
> return -ECHILD;
> ret = __nfs_revalidate_inode(server, inode);
> }
> if (ret == 0 && !execute_ok(inode))
> ret = -EACCES;
> return ret;
> }
>
With that change I also don't see a 2nd GETATTR sent. Would you mind
creating a patch for it?