2018-02-27 17:09:05

by Olga Kornievskaia

Subject: Looping on ERR_OLD_STATEID on LOCK

Hi folks,

I'd like to understand why nfs4_do_handle_exception() for
ERR_OLD_STATEID just retries the operation. What happens is that the
client resends that same operation with the same stateid over to the
server and gets the same ERR_OLD_STATEID back which puts the client in
an infinite loop (and a hung state).

Shouldn't the client instead treat ERR_OLD_STATEID same as ERR_BAD_STATEID?

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c8b554a..2e006bc 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -411,6 +411,7 @@ static int nfs4_do_handle_exception(struct
nfs_server *server,
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_EXPIRED:
case -NFS4ERR_BAD_STATEID:
+ case -NFS4ERR_OLD_STATEID:
if (inode != NULL && stateid != NULL) {
nfs_inode_find_state_and_recover(inode,
stateid);
@@ -477,7 +478,6 @@ static int nfs4_do_handle_exception(struct
nfs_server *server,
return 0;

case -NFS4ERR_RETRY_UNCACHED_REP:
- case -NFS4ERR_OLD_STATEID:
exception->retry = 1;
break;
case -NFS4ERR_BADOWNER:

Or alternative, for the LOCK it should be:
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c8b554a..ce846faf 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6441,6 +6441,7 @@ static void nfs4_handle_setlk_error(struct
nfs_server *server, struct nfs4_lock_
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_EXPIRED:
case -NFS4ERR_BAD_STATEID:
+ case -NFS4ERR_OLD_STATEID:
lsp->ls_seqid.flags &= ~NFS_SEQID_CONFIRMED;
if (new_lock_owner != 0 ||
test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) != 0)

2018-02-28 14:04:02

by Trond Myklebust

Subject: Re: Looping on ERR_OLD_STATEID on LOCK

T24gVHVlLCAyMDE4LTAyLTI3IGF0IDEyOjA5IC0wNTAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90
ZToNCj4gSGkgZm9sa3MsDQo+IA0KPiBJJ2QgbGlrZSB0byB1bmRlcnN0YW5kIHdoeSBuZnM0X2Rv
X2hhbmRsZV9leGNlcHRpb24oKSBmb3INCj4gRVJSX09MRF9TVEFURUlEIGp1c3QgcmV0cmllcyB0
aGUgb3BlcmF0aW9uLiBXaGF0IGhhcHBlbnMgaXMgdGhhdCB0aGUNCj4gY2xpZW50IHJlc2VuZHMg
dGhhdCBzYW1lIG9wZXJhdGlvbiB3aXRoIHRoZSBzYW1lIHN0YXRlaWQgb3ZlciB0byB0aGUNCj4g
c2VydmVyIGFuZCBnZXRzIHRoZSBzYW1lIEVSUl9PTERfU1RBVEVJRCBiYWNrIHdoaWNoIHB1dHMg
dGhlIGNsaWVudA0KPiBpbg0KPiBhbiBpbmZpbml0ZSBsb29wIChhbmQgYSBodW5nIHN0YXRlKS4N
Cj4gDQo+IFNob3VsZG4ndCB0aGUgY2xpZW50IGluc3RlYWQgdHJlYXQgRVJSX09MRF9TVEFURUlE
IHNhbWUgYXMNCj4gRVJSX0JBRF9TVEFURUlEPw0KDQpObywgTkZTNEVSUl9PTERfU1RBVEVJRCBp
cyBhbG1vc3QgYWx3YXlzIGR1ZSB0byBhIHJhY2UgYmV0d2VlbiBhbg0KaW5jb21pbmcgUlBDIHJl
cGx5IGFuZCBvdXRnb2luZyBSUEMgY2FsbC4gSXQncyB3aGVuIHRoZSBzZXJ2ZXIgYnVtcHMNCnRo
ZSBzdGF0ZSwgYW5kIHRoZSBjbGllbnQgaGFwcGVucyBub3QgdG8ga25vdyBhYm91dCBpdCB5ZXQu
DQoNCklPVzogV2UgZGVmaW5pdGVseSBkbyBub3Qgd2FudCB0byB0cmVhdCBpdCBhcyBhIHBlcm1h
bmVudCBlcnJvci4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFp
bnRhaW5lciwgUHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg==