2015-06-12 18:30:58

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 1/1] Recover from stateid-type error on SETATTR

Client can receives stateid-type error (eg., BAD_STATEID) on SETATTR when
delegation stateid was used. When no open state exists, in case of application
calling truncate() on the file, client has no state to recover and fails with
EIO.

Instead, upon such error, return the bad delegation and then resend the
SETATTR with a zero stateid.

Signed-off: Olga Kornievskaia <[email protected]>
---
fs/nfs/delegation.h | 6 ++++++
fs/nfs/nfs4proc.c | 7 ++++++-
2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index e3c20a3..e37165f 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -62,6 +62,12 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
int nfs4_have_delegation(struct inode *inode, fmode_t flags);
int nfs4_check_delegation(struct inode *inode, fmode_t flags);

+static inline int nfs4_have_any_delegation(struct inode *inode)
+{
+ struct nfs_inode *nfsi = NFS_I(inode);
+ return rcu_access_pointer(nfsi->delegation) ? 1 : 0;
+}
+
#endif

static inline int nfs_have_delegated_attributes(struct inode *inode)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ad7cf7e..63b3581 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -360,8 +360,13 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
case -NFS4ERR_DELEG_REVOKED:
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_BAD_STATEID:
- if (state == NULL)
+ if (state == NULL) {
+ if (nfs4_have_any_delegation(inode)) {
+ nfs4_inode_return_delegation(inode);
+ exception->retry = 1;
+ }
break;
+ }
ret = nfs4_schedule_stateid_recovery(server, state);
if (ret < 0)
break;
--
1.8.3.1



2015-06-12 18:37:04

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/1] Recover from stateid-type error on SETATTR

Hi Olga,

On Fri, Jun 12, 2015 at 2:30 PM, Olga Kornievskaia <[email protected]> wrote:
> Client can receives stateid-type error (eg., BAD_STATEID) on SETATTR when
> delegation stateid was used. When no open state exists, in case of application
> calling truncate() on the file, client has no state to recover and fails with
> EIO.
>
> Instead, upon such error, return the bad delegation and then resend the
> SETATTR with a zero stateid.
>
> Signed-off: Olga Kornievskaia <[email protected]>
> ---
> fs/nfs/delegation.h | 6 ++++++
> fs/nfs/nfs4proc.c | 7 ++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
> index e3c20a3..e37165f 100644
> --- a/fs/nfs/delegation.h
> +++ b/fs/nfs/delegation.h
> @@ -62,6 +62,12 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
> int nfs4_have_delegation(struct inode *inode, fmode_t flags);
> int nfs4_check_delegation(struct inode *inode, fmode_t flags);
>
> +static inline int nfs4_have_any_delegation(struct inode *inode)
> +{
> + struct nfs_inode *nfsi = NFS_I(inode);
> + return rcu_access_pointer(nfsi->delegation) ? 1 : 0;
> +}

What would this do that isn't already covered by
nfs4_have_delegation(inode, FMODE_READ)?

Cheers
Trond

2015-06-12 18:48:38

by Kornievskaia, Olga

[permalink] [raw]
Subject: Re: [PATCH 1/1] Recover from stateid-type error on SETATTR

DQo+IE9uIEp1biAxMiwgMjAxNSwgYXQgMjozNyBQTSwgVHJvbmQgTXlrbGVidXN0IDx0cm9uZC5t
eWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tPiB3cm90ZToNCj4gDQo+IEhpIE9sZ2EsDQo+IA0KPiBP
biBGcmksIEp1biAxMiwgMjAxNSBhdCAyOjMwIFBNLCBPbGdhIEtvcm5pZXZza2FpYSA8a29sZ2FA
bmV0YXBwLmNvbT4gd3JvdGU6DQo+PiBDbGllbnQgY2FuIHJlY2VpdmVzIHN0YXRlaWQtdHlwZSBl
cnJvciAoZWcuLCBCQURfU1RBVEVJRCkgb24gU0VUQVRUUiB3aGVuDQo+PiBkZWxlZ2F0aW9uIHN0
YXRlaWQgd2FzIHVzZWQuIFdoZW4gbm8gb3BlbiBzdGF0ZSBleGlzdHMsIGluIGNhc2Ugb2YgYXBw
bGljYXRpb24NCj4+IGNhbGxpbmcgdHJ1bmNhdGUoKSBvbiB0aGUgZmlsZSwgY2xpZW50IGhhcyBu
byBzdGF0ZSB0byByZWNvdmVyIGFuZCBmYWlscyB3aXRoDQo+PiBFSU8uDQo+PiANCj4+IEluc3Rl
YWQsIHVwb24gc3VjaCBlcnJvciwgcmV0dXJuIHRoZSBiYWQgZGVsZWdhdGlvbiBhbmQgdGhlbiBy
ZXNlbmQgdGhlDQo+PiBTRVRBVFRSIHdpdGggYSB6ZXJvIHN0YXRlaWQuDQo+PiANCj4+IFNpZ25l
ZC1vZmY6IE9sZ2EgS29ybmlldnNrYWlhIDxrb2xnYUBuZXRhcHAuY29tPg0KPj4gLS0tDQo+PiBm
cy9uZnMvZGVsZWdhdGlvbi5oIHwgNiArKysrKysNCj4+IGZzL25mcy9uZnM0cHJvYy5jICAgfCA3
ICsrKysrKy0NCj4+IDIgZmlsZXMgY2hhbmdlZCwgMTIgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlv
bigtKQ0KPj4gDQo+PiBkaWZmIC0tZ2l0IGEvZnMvbmZzL2RlbGVnYXRpb24uaCBiL2ZzL25mcy9k
ZWxlZ2F0aW9uLmgNCj4+IGluZGV4IGUzYzIwYTMuLmUzNzE2NWYgMTAwNjQ0DQo+PiAtLS0gYS9m
cy9uZnMvZGVsZWdhdGlvbi5oDQo+PiArKysgYi9mcy9uZnMvZGVsZWdhdGlvbi5oDQo+PiBAQCAt
NjIsNiArNjIsMTIgQEAgdm9pZCBuZnNfbWFya19kZWxlZ2F0aW9uX3JlZmVyZW5jZWQoc3RydWN0
IG5mc19kZWxlZ2F0aW9uICpkZWxlZ2F0aW9uKTsNCj4+IGludCBuZnM0X2hhdmVfZGVsZWdhdGlv
bihzdHJ1Y3QgaW5vZGUgKmlub2RlLCBmbW9kZV90IGZsYWdzKTsNCj4+IGludCBuZnM0X2NoZWNr
X2RlbGVnYXRpb24oc3RydWN0IGlub2RlICppbm9kZSwgZm1vZGVfdCBmbGFncyk7DQo+PiANCj4+
ICtzdGF0aWMgaW5saW5lIGludCBuZnM0X2hhdmVfYW55X2RlbGVnYXRpb24oc3RydWN0IGlub2Rl
ICppbm9kZSkNCj4+ICt7DQo+PiArICAgICAgIHN0cnVjdCBuZnNfaW5vZGUgKm5mc2kgPSBORlNf
SShpbm9kZSk7DQo+PiArICAgICAgIHJldHVybiByY3VfYWNjZXNzX3BvaW50ZXIobmZzaS0+ZGVs
ZWdhdGlvbikgPyAxIDogMDsNCj4+ICt9DQo+IA0KPiBXaGF0IHdvdWxkIHRoaXMgZG8gdGhhdCBp
c24ndCBhbHJlYWR5IGNvdmVyZWQgYnkNCj4gbmZzNF9oYXZlX2RlbGVnYXRpb24oaW5vZGUsIEZN
T0RFX1JFQUQpPw0KDQpXZSBuZWVkIHRvIHJldHVybiBkZWxlZ2F0aW9uIHJlZ2FyZGxlc3Mgb2Yg
d2hldGhlciBpdOKAmXMgcmVhZCBvciB3cml0ZS4gDQoNCj4gDQo+IENoZWVycw0KPiAgVHJvbmQN
Cg0K

2015-06-12 18:56:37

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 1/1] Recover from stateid-type error on SETATTR

Hey Trond,

On 06/12/2015 02:37 PM, Trond Myklebust wrote:
> Hi Olga,
>
> On Fri, Jun 12, 2015 at 2:30 PM, Olga Kornievskaia <[email protected]> wrote:
>> Client can receives stateid-type error (eg., BAD_STATEID) on SETATTR when
>> delegation stateid was used. When no open state exists, in case of application
>> calling truncate() on the file, client has no state to recover and fails with
>> EIO.
>>
>> Instead, upon such error, return the bad delegation and then resend the
>> SETATTR with a zero stateid.
>>
>> Signed-off: Olga Kornievskaia <[email protected]>
>> ---
>> fs/nfs/delegation.h | 6 ++++++
>> fs/nfs/nfs4proc.c | 7 ++++++-
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
>> index e3c20a3..e37165f 100644
>> --- a/fs/nfs/delegation.h
>> +++ b/fs/nfs/delegation.h
>> @@ -62,6 +62,12 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
>> int nfs4_have_delegation(struct inode *inode, fmode_t flags);
>> int nfs4_check_delegation(struct inode *inode, fmode_t flags);
>>
>> +static inline int nfs4_have_any_delegation(struct inode *inode)
>> +{
>> + struct nfs_inode *nfsi = NFS_I(inode);
>> + return rcu_access_pointer(nfsi->delegation) ? 1 : 0;
>> +}
>
> What would this do that isn't already covered by
> nfs4_have_delegation(inode, FMODE_READ)?

It looks like _nfs4_do_settattr() can select between FMODE_READ and FMODE_WRITE, so wouldn't we otherwise need to call nfs4_have_delegation() twice? Or am I missing something?

Anna

>
> Cheers
> Trond
>


2015-06-12 20:02:53

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/1] Recover from stateid-type error on SETATTR

On Fri, Jun 12, 2015 at 2:48 PM, Kornievskaia, Olga
<[email protected]> wrote:
>
>> On Jun 12, 2015, at 2:37 PM, Trond Myklebust <[email protected]> wrote:
>>
>> Hi Olga,
>>
>> On Fri, Jun 12, 2015 at 2:30 PM, Olga Kornievskaia <[email protected]> wrote:
>>> Client can receives stateid-type error (eg., BAD_STATEID) on SETATTR when
>>> delegation stateid was used. When no open state exists, in case of application
>>> calling truncate() on the file, client has no state to recover and fails with
>>> EIO.
>>>
>>> Instead, upon such error, return the bad delegation and then resend the
>>> SETATTR with a zero stateid.
>>>
>>> Signed-off: Olga Kornievskaia <[email protected]>
>>> ---
>>> fs/nfs/delegation.h | 6 ++++++
>>> fs/nfs/nfs4proc.c | 7 ++++++-
>>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
>>> index e3c20a3..e37165f 100644
>>> --- a/fs/nfs/delegation.h
>>> +++ b/fs/nfs/delegation.h
>>> @@ -62,6 +62,12 @@ void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
>>> int nfs4_have_delegation(struct inode *inode, fmode_t flags);
>>> int nfs4_check_delegation(struct inode *inode, fmode_t flags);
>>>
>>> +static inline int nfs4_have_any_delegation(struct inode *inode)
>>> +{
>>> + struct nfs_inode *nfsi = NFS_I(inode);
>>> + return rcu_access_pointer(nfsi->delegation) ? 1 : 0;
>>> +}
>>
>> What would this do that isn't already covered by
>> nfs4_have_delegation(inode, FMODE_READ)?
>
> We need to return delegation regardless of whether it’s read or write.

Yes, and the test for nfs4_have_delegation(inode, FMODE_READ) is
intentionally satisfied when you hold a write delegation, because it
guarantees a superset of the caching guarantees that a read delegation
would offer.

Cheers
Trond