Re: sunrpc/cache.c: races while updating cache entries

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

SGksCgpoZXJlIGlzIGEgcGF0Y2ggaW4gdHdvIHZlcnNpb25zLCBvbmUgZm9yIG1haW5saW5l
IGFuZAp0aGUgc2Vjb25kIGZvciBTTEVTMTEgU1AxLiBJIGRvbid0IGtub3csIHdoZXRoZXIg
dGhpcwppcyB0aGUgYmVzdCBzb2x1dGlvbiwgYnV0IHRoZSB2ZXJzaW9uIGZvciBTTEVTMTEg
Zml4ZXMKdGhlIHByb2JsZW0gZm9yIG1lLiBJdCBhbHJlYWR5IGlzIHRlc3RlZCBmb3IgYWJv
dXQgMTIKaG91cnMgYW5kIEkganVzdCBzdGFydGVkIHRoZSB0ZXN0IGFnYWluIHRvIGhhdmUg
aXQgcnVuCmZvciB0aGUgZW50aXJlIHdlZWtlbmQuCgpUaGUgcGF0Y2ggZm9yIG1haW5saW5l
IGlzIHVudGVzdGVkLgoKQm9kbwoKIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMK
CkZyb206IEJvZG8gU3Ryb2Vzc2VyIDxic3Ryb2Vzc2VyQHRzLmZ1aml0c3UuY29tPgpEYXRl
OiBGcmksIDE1IE1hciAyMDEzClN1YmplY3Q6IFtQQVRDSF0gbmV0OiBzdW5ycGM6IGZpeCBy
YWNlIGluIFJQQyBjYWNoZQoKVGhpcyBwYXRjaCBhcHBsaWVzIHRvIGxpbnV4LTMuNy42IHBs
dXMgMiBlYXJsaWVyCnBhdGNoZXMgdGhhdCBhbHNvIGZpeCByYWNlcwoKSWYgYmV0d2VlbiBh
IHRocmVhZCdzIGNhbGxzIHRvIHN1bnJwY19jYWNoZV9sb29rdXAoKQphbmQgY2FjaGVfY2hl
Y2soKSB0aGUgY2FjaGUgZW50cnkgcmV0dXJuZWQgYnkgbG9va3VwCmlzIG1vZGlmaWVkIGJ5
CmEpIHN1bnJwY19jYWNoZV91cGRhdGUoKSBzZXR0aW5nIGV4cGlyeV90aW1lIGlzIHNldCB0
byAwCmIpIGNhY2hlX2NsZWFuKCkgdW5oYXNoaW5nIGl0LCBhcyBpdCBub3cgaXMgZXhwaXJl
ZAp0aGVuIHRoZSB0aHJlYWQgd2lsbCBwZXJmb3JtIGFuIHVwY2FsbCBmb3IgdGhlIG91dGRh
dGVkCmNhY2hlIGVudHJ5LiBUaGF0IGNhY2hlX3JlcXVlc3Qgd2lsbCBzdGF5IG9uIHRoZSBx
dWV1ZQpmb3IgZXZlciwgYXMgYSByZXBseSB0byB0aGUgcmVxdWVzdCB3aWxsIGFsd2F5cyBt
YXRjaAphIG5ld2VyIGNhY2hlIGVudHJ5IGFuZCB0aGUgcmVxdWVzdCBpc24ndCBkZXF1ZXVl
ZC4KT1RPSCwgY2FjaGVfY2xlYW4oKSBhbHJlYWR5IHVuaGFzaGVkIHRoZSBvbGQgZW50cnks
IHRodXMKY2FjaGVfY2xlYW4oKSB3b24ndCBkZXF1ZXVlIHRoZSByZXF1ZXN0IGFsc28uCgpU
byBmaXggdGhlIHByb2JsZW0sIGluIGNhY2hlX2NoZWNrKCkgd2UgcmVjaGVjayB0aGUgc3Rh
dGUKb2YgdGhlIGNhY2hlIGVudHJ5IGFmdGVyIGhhdmluZyBzZXQgQ0FDSEVfUEVORElORy4g
U28gd2UKYXJlIHN1cmUgdG8gbmV2ZXIgZG8gYW4gdXBjYWxsIGZvciBhbiBvdXRkYXRlZCBl
bnRyeS4KClNpZ25lZC1vZmYtYnk6IEJvZG8gU3Ryb2Vzc2VyIDxic3Ryb2Vzc2VyQHRzLmZ1
aml0c3UuY29tPgotLS0KCi0tLSBhL25ldC9zdW5ycGMvY2FjaGUuYwkyMDEzLTAzLTE1IDIx
OjE0OjU0LjAwMDAwMDAwMCArMDEwMAorKysgYi9uZXQvc3VucnBjL2NhY2hlLmMJMjAxMy0w
My0xNSAyMToyOTowOS4wMDAwMDAwMDAgKzAxMDAKQEAgLTIyMiw2ICsyMjIsMTcgQEAgc3Rh
dGljIGlubGluZSBpbnQgY2FjaGVfaXNfdmFsaWQoc3RydWN0CiAJfQogfQogCitzdGF0aWMg
aW5saW5lIGludCBjYWNoZV93YW50c191cGNhbGwoc3RydWN0IGNhY2hlX2hlYWQgKmgsIGlu
dCBmaW5hbCkKK3sKKwlsb25nIHJlZnJlc2hfYWdlID0gKGgtPmV4cGlyeV90aW1lIC0gaC0+
bGFzdF9yZWZyZXNoKTsKKwlsb25nIGFnZSA9IHNlY29uZHNfc2luY2VfYm9vdCgpIC0gaC0+
bGFzdF9yZWZyZXNoOworCisJaWYgKGZpbmFsKQorCQlkcHJpbnRrKCJSUEM6ICAgICAgIFdh
bnQgdXBkYXRlLCByZWZhZ2U9JWxkLCBhZ2U9JWxkXG4iLCByZWZyZXNoX2FnZSwgYWdlKTsK
KworCXJldHVybiBoLT5leHBpcnlfdGltZSAmJiBhZ2UgPiByZWZyZXNoX2FnZS8yOworfQor
CiBzdGF0aWMgaW50IHRyeV90b19uZWdhdGVfZW50cnkoc3RydWN0IGNhY2hlX2RldGFpbCAq
ZGV0YWlsLCBzdHJ1Y3QgY2FjaGVfaGVhZCAqaCkKIHsKIAlpbnQgcnY7CkBAIC0yNTUsMjQg
KzI2NiwyMiBAQCBzdGF0aWMgaW50IHRyeV90b19uZWdhdGVfZW50cnkoc3RydWN0IGNhCiBp
bnQgY2FjaGVfY2hlY2soc3RydWN0IGNhY2hlX2RldGFpbCAqZGV0YWlsLAogCQkgICAgc3Ry
dWN0IGNhY2hlX2hlYWQgKmgsIHN0cnVjdCBjYWNoZV9yZXEgKnJxc3RwKQogewotCWludCBy
djsKLQlsb25nIHJlZnJlc2hfYWdlLCBhZ2U7CisJaW50IHJ2LCByYzsKIAogCS8qIEZpcnN0
IGRlY2lkZSByZXR1cm4gc3RhdHVzIGFzIGJlc3Qgd2UgY2FuICovCiAJcnYgPSBjYWNoZV9p
c192YWxpZChkZXRhaWwsIGgpOwogCi0JLyogbm93IHNlZSBpZiB3ZSB3YW50IHRvIHN0YXJ0
IGFuIHVwY2FsbCAqLwotCXJlZnJlc2hfYWdlID0gKGgtPmV4cGlyeV90aW1lIC0gaC0+bGFz
dF9yZWZyZXNoKTsKLQlhZ2UgPSBzZWNvbmRzX3NpbmNlX2Jvb3QoKSAtIGgtPmxhc3RfcmVm
cmVzaDsKLQogCWlmIChycXN0cCA9PSBOVUxMKSB7CiAJCWlmIChydiA9PSAtRUFHQUlOKQog
CQkJcnYgPSAtRU5PRU5UOwotCX0gZWxzZSBpZiAocnYgPT0gLUVBR0FJTiB8fCBhZ2UgPiBy
ZWZyZXNoX2FnZS8yKSB7Ci0JCWRwcmludGsoIlJQQzogICAgICAgV2FudCB1cGRhdGUsIHJl
ZmFnZT0lbGQsIGFnZT0lbGRcbiIsCi0JCQkJcmVmcmVzaF9hZ2UsIGFnZSk7CisJfSBlbHNl
IGlmIChydiA9PSAtRUFHQUlOIHx8IGNhY2hlX3dhbnRzX3VwY2FsbChoLCAwKSkgewogCQlp
ZiAoIXRlc3RfYW5kX3NldF9iaXQoQ0FDSEVfUEVORElORywgJmgtPmZsYWdzKSkgewotCQkJ
c3dpdGNoIChjYWNoZV9tYWtlX3VwY2FsbChkZXRhaWwsIGgpKSB7CisJCQlydiA9IGNhY2hl
X2lzX3ZhbGlkKGRldGFpbCwgaCk7CisJCQlpZiAocnYgPT0gLUVBR0FJTiB8fCBjYWNoZV93
YW50c191cGNhbGwoaCwgMSkpCisJCQkJcmMgPSBjYWNoZV9tYWtlX3VwY2FsbChkZXRhaWws
IGgpOworCQkJZWxzZQorCQkJCXJjID0gLUVBR0FJTjsKKwkJCXN3aXRjaCAocmMpIHsKIAkJ
CWNhc2UgLUVJTlZBTDoKIAkJCQlydiA9IHRyeV90b19uZWdhdGVfZW50cnkoZGV0YWlsLCBo
KTsKIAkJCQlicmVhazsKCiMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjCgpGcm9t
OiBCb2RvIFN0cm9lc3NlciA8YnN0cm9lc3NlckB0cy5mdWppdHN1LmNvbT4KRGF0ZTogRnJp
LCAxNSBNYXIgMjAxMwpTdWJqZWN0OiBbUEFUQ0hdIG5ldDogc3VucnBjOiBmaXggcmFjZSBp
biBSUEMgY2FjaGUKClRoaXMgcGF0Y2ggYXBwbGllcyB0byBsaW51eC0yLjYuMzIuNTktMC43
LjEuNDU0My4wLlBURgpwbHVzIDIgZWFybGllciBwYXRjaGVzIHRoYXQgYWxzbyBmaXggcmFj
ZXMKCklmIGJldHdlZW4gYSB0aHJlYWQncyBjYWxscyB0byBzdW5ycGNfY2FjaGVfbG9va3Vw
KCkKYW5kIGNhY2hlX2NoZWNrKCkgdGhlIGNhY2hlIGVudHJ5IHJldHVybmVkIGJ5IGxvb2t1
cAppcyBtb2RpZmllZCBieQphKSBzdW5ycGNfY2FjaGVfdXBkYXRlKCkgc2V0dGluZyBleHBp
cnlfdGltZSBpcyBzZXQgdG8gMApiKSBjYWNoZV9jbGVhbigpIHVuaGFzaGluZyBpdCwgYXMg
aXQgbm93IGlzIGV4cGlyZWQKdGhlbiB0aGUgdGhyZWFkIHdpbGwgcGVyZm9ybSBhbiB1cGNh
bGwgZm9yIHRoZSBvdXRkYXRlZApjYWNoZSBlbnRyeS4gVGhhdCBjYWNoZV9yZXF1ZXN0IHdp
bGwgc3RheSBvbiB0aGUgcXVldWUKZm9yIGV2ZXIsIGFzIGEgcmVwbHkgdG8gdGhlIHJlcXVl
c3Qgd2lsbCBhbHdheXMgbWF0Y2gKYSBuZXdlciBjYWNoZSBlbnRyeSBhbmQgdGhlIHJlcXVl
c3QgaXNuJ3QgZGVxdWV1ZWQuCk9UT0gsIGNhY2hlX2NsZWFuKCkgYWxyZWFkeSB1bmhhc2hl
ZCB0aGUgb2xkIGVudHJ5LCB0aHVzCmNhY2hlX2NsZWFuKCkgd29uJ3QgZGVxdWV1ZSB0aGUg
cmVxdWVzdCBhbHNvLgoKT24gdGhpcyBvbGRlciBTVyB2ZXJzaW9uLCB0aGUgcHJvYmxlbSBh
ZGRpdGlvbmFsbHkgbGVhZHMKdG8gUlBDIHJlcXVlc3RzIGJlaW5nIGRyb3BwZWQuCgpUbyBm
aXggdGhlIHByb2JsZW0sIGluIGNhY2hlX2NoZWNrKCkgd2UgcmVjaGVjayB0aGUgc3RhdGUK
b2YgdGhlIGNhY2hlIGVudHJ5IGFmdGVyIGhhdmluZyBzZXQgQ0FDSEVfUEVORElORy4gU28g
d2UKYXJlIHN1cmUgdG8gbmV2ZXIgZG8gYW4gdXBjYWxsIGZvciBhbiBvdXRkYXRlZCBlbnRy
eS4KClNpZ25lZC1vZmYtYnk6IEJvZG8gU3Ryb2Vzc2VyIDxic3Ryb2Vzc2VyQHRzLmZ1aml0
c3UuY29tPgotLS0KCi0tLSBhL25ldC9zdW5ycGMvY2FjaGUuYwkyMDEzLTAzLTE1IDIwOjAz
OjU0LjAwMDAwMDAwMCArMDEwMAorKysgYi9uZXQvc3VucnBjL2NhY2hlLmMJMjAxMy0wMy0x
NSAyMDoxNDoxMy4wMDAwMDAwMDAgKzAxMDAKQEAgLTE4NCw3ICsxODQsNyBAQCBzdGF0aWMg
aW50IGNhY2hlX21ha2VfdXBjYWxsKHN0cnVjdCBjYWNoCiBzdGF0aWMgaW5saW5lIGludCBj
YWNoZV9pc192YWxpZChzdHJ1Y3QgY2FjaGVfZGV0YWlsICpkZXRhaWwsIHN0cnVjdCBjYWNo
ZV9oZWFkICpoKQogewogCWlmICghdGVzdF9iaXQoQ0FDSEVfVkFMSUQsICZoLT5mbGFncykg
fHwKLQkgICAgaC0+ZXhwaXJ5X3RpbWUgPCBtb25vdG9uaWNfc2Vjb25kcygpKQorCSAgICAo
aC0+ZXhwaXJ5X3RpbWUgJiYgaC0+ZXhwaXJ5X3RpbWUgPCBtb25vdG9uaWNfc2Vjb25kcygp
KSkKIAkJcmV0dXJuIC1FQUdBSU47CiAJZWxzZSBpZiAoZGV0YWlsLT5mbHVzaF90aW1lID4g
aC0+bGFzdF9yZWZyZXNoKQogCQlyZXR1cm4gLUVBR0FJTjsKQEAgLTE5Nyw2ICsxOTcsMTcg
QEAgc3RhdGljIGlubGluZSBpbnQgY2FjaGVfaXNfdmFsaWQoc3RydWN0CiAJfQogfQogCitz
dGF0aWMgaW5saW5lIGludCBjYWNoZV93YW50c191cGNhbGwoc3RydWN0IGNhY2hlX2hlYWQg
KmgsIGludCBmaW5hbCkKK3sKKwlsb25nIHJlZnJlc2hfYWdlID0gKGgtPmV4cGlyeV90aW1l
IC0gaC0+bGFzdF9yZWZyZXNoKTsKKwlsb25nIGFnZSA9IG1vbm90b25pY19zZWNvbmRzKCkg
LSBoLT5sYXN0X3JlZnJlc2g7CisKKwlpZiAoZmluYWwpCisJCWRwcmludGsoIlJQQzogICAg
ICAgV2FudCB1cGRhdGUsIHJlZmFnZT0lbGQsIGFnZT0lbGRcbiIsIHJlZnJlc2hfYWdlLCBh
Z2UpOworCisJcmV0dXJuIGgtPmV4cGlyeV90aW1lICYmIGFnZSA+IHJlZnJlc2hfYWdlLzI7
Cit9CisKIC8qCiAgKiBUaGlzIGlzIHRoZSBnZW5lcmljIGNhY2hlIG1hbmFnZW1lbnQgcm91
dGluZSBmb3IgYWxsCiAgKiB0aGUgYXV0aGVudGljYXRpb24gY2FjaGVzLgpAQCAtMjE0LDI0
ICsyMjUsMjIgQEAgc3RhdGljIGlubGluZSBpbnQgY2FjaGVfaXNfdmFsaWQoc3RydWN0CiBp
bnQgY2FjaGVfY2hlY2soc3RydWN0IGNhY2hlX2RldGFpbCAqZGV0YWlsLAogCQkgICAgc3Ry
dWN0IGNhY2hlX2hlYWQgKmgsIHN0cnVjdCBjYWNoZV9yZXEgKnJxc3RwKQogewotCWludCBy
djsKLQlsb25nIHJlZnJlc2hfYWdlLCBhZ2U7CisJaW50IHJ2LCByYzsKIAogCS8qIEZpcnN0
IGRlY2lkZSByZXR1cm4gc3RhdHVzIGFzIGJlc3Qgd2UgY2FuICovCiAJcnYgPSBjYWNoZV9p
c192YWxpZChkZXRhaWwsIGgpOwogCi0JLyogbm93IHNlZSBpZiB3ZSB3YW50IHRvIHN0YXJ0
IGFuIHVwY2FsbCAqLwotCXJlZnJlc2hfYWdlID0gKGgtPmV4cGlyeV90aW1lIC0gaC0+bGFz
dF9yZWZyZXNoKTsKLQlhZ2UgPSBtb25vdG9uaWNfc2Vjb25kcygpIC0gaC0+bGFzdF9yZWZy
ZXNoOwotCiAJaWYgKHJxc3RwID09IE5VTEwpIHsKIAkJaWYgKHJ2ID09IC1FQUdBSU4pCiAJ
CQlydiA9IC1FTk9FTlQ7Ci0JfSBlbHNlIGlmIChydiA9PSAtRUFHQUlOIHx8IGFnZSA+IHJl
ZnJlc2hfYWdlLzIpIHsKLQkJZHByaW50aygiUlBDOiAgICAgICBXYW50IHVwZGF0ZSwgcmVm
YWdlPSVsZCwgYWdlPSVsZFxuIiwKLQkJCQlyZWZyZXNoX2FnZSwgYWdlKTsKKwl9IGVsc2Ug
aWYgKHJ2ID09IC1FQUdBSU4gfHwgY2FjaGVfd2FudHNfdXBjYWxsKGgsIDApKSB7CiAJCWlm
ICghdGVzdF9hbmRfc2V0X2JpdChDQUNIRV9QRU5ESU5HLCAmaC0+ZmxhZ3MpKSB7Ci0JCQlz
d2l0Y2ggKGNhY2hlX21ha2VfdXBjYWxsKGRldGFpbCwgaCkpIHsKKwkJCXJ2ID0gY2FjaGVf
aXNfdmFsaWQoZGV0YWlsLCBoKTsKKwkJCWlmIChydiA9PSAtRUFHQUlOIHx8IGNhY2hlX3dh
bnRzX3VwY2FsbChoLCAxKSkKKwkJCQlyYyA9IGNhY2hlX21ha2VfdXBjYWxsKGRldGFpbCwg
aCk7CisJCQllbHNlCisJCQkJcmMgPSAtRUFHQUlOOworCQkJc3dpdGNoIChyYykgewogCQkJ
Y2FzZSAtRUlOVkFMOgogCQkJCXdyaXRlX2xvY2soJmRldGFpbC0+aGFzaF9sb2NrKTsKIAkJ
CQlpZiAocnYpIHsK

Re: sunrpc/cache.c: races while updating cache entries

[J. Bruce Fields]

On Thu, Jun 13, 2013 at 11:54:56AM +1000, NeilBrown wrote:
> On 03 Jun 2013 16:27:06 +0200 Bodo Stroesser <bstroesser@ts.fujitsu.com>
> wrote:
> 
> > On Fri, Apr 19, 2013 at 06:56:00PM +0200, Bodo Stroesser wrote:
> > > 
> > > We started the test of the -SP2 (and mainline) series on Tue, 9th, but had no
> > > success.
> > > We did _not_ find a problem with the patches, but under -SP2 our test scenario
> > > has less than 40% of the throughput we saw under -SP1. With that low
> > > performance, we had a 4 day run without any dropped RPC request. But we don't
> > > know the error rate without the patches under these conditions. So we can't
> > > give an o.k. for the patches yet.
> > > 
> > > Currently we try to find the reason for the different behavior of SP1 and SP2
> > > 
> > 
> > Hi,
> > 
> > sorry for the delay. Meanwhile we found the reason for the small throughput
> > with -SP2. The problem resulted from a change in our own software.
> > 
> > Thus I could fix this and started a test on last Tuesday. I stopped the test
> > today after 6 days without any lost RPC. Without the patches I saw the first
> > dropped RPC after 3 hours. Thus, I think the patches for -SP2 are fine. 
> > 
> > @Neil: would patch 0006 of the -SP1 patchset be a good additional change for
> > mainline?
> > 
> > Bodo
> 
> Thanks for all the testing.
> 
> Bruce: where are you at with these?  Are you holding one to some that I sent
> previously, or should I resend them all?

No, I'm not holding on to any--if you could resend them all that would
be great.

--b.

> 
> 
> Bodo: no, I don't think that patch is appropriate for mainline.  It causes
> sunrpc_cache_pipe_upcall to abort if ->expiry_time is zero.  There is
> certainly no point in doing an upcall in that case, but the code in mainline
> is quite different to the code in -SP1 against which that patch made sense.
> 
> For mainline an equivalent optimisation which probably makes the interesting
> case more obvious would be:
> 
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index d01eb07..291cc47 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -262,7 +262,8 @@ int cache_check(struct cache_detail *detail,
>  	if (rqstp == NULL) {
>  		if (rv == -EAGAIN)
>  			rv = -ENOENT;
> -	} else if (rv == -EAGAIN || age > refresh_age/2) {
> +	} else if (rv == -EAGAIN ||
> +		   (refresh_age > 0 && age > refresh_age/2)) {
>  		dprintk("RPC:       Want update, refage=%ld, age=%ld\n",
>  				refresh_age, age);
>  		if (!test_and_set_bit(CACHE_PENDING, &h->flags)) {
> 
> 
> i.e. trap that case in cache_check.
> 
> NeilBrown

Re: sunrpc/cache.c: races while updating cache entries

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2393 bytes --]

On 03 Jun 2013 16:27:06 +0200 Bodo Stroesser <bstroesser@ts.fujitsu.com>
wrote:

> On Fri, Apr 19, 2013 at 06:56:00PM +0200, Bodo Stroesser wrote:
> > 
> > We started the test of the -SP2 (and mainline) series on Tue, 9th, but had no
> > success.
> > We did _not_ find a problem with the patches, but under -SP2 our test scenario
> > has less than 40% of the throughput we saw under -SP1. With that low
> > performance, we had a 4 day run without any dropped RPC request. But we don't
> > know the error rate without the patches under these conditions. So we can't
> > give an o.k. for the patches yet.
> > 
> > Currently we try to find the reason for the different behavior of SP1 and SP2
> > 
> 
> Hi,
> 
> sorry for the delay. Meanwhile we found the reason for the small throughput
> with -SP2. The problem resulted from a change in our own software.
> 
> Thus I could fix this and started a test on last Tuesday. I stopped the test
> today after 6 days without any lost RPC. Without the patches I saw the first
> dropped RPC after 3 hours. Thus, I think the patches for -SP2 are fine. 
> 
> @Neil: would patch 0006 of the -SP1 patchset be a good additional change for
> mainline?
> 
> Bodo

Thanks for all the testing.

Bruce: where are you at with these?  Are you holding one to some that I sent
previously, or should I resend them all?


Bodo: no, I don't think that patch is appropriate for mainline.  It causes
sunrpc_cache_pipe_upcall to abort if ->expiry_time is zero.  There is
certainly no point in doing an upcall in that case, but the code in mainline
is quite different to the code in -SP1 against which that patch made sense.

For mainline an equivalent optimisation which probably makes the interesting
case more obvious would be:

diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index d01eb07..291cc47 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -262,7 +262,8 @@ int cache_check(struct cache_detail *detail,
 	if (rqstp == NULL) {
 		if (rv == -EAGAIN)
 			rv = -ENOENT;
-	} else if (rv == -EAGAIN || age > refresh_age/2) {
+	} else if (rv == -EAGAIN ||
+		   (refresh_age > 0 && age > refresh_age/2)) {
 		dprintk("RPC:       Want update, refage=%ld, age=%ld\n",
 				refresh_age, age);
 		if (!test_and_set_bit(CACHE_PENDING, &h->flags)) {


i.e. trap that case in cache_check.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gRnJpLCBBcHIgMTksIDIwMTMgYXQgMDY6NTY6MDBQTSArMDIwMCwgQm9kbyBTdHJvZXNz
ZXIgd3JvdGU6Cj4gCj4gV2Ugc3RhcnRlZCB0aGUgdGVzdCBvZiB0aGUgLVNQMiAoYW5kIG1h
aW5saW5lKSBzZXJpZXMgb24gVHVlLCA5dGgsIGJ1dCBoYWQgbm8KPiBzdWNjZXNzLgo+IFdl
IGRpZCBfbm90XyBmaW5kIGEgcHJvYmxlbSB3aXRoIHRoZSBwYXRjaGVzLCBidXQgdW5kZXIg
LVNQMiBvdXIgdGVzdCBzY2VuYXJpbwo+IGhhcyBsZXNzIHRoYW4gNDAlIG9mIHRoZSB0aHJv
dWdocHV0IHdlIHNhdyB1bmRlciAtU1AxLiBXaXRoIHRoYXQgbG93Cj4gcGVyZm9ybWFuY2Us
IHdlIGhhZCBhIDQgZGF5IHJ1biB3aXRob3V0IGFueSBkcm9wcGVkIFJQQyByZXF1ZXN0LiBC
dXQgd2UgZG9uJ3QKPiBrbm93IHRoZSBlcnJvciByYXRlIHdpdGhvdXQgdGhlIHBhdGNoZXMg
dW5kZXIgdGhlc2UgY29uZGl0aW9ucy4gU28gd2UgY2FuJ3QKPiBnaXZlIGFuIG8uay4gZm9y
IHRoZSBwYXRjaGVzIHlldC4KPiAKPiBDdXJyZW50bHkgd2UgdHJ5IHRvIGZpbmQgdGhlIHJl
YXNvbiBmb3IgdGhlIGRpZmZlcmVudCBiZWhhdmlvciBvZiBTUDEgYW5kIFNQMgo+IAoKSGks
Cgpzb3JyeSBmb3IgdGhlIGRlbGF5LiBNZWFud2hpbGUgd2UgZm91bmQgdGhlIHJlYXNvbiBm
b3IgdGhlIHNtYWxsIHRocm91Z2hwdXQKd2l0aCAtU1AyLiBUaGUgcHJvYmxlbSByZXN1bHRl
ZCBmcm9tIGEgY2hhbmdlIGluIG91ciBvd24gc29mdHdhcmUuCgpUaHVzIEkgY291bGQgZml4
IHRoaXMgYW5kIHN0YXJ0ZWQgYSB0ZXN0IG9uIGxhc3QgVHVlc2RheS4gSSBzdG9wcGVkIHRo
ZSB0ZXN0CnRvZGF5IGFmdGVyIDYgZGF5cyB3aXRob3V0IGFueSBsb3N0IFJQQy4gV2l0aG91
dCB0aGUgcGF0Y2hlcyBJIHNhdyB0aGUgZmlyc3QKZHJvcHBlZCBSUEMgYWZ0ZXIgMyBob3Vy
cy4gVGh1cywgSSB0aGluayB0aGUgcGF0Y2hlcyBmb3IgLVNQMiBhcmUgZmluZS4gCgpATmVp
bDogd291bGQgcGF0Y2ggMDAwNiBvZiB0aGUgLVNQMSBwYXRjaHNldCBiZSBhIGdvb2QgYWRk
aXRpb25hbCBjaGFuZ2UgZm9yCm1haW5saW5lPwoKQm9kbwo=

Re: sunrpc/cache.c: races while updating cache entries

[Namjae Jeon]

Hi.

Sorry for interrupt.
I fixed my issue using this patch(nfsd4: fix hang on fast-booting nfs
servers). it was different issue with this subject on current mail.

Thanks.

2013/5/10, Namjae Jeon <linkinjeon@gmail.com>:
> Hi. Bodo.
>
> We are facing issues with respect to the SUNRPC cache.
> In our case we have two targets connected back-to-back
> NFS Server: Kernel version, 2.6.35
>
> At times, when Client tries to connect to the Server it stucks for
> very long duration and keeps on trying to mount.
>
> When we try to figure out using logs, we checked that client was not
> getting response of FSINFO request.
>
> Further, by debugging we found that the request was getting dropped at
> the SERVER, so this request was not being served.
>
> In the code we reached this point:
> svcauth_unix_set_client()->
>   gi = unix_gid_find(cred->cr_uid, rqstp);
>         switch (PTR_ERR(gi)) {
>         case -EAGAIN:
>                 return SVC_DROP;
>
> This path is related with the SUNRPC cache management.
>
> When we remove this UNIX_GID_FIND path from our code, there is no problem.
>
> When we try to figure the possible related problems as per our
> scneario, We found that you have faced similar issue for RACE in the
> cache.
> Can you please suggest what could be the problem  so that we can check
> further ?
>
> Or from the solution if you encounter the similar situation.
> can you please suggest on the possible patches for 2.6.35 - which we
> can try in our environment ?
>
> We will be highly grateful.
>
> Thanks
>
>
> 2013/4/20, Bodo Stroesser <bstroesser@ts.fujitsu.com>:
>> On 05 Apr 2013 23:09:00 +0100 J. Bruce Fields <bfields@fieldses.org>
>> wrote:
>>> On Fri, Apr 05, 2013 at 05:33:49PM +0200, Bodo Stroesser wrote:
>>> > On 05 Apr 2013 14:40:00 +0100 J. Bruce Fields <bfields@fieldses.org>
>>> > wrote:
>>> > > On Thu, Apr 04, 2013 at 07:59:35PM +0200, Bodo Stroesser wrote:
>>> > > > There is no reason for apologies. The thread meanwhile seems to be
>>> > > > a
>>> > > > bit
>>> > > > confusing :-)
>>> > > >
>>> > > > Current state is:
>>> > > >
>>> > > > - Neil Brown has created two series of patches. One for SLES11-SP1
>>> > > > and a
>>> > > >   second one for -SP2
>>> > > >
>>> > > > - AFAICS, the series for -SP2 will match with mainline also.
>>> > > >
>>> > > > - Today I found and fixed the (hopefully) last problem in the -SP1
>>> > > > series.
>>> > > >   My test using this patchset will run until Monday.
>>> > > >
>>> > > > - Provided the test on SP1 succeeds, probably on Tuesday I'll
>>> > > > start
>>> > > > to test
>>> > > >   the patches for SP2 (and mainline). If it runs fine, we'll have
>>> > > > a
>>> > > > tested
>>> > > >   patchset not later than Mon 15th.
>>> > >
>>> > > OK, great, as long as it hasn't just been forgotten!
>>> > >
>>> > > I'd also be curious to understand why we aren't getting a lot of
>>> > > complaints about this from elsewhere....  Is there something unique
>>> > > about your setup?  Do the bugs that remain upstream take a long time
>>> > > to
>>> > > reproduce?
>>> > >
>>> > > --b.
>>> > >
>>> >
>>> > It's no secret, what we are doing. So let me try to explain:
>>>
>>> Thanks for the detailed explanation!  I'll look forward to the patches.
>>>
>>> --b.
>>>
>>
>> Let me give an intermediate result:
>>
>> The test of the -SP1 patch series succeeded.
>>
>> We started the test of the -SP2 (and mainline) series on Tue, 9th, but
>> had
>> no
>> success.
>> We did _not_ find a problem with the patches, but under -SP2 our test
>> scenario
>> has less than 40% of the throughput we saw under -SP1. With that low
>> performance, we had a 4 day run without any dropped RPC request. But we
>> don't
>> know the error rate without the patches under these conditions. So we
>> can't
>> give an o.k. for the patches yet.
>>
>> Currently we try to find the reason for the different behavior of SP1 and
>> SP2
>>
>> Bodo
>>
>

Re: sunrpc/cache.c: races while updating cache entries

[Namjae Jeon]

Hi. Bodo.

We are facing issues with respect to the SUNRPC cache.
In our case we have two targets connected back-to-back
NFS Server: Kernel version, 2.6.35

At times, when Client tries to connect to the Server it stucks for
very long duration and keeps on trying to mount.

When we try to figure out using logs, we checked that client was not
getting response of FSINFO request.

Further, by debugging we found that the request was getting dropped at
the SERVER, so this request was not being served.

In the code we reached this point:
svcauth_unix_set_client()->
  gi = unix_gid_find(cred->cr_uid, rqstp);
        switch (PTR_ERR(gi)) {
        case -EAGAIN:
                return SVC_DROP;

This path is related with the SUNRPC cache management.

When we remove this UNIX_GID_FIND path from our code, there is no problem.

When we try to figure the possible related problems as per our
scneario, We found that you have faced similar issue for RACE in the
cache.
Can you please suggest what could be the problem  so that we can check further ?

Or from the solution if you encounter the similar situation.
can you please suggest on the possible patches for 2.6.35 - which we
can try in our environment ?

We will be highly grateful.

Thanks


2013/4/20, Bodo Stroesser <bstroesser@ts.fujitsu.com>:
> On 05 Apr 2013 23:09:00 +0100 J. Bruce Fields <bfields@fieldses.org> wrote:
>> On Fri, Apr 05, 2013 at 05:33:49PM +0200, Bodo Stroesser wrote:
>> > On 05 Apr 2013 14:40:00 +0100 J. Bruce Fields <bfields@fieldses.org>
>> > wrote:
>> > > On Thu, Apr 04, 2013 at 07:59:35PM +0200, Bodo Stroesser wrote:
>> > > > There is no reason for apologies. The thread meanwhile seems to be a
>> > > > bit
>> > > > confusing :-)
>> > > >
>> > > > Current state is:
>> > > >
>> > > > - Neil Brown has created two series of patches. One for SLES11-SP1
>> > > > and a
>> > > >   second one for -SP2
>> > > >
>> > > > - AFAICS, the series for -SP2 will match with mainline also.
>> > > >
>> > > > - Today I found and fixed the (hopefully) last problem in the -SP1
>> > > > series.
>> > > >   My test using this patchset will run until Monday.
>> > > >
>> > > > - Provided the test on SP1 succeeds, probably on Tuesday I'll start
>> > > > to test
>> > > >   the patches for SP2 (and mainline). If it runs fine, we'll have a
>> > > > tested
>> > > >   patchset not later than Mon 15th.
>> > >
>> > > OK, great, as long as it hasn't just been forgotten!
>> > >
>> > > I'd also be curious to understand why we aren't getting a lot of
>> > > complaints about this from elsewhere....  Is there something unique
>> > > about your setup?  Do the bugs that remain upstream take a long time
>> > > to
>> > > reproduce?
>> > >
>> > > --b.
>> > >
>> >
>> > It's no secret, what we are doing. So let me try to explain:
>>
>> Thanks for the detailed explanation!  I'll look forward to the patches.
>>
>> --b.
>>
>
> Let me give an intermediate result:
>
> The test of the -SP1 patch series succeeded.
>
> We started the test of the -SP2 (and mainline) series on Tue, 9th, but had
> no
> success.
> We did _not_ find a problem with the patches, but under -SP2 our test
> scenario
> has less than 40% of the throughput we saw under -SP1. With that low
> performance, we had a 4 day run without any dropped RPC request. But we
> don't
> know the error rate without the patches under these conditions. So we can't
> give an o.k. for the patches yet.
>
> Currently we try to find the reason for the different behavior of SP1 and
> SP2
>
> Bodo
>

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMDUgQXByIDIwMTMgMjM6MDk6MDAgKzAxMDAgSi4gQnJ1Y2UgRmllbGRzIDxiZmllbGRz
QGZpZWxkc2VzLm9yZz4gd3JvdGU6Cj4gT24gRnJpLCBBcHIgMDUsIDIwMTMgYXQgMDU6MzM6
NDlQTSArMDIwMCwgQm9kbyBTdHJvZXNzZXIgd3JvdGU6Cj4gPiBPbiAwNSBBcHIgMjAxMyAx
NDo0MDowMCArMDEwMCBKLiBCcnVjZSBGaWVsZHMgPGJmaWVsZHNAZmllbGRzZXMub3JnPiB3
cm90ZToKPiA+ID4gT24gVGh1LCBBcHIgMDQsIDIwMTMgYXQgMDc6NTk6MzVQTSArMDIwMCwg
Qm9kbyBTdHJvZXNzZXIgd3JvdGU6Cj4gPiA+ID4gVGhlcmUgaXMgbm8gcmVhc29uIGZvciBh
cG9sb2dpZXMuIFRoZSB0aHJlYWQgbWVhbndoaWxlIHNlZW1zIHRvIGJlIGEgYml0Cj4gPiA+
ID4gY29uZnVzaW5nIDotKQo+ID4gPiA+IAo+ID4gPiA+IEN1cnJlbnQgc3RhdGUgaXM6Cj4g
PiA+ID4gCj4gPiA+ID4gLSBOZWlsIEJyb3duIGhhcyBjcmVhdGVkIHR3byBzZXJpZXMgb2Yg
cGF0Y2hlcy4gT25lIGZvciBTTEVTMTEtU1AxIGFuZCBhCj4gPiA+ID4gICBzZWNvbmQgb25l
IGZvciAtU1AyCj4gPiA+ID4gCj4gPiA+ID4gLSBBRkFJQ1MsIHRoZSBzZXJpZXMgZm9yIC1T
UDIgd2lsbCBtYXRjaCB3aXRoIG1haW5saW5lIGFsc28uCj4gPiA+ID4gCj4gPiA+ID4gLSBU
b2RheSBJIGZvdW5kIGFuZCBmaXhlZCB0aGUgKGhvcGVmdWxseSkgbGFzdCBwcm9ibGVtIGlu
IHRoZSAtU1AxIHNlcmllcy4KPiA+ID4gPiAgIE15IHRlc3QgdXNpbmcgdGhpcyBwYXRjaHNl
dCB3aWxsIHJ1biB1bnRpbCBNb25kYXkuCj4gPiA+ID4gCj4gPiA+ID4gLSBQcm92aWRlZCB0
aGUgdGVzdCBvbiBTUDEgc3VjY2VlZHMsIHByb2JhYmx5IG9uIFR1ZXNkYXkgSSdsbCBzdGFy
dCB0byB0ZXN0Cj4gPiA+ID4gICB0aGUgcGF0Y2hlcyBmb3IgU1AyIChhbmQgbWFpbmxpbmUp
LiBJZiBpdCBydW5zIGZpbmUsIHdlJ2xsIGhhdmUgYSB0ZXN0ZWQKPiA+ID4gPiAgIHBhdGNo
c2V0IG5vdCBsYXRlciB0aGFuIE1vbiAxNXRoLgo+ID4gPiAKPiA+ID4gT0ssIGdyZWF0LCBh
cyBsb25nIGFzIGl0IGhhc24ndCBqdXN0IGJlZW4gZm9yZ290dGVuIQo+ID4gPiAKPiA+ID4g
SSdkIGFsc28gYmUgY3VyaW91cyB0byB1bmRlcnN0YW5kIHdoeSB3ZSBhcmVuJ3QgZ2V0dGlu
ZyBhIGxvdCBvZgo+ID4gPiBjb21wbGFpbnRzIGFib3V0IHRoaXMgZnJvbSBlbHNld2hlcmUu
Li4uICBJcyB0aGVyZSBzb21ldGhpbmcgdW5pcXVlCj4gPiA+IGFib3V0IHlvdXIgc2V0dXA/
ICBEbyB0aGUgYnVncyB0aGF0IHJlbWFpbiB1cHN0cmVhbSB0YWtlIGEgbG9uZyB0aW1lIHRv
Cj4gPiA+IHJlcHJvZHVjZT8KPiA+ID4gCj4gPiA+IC0tYi4KPiA+ID4gCj4gPiAKPiA+IEl0
J3Mgbm8gc2VjcmV0LCB3aGF0IHdlIGFyZSBkb2luZy4gU28gbGV0IG1lIHRyeSB0byBleHBs
YWluOgo+IAo+IFRoYW5rcyBmb3IgdGhlIGRldGFpbGVkIGV4cGxhbmF0aW9uISAgSSdsbCBs
b29rIGZvcndhcmQgdG8gdGhlIHBhdGNoZXMuCj4gCj4gLS1iLgo+IAoKTGV0IG1lIGdpdmUg
YW4gaW50ZXJtZWRpYXRlIHJlc3VsdDoKClRoZSB0ZXN0IG9mIHRoZSAtU1AxIHBhdGNoIHNl
cmllcyBzdWNjZWVkZWQuCgpXZSBzdGFydGVkIHRoZSB0ZXN0IG9mIHRoZSAtU1AyIChhbmQg
bWFpbmxpbmUpIHNlcmllcyBvbiBUdWUsIDl0aCwgYnV0IGhhZCBubwpzdWNjZXNzLgpXZSBk
aWQgX25vdF8gZmluZCBhIHByb2JsZW0gd2l0aCB0aGUgcGF0Y2hlcywgYnV0IHVuZGVyIC1T
UDIgb3VyIHRlc3Qgc2NlbmFyaW8KaGFzIGxlc3MgdGhhbiA0MCUgb2YgdGhlIHRocm91Z2hw
dXQgd2Ugc2F3IHVuZGVyIC1TUDEuIFdpdGggdGhhdCBsb3cKcGVyZm9ybWFuY2UsIHdlIGhh
ZCBhIDQgZGF5IHJ1biB3aXRob3V0IGFueSBkcm9wcGVkIFJQQyByZXF1ZXN0LiBCdXQgd2Ug
ZG9uJ3QKa25vdyB0aGUgZXJyb3IgcmF0ZSB3aXRob3V0IHRoZSBwYXRjaGVzIHVuZGVyIHRo
ZXNlIGNvbmRpdGlvbnMuIFNvIHdlIGNhbid0CmdpdmUgYW4gby5rLiBmb3IgdGhlIHBhdGNo
ZXMgeWV0LgoKQ3VycmVudGx5IHdlIHRyeSB0byBmaW5kIHRoZSByZWFzb24gZm9yIHRoZSBk
aWZmZXJlbnQgYmVoYXZpb3Igb2YgU1AxIGFuZCBTUDIKCkJvZG8K

Re: sunrpc/cache.c: races while updating cache entries

[J. Bruce Fields]

On Fri, Apr 05, 2013 at 05:33:49PM +0200, Bodo Stroesser wrote:
> On 05 Apr 2013 14:40:00 +0100 J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Thu, Apr 04, 2013 at 07:59:35PM +0200, Bodo Stroesser wrote:
> > > There is no reason for apologies. The thread meanwhile seems to be a bit
> > > confusing :-)
> > > 
> > > Current state is:
> > > 
> > > - Neil Brown has created two series of patches. One for SLES11-SP1 and a
> > >   second one for -SP2
> > > 
> > > - AFAICS, the series for -SP2 will match with mainline also.
> > > 
> > > - Today I found and fixed the (hopefully) last problem in the -SP1 series.
> > >   My test using this patchset will run until Monday.
> > > 
> > > - Provided the test on SP1 succeeds, probably on Tuesday I'll start to test
> > >   the patches for SP2 (and mainline). If it runs fine, we'll have a tested
> > >   patchset not later than Mon 15th.
> > 
> > OK, great, as long as it hasn't just been forgotten!
> > 
> > I'd also be curious to understand why we aren't getting a lot of
> > complaints about this from elsewhere....  Is there something unique
> > about your setup?  Do the bugs that remain upstream take a long time to
> > reproduce?
> > 
> > --b.
> > 
> 
> It's no secret, what we are doing. So let me try to explain:

Thanks for the detailed explanation!  I'll look forward to the patches.

--b.

> 
> We build appliances for storage purposes. Each appliance mainly consists of
> a cluster of servers and a bunch of FibreChannel RAID systems. The servers
> of the appliance run SLES11.
> 
> One ore more of the servers in the cluster can act as a NFS server.
> 
> Each NFS server is connected to the RAID systems and has two 10 GBit/s Ethernet
> controllers for the link to the clients.
> 
> The appliance not only offers NFS access for clients, but also has some other
> types of interfaces to be used by the clients.
> 
> For QA of the appliances we use a special test system, that runs the entire
> appliance with all its interfaces under heavy load.
> 
> For the test of the NFS interfaces of the appliance, we connect the Ethernet
> links one by one to 10 GBit/s Ethernet controllers on a linux machine of the
> test system.
> 
> The SW on the test system for each Ethernet link uses 32 TCP connections to the
> NFS server in parallel. 
> 
> So between NFS server of the appliance and linux machine of the test system we
> have two 10 GBit/s links with 32 TCP/RPC/NFS_V3 connections each. Each link
> is running at up to 1 GByte/s throughput (per second and per link a total of
> 32k NFS3_READ or NFS3_WRITE RPCs of 32k data each.)
> 
> Normal Linux-NFS-Clients open only one single connection to a specific NFS
> server, even if there are multiple mounts. We do not use the linux builtin
> client, but create a RPC client by clnttcp_create() and do the NFS handling
> directly. Thus we can have multiple connections and we immediately can
> see if something goes wrong (e.g. if a RPC request is dropped), while the
> builtin linux client probably would do a silent retry. (But probably one
> could see single connections hang for a few minutes sporadically. Maybe
> someone hit by this would complain about the network ...)
> 
> As a side effect of this test setup all 64 connections to the NFS server
> use the same uid/gid and all 32 connections on one link come from the same
> ip address. This - as we know now - maximizes the stress for a single entry
> of the caches.
> 
> With our test setup at the beginning we had more than two dropped RPC request
> per hour and per NFS server. (Of course, this rate varied widely.) With each
> single change in cache.c the rate went down. The latest drop caused by a
> missing detail in the latest patchset for -SP1 occured after more than 2 days
> of testing!
> 
> Thus, to verify the patches I schedule a test for at least 4 days.
> 
> HTH
> Bodo

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMDUgQXByIDIwMTMgMTQ6NDA6MDAgKzAxMDAgSi4gQnJ1Y2UgRmllbGRzIDxiZmllbGRz
QGZpZWxkc2VzLm9yZz4gd3JvdGU6Cj4gT24gVGh1LCBBcHIgMDQsIDIwMTMgYXQgMDc6NTk6
MzVQTSArMDIwMCwgQm9kbyBTdHJvZXNzZXIgd3JvdGU6Cj4gPiBUaGVyZSBpcyBubyByZWFz
b24gZm9yIGFwb2xvZ2llcy4gVGhlIHRocmVhZCBtZWFud2hpbGUgc2VlbXMgdG8gYmUgYSBi
aXQKPiA+IGNvbmZ1c2luZyA6LSkKPiA+IAo+ID4gQ3VycmVudCBzdGF0ZSBpczoKPiA+IAo+
ID4gLSBOZWlsIEJyb3duIGhhcyBjcmVhdGVkIHR3byBzZXJpZXMgb2YgcGF0Y2hlcy4gT25l
IGZvciBTTEVTMTEtU1AxIGFuZCBhCj4gPiAgIHNlY29uZCBvbmUgZm9yIC1TUDIKPiA+IAo+
ID4gLSBBRkFJQ1MsIHRoZSBzZXJpZXMgZm9yIC1TUDIgd2lsbCBtYXRjaCB3aXRoIG1haW5s
aW5lIGFsc28uCj4gPiAKPiA+IC0gVG9kYXkgSSBmb3VuZCBhbmQgZml4ZWQgdGhlIChob3Bl
ZnVsbHkpIGxhc3QgcHJvYmxlbSBpbiB0aGUgLVNQMSBzZXJpZXMuCj4gPiAgIE15IHRlc3Qg
dXNpbmcgdGhpcyBwYXRjaHNldCB3aWxsIHJ1biB1bnRpbCBNb25kYXkuCj4gPiAKPiA+IC0g
UHJvdmlkZWQgdGhlIHRlc3Qgb24gU1AxIHN1Y2NlZWRzLCBwcm9iYWJseSBvbiBUdWVzZGF5
IEknbGwgc3RhcnQgdG8gdGVzdAo+ID4gICB0aGUgcGF0Y2hlcyBmb3IgU1AyIChhbmQgbWFp
bmxpbmUpLiBJZiBpdCBydW5zIGZpbmUsIHdlJ2xsIGhhdmUgYSB0ZXN0ZWQKPiA+ICAgcGF0
Y2hzZXQgbm90IGxhdGVyIHRoYW4gTW9uIDE1dGguCj4gCj4gT0ssIGdyZWF0LCBhcyBsb25n
IGFzIGl0IGhhc24ndCBqdXN0IGJlZW4gZm9yZ290dGVuIQo+IAo+IEknZCBhbHNvIGJlIGN1
cmlvdXMgdG8gdW5kZXJzdGFuZCB3aHkgd2UgYXJlbid0IGdldHRpbmcgYSBsb3Qgb2YKPiBj
b21wbGFpbnRzIGFib3V0IHRoaXMgZnJvbSBlbHNld2hlcmUuLi4uICBJcyB0aGVyZSBzb21l
dGhpbmcgdW5pcXVlCj4gYWJvdXQgeW91ciBzZXR1cD8gIERvIHRoZSBidWdzIHRoYXQgcmVt
YWluIHVwc3RyZWFtIHRha2UgYSBsb25nIHRpbWUgdG8KPiByZXByb2R1Y2U/Cj4gCj4gLS1i
Lgo+IAoKSXQncyBubyBzZWNyZXQsIHdoYXQgd2UgYXJlIGRvaW5nLiBTbyBsZXQgbWUgdHJ5
IHRvIGV4cGxhaW46CgpXZSBidWlsZCBhcHBsaWFuY2VzIGZvciBzdG9yYWdlIHB1cnBvc2Vz
LiBFYWNoIGFwcGxpYW5jZSBtYWlubHkgY29uc2lzdHMgb2YKYSBjbHVzdGVyIG9mIHNlcnZl
cnMgYW5kIGEgYnVuY2ggb2YgRmlicmVDaGFubmVsIFJBSUQgc3lzdGVtcy4gVGhlIHNlcnZl
cnMKb2YgdGhlIGFwcGxpYW5jZSBydW4gU0xFUzExLgoKT25lIG9yZSBtb3JlIG9mIHRoZSBz
ZXJ2ZXJzIGluIHRoZSBjbHVzdGVyIGNhbiBhY3QgYXMgYSBORlMgc2VydmVyLgoKRWFjaCBO
RlMgc2VydmVyIGlzIGNvbm5lY3RlZCB0byB0aGUgUkFJRCBzeXN0ZW1zIGFuZCBoYXMgdHdv
IDEwIEdCaXQvcyBFdGhlcm5ldApjb250cm9sbGVycyBmb3IgdGhlIGxpbmsgdG8gdGhlIGNs
aWVudHMuCgpUaGUgYXBwbGlhbmNlIG5vdCBvbmx5IG9mZmVycyBORlMgYWNjZXNzIGZvciBj
bGllbnRzLCBidXQgYWxzbyBoYXMgc29tZSBvdGhlcgp0eXBlcyBvZiBpbnRlcmZhY2VzIHRv
IGJlIHVzZWQgYnkgdGhlIGNsaWVudHMuCgpGb3IgUUEgb2YgdGhlIGFwcGxpYW5jZXMgd2Ug
dXNlIGEgc3BlY2lhbCB0ZXN0IHN5c3RlbSwgdGhhdCBydW5zIHRoZSBlbnRpcmUKYXBwbGlh
bmNlIHdpdGggYWxsIGl0cyBpbnRlcmZhY2VzIHVuZGVyIGhlYXZ5IGxvYWQuCgpGb3IgdGhl
IHRlc3Qgb2YgdGhlIE5GUyBpbnRlcmZhY2VzIG9mIHRoZSBhcHBsaWFuY2UsIHdlIGNvbm5l
Y3QgdGhlIEV0aGVybmV0CmxpbmtzIG9uZSBieSBvbmUgdG8gMTAgR0JpdC9zIEV0aGVybmV0
IGNvbnRyb2xsZXJzIG9uIGEgbGludXggbWFjaGluZSBvZiB0aGUKdGVzdCBzeXN0ZW0uCgpU
aGUgU1cgb24gdGhlIHRlc3Qgc3lzdGVtIGZvciBlYWNoIEV0aGVybmV0IGxpbmsgdXNlcyAz
MiBUQ1AgY29ubmVjdGlvbnMgdG8gdGhlCk5GUyBzZXJ2ZXIgaW4gcGFyYWxsZWwuIAoKU28g
YmV0d2VlbiBORlMgc2VydmVyIG9mIHRoZSBhcHBsaWFuY2UgYW5kIGxpbnV4IG1hY2hpbmUg
b2YgdGhlIHRlc3Qgc3lzdGVtIHdlCmhhdmUgdHdvIDEwIEdCaXQvcyBsaW5rcyB3aXRoIDMy
IFRDUC9SUEMvTkZTX1YzIGNvbm5lY3Rpb25zIGVhY2guIEVhY2ggbGluawppcyBydW5uaW5n
IGF0IHVwIHRvIDEgR0J5dGUvcyB0aHJvdWdocHV0IChwZXIgc2Vjb25kIGFuZCBwZXIgbGlu
ayBhIHRvdGFsIG9mCjMyayBORlMzX1JFQUQgb3IgTkZTM19XUklURSBSUENzIG9mIDMyayBk
YXRhIGVhY2guKQoKTm9ybWFsIExpbnV4LU5GUy1DbGllbnRzIG9wZW4gb25seSBvbmUgc2lu
Z2xlIGNvbm5lY3Rpb24gdG8gYSBzcGVjaWZpYyBORlMKc2VydmVyLCBldmVuIGlmIHRoZXJl
IGFyZSBtdWx0aXBsZSBtb3VudHMuIFdlIGRvIG5vdCB1c2UgdGhlIGxpbnV4IGJ1aWx0aW4K
Y2xpZW50LCBidXQgY3JlYXRlIGEgUlBDIGNsaWVudCBieSBjbG50dGNwX2NyZWF0ZSgpIGFu
ZCBkbyB0aGUgTkZTIGhhbmRsaW5nCmRpcmVjdGx5LiBUaHVzIHdlIGNhbiBoYXZlIG11bHRp
cGxlIGNvbm5lY3Rpb25zIGFuZCB3ZSBpbW1lZGlhdGVseSBjYW4Kc2VlIGlmIHNvbWV0aGlu
ZyBnb2VzIHdyb25nIChlLmcuIGlmIGEgUlBDIHJlcXVlc3QgaXMgZHJvcHBlZCksIHdoaWxl
IHRoZQpidWlsdGluIGxpbnV4IGNsaWVudCBwcm9iYWJseSB3b3VsZCBkbyBhIHNpbGVudCBy
ZXRyeS4gKEJ1dCBwcm9iYWJseSBvbmUKY291bGQgc2VlIHNpbmdsZSBjb25uZWN0aW9ucyBo
YW5nIGZvciBhIGZldyBtaW51dGVzIHNwb3JhZGljYWxseS4gTWF5YmUKc29tZW9uZSBoaXQg
YnkgdGhpcyB3b3VsZCBjb21wbGFpbiBhYm91dCB0aGUgbmV0d29yayAuLi4pCgpBcyBhIHNp
ZGUgZWZmZWN0IG9mIHRoaXMgdGVzdCBzZXR1cCBhbGwgNjQgY29ubmVjdGlvbnMgdG8gdGhl
IE5GUyBzZXJ2ZXIKdXNlIHRoZSBzYW1lIHVpZC9naWQgYW5kIGFsbCAzMiBjb25uZWN0aW9u
cyBvbiBvbmUgbGluayBjb21lIGZyb20gdGhlIHNhbWUKaXAgYWRkcmVzcy4gVGhpcyAtIGFz
IHdlIGtub3cgbm93IC0gbWF4aW1pemVzIHRoZSBzdHJlc3MgZm9yIGEgc2luZ2xlIGVudHJ5
Cm9mIHRoZSBjYWNoZXMuCgpXaXRoIG91ciB0ZXN0IHNldHVwIGF0IHRoZSBiZWdpbm5pbmcg
d2UgaGFkIG1vcmUgdGhhbiB0d28gZHJvcHBlZCBSUEMgcmVxdWVzdApwZXIgaG91ciBhbmQg
cGVyIE5GUyBzZXJ2ZXIuIChPZiBjb3Vyc2UsIHRoaXMgcmF0ZSB2YXJpZWQgd2lkZWx5Likg
V2l0aCBlYWNoCnNpbmdsZSBjaGFuZ2UgaW4gY2FjaGUuYyB0aGUgcmF0ZSB3ZW50IGRvd24u
IFRoZSBsYXRlc3QgZHJvcCBjYXVzZWQgYnkgYQptaXNzaW5nIGRldGFpbCBpbiB0aGUgbGF0
ZXN0IHBhdGNoc2V0IGZvciAtU1AxIG9jY3VyZWQgYWZ0ZXIgbW9yZSB0aGFuIDIgZGF5cwpv
ZiB0ZXN0aW5nIQoKVGh1cywgdG8gdmVyaWZ5IHRoZSBwYXRjaGVzIEkgc2NoZWR1bGUgYSB0
ZXN0IGZvciBhdCBsZWFzdCA0IGRheXMuCgpIVEgKQm9kbwo=

Re: sunrpc/cache.c: races while updating cache entries

[J. Bruce Fields]

On Thu, Apr 04, 2013 at 07:59:35PM +0200, Bodo Stroesser wrote:
> There is no reason for apologies. The thread meanwhile seems to be a bit
> confusing :-)
> 
> Current state is:
> 
> - Neil Brown has created two series of patches. One for SLES11-SP1 and a
>   second one for -SP2
> 
> - AFAICS, the series for -SP2 will match with mainline also.
> 
> - Today I found and fixed the (hopefully) last problem in the -SP1 series.
>   My test using this patchset will run until Monday.
> 
> - Provided the test on SP1 succeeds, probably on Tuesday I'll start to test
>   the patches for SP2 (and mainline). If it runs fine, we'll have a tested
>   patchset not later than Mon 15th.

OK, great, as long as it hasn't just been forgotten!

I'd also be curious to understand why we aren't getting a lot of
complaints about this from elsewhere....  Is there something unique
about your setup?  Do the bugs that remain upstream take a long time to
reproduce?

--b.

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMDMgQXByIDIwMTMgMjA6MzY6MDAgKzAxMDAgSi4gQnJ1Y2UgRmllbGRzIDxiZmllbGRz
QGZpZWxkc2VzLm9yZz4gd3JvdGU6Cj4gT24gVGh1LCBNYXIgMjEsIDIwMTMgYXQgMDU6NDE6
MzVQTSArMDEwMCwgQm9kbyBTdHJvZXNzZXIgd3JvdGU6Cj4gPiBPbiAyMSBNYXIgMjAxMyAw
MDozNDowMCArMDEwMCBOZWlsQnJvd24gPG5laWxiQHN1c2UuZGU+IHdyb3RlOgo+ID4gPiBU
aGlzIGFwcGxpZXMgb25seSB0byBTTEVTMTEtU1AxICgyLjYuMzIrKSByaWdodD8KPiA+ID4g
SW4gbWFpbmxpbmUgdGhlIHJlcXVlc3Qgd29uJ3QgYmUgZHJvcHBlZCBiZWNhdXNlIHRoZSBj
YWNoZSBpdGVtIGlzbid0IGFzc3VtZWQKPiA+ID4gdG8gc3RpbGwgYmUgdmFsaWQuCj4gPgo+
ID4gSSBhZ3JlZSwgdGhlcmUgd2lsbCBiZSBubyBkcm9wIGluIG1haW5saW5lLgo+ID4KPiA+
IEJ1dCBpbiBtYWlubGluZSB0aGVyZSB3aWxsIGJlIGEgdXNlbGVzcyB1cGNhbGwuIFRoYXQg
dXBjYWxsIG1pZ2h0IGV2ZW4gYmUKPiA+IHByb2Nlc3NlZCBieSB0aGUgcmVhZGVyLCBpZiBj
YWNoZV9jbGVhbiBpc24ndCBmYXN0ZXIgdGhlbiB0aGUgcmVhZGVyLgo+ID4gVGhlIGFuc3dl
ciBmcm9tIHRoZSByZWFkZXIgd2lsbCBhZ2FpbiByZXBsYWNlIHRoZSBjdXJyZW50IGNhY2hl
IGVudHJ5Cj4gPiAod2hpY2ggaXMganVzdCBhIGZldyBtaWNyb3NlY29uZHMgb2xkKSBieSBh
IG5ldyBvbmUuIFRoZSBuZXcgb25lIG11c3QgYmUKPiA+IGFsbG9jYXRlZCwgdGhlIHJlcGxh
Y2VkIG9uZSBtdXN0IGxhdGVyIGJlIGNsZWFuZWQuIFRoZSBzaW5nbGUgaXRlbSBjYWNoZQo+
ID4gaXMgaW52YWxpZGF0ZWQgYWdhaW4uCj4gPgo+ID4gSW4gd29yc3QgY2FzZSwgdGhlIHVu
bmVjZXNzYXJ5IHJlcGxhY2VtZW50IGFsc28gY291bGQgdHJpZ2dlciB0aGUgbmV4dAo+ID4g
cm91bmQgb2YgdGhlIGdhbWUgLi4uCj4gPgo+ID4gU28gaW4gbXkgb3BpbmlvbiBpdCB3b3Vs
ZCBiZSBiZXR0ZXIgdG8gYWRkIHRoZSBwYXRjaCB5b3Ugc3VnZ2VzdGVkIGJlbG93Cj4gPiB0
byBtYWlubGluZSBhbHNvLgo+ID4KPiA+ID4KPiA+ID4gU28gd2UgbmVlZCB0byBtYWtlIHN1
cmUgdGhhdCBzdW5ycGNfY2FjaGVfcGlwZV91cGNhbGwgZG9lc24ndCBtYWtlIGFuIHVwY2Fs
bAo+ID4gPiBvbiBhIGNhY2hlIGl0ZW0gdGhhdCBoYXMgYmVlbiByZXBsYWNlZC4gIEknZCBy
YXRoZXIgbm90IHVzZSB0aGUgQ0FDSEVfQ0xFQU4KPiA+ID4gYml0ICh3aGV0aGVyIHJlbmFt
ZWQgb3Igbm90KSBhcyBpdCBoYXMgYSB3ZWxsIGRlZmluZWQgbWVhbmluZyAiaGFzIGJlZW4K
PiA+ID4gcmVtb3ZlZCBmcm9tIGNhY2hlIiBhbmQgSSdkIHJhdGhlciBub3QgYmx1ciB0aGF0
IG1lYW5pbmcuCj4gPiA+IFdlIGFscmVhZHkgaGF2ZSBhIHN0YXRlIHRoYXQgbWVhbnMgInRo
aXMgaGFzIGJlZW4gcmVwbGFjZSItIC0+ZXhwaXJ5X3RpbWUgaXMKPiA+ID4gMC4KPiA+ID4g
U28gaG93IGFib3V0IGFkZGluZwo+ID4gPiAgICBpZiAoaC0+ZXhwaXJ5X3RpbWUgPT0gMCkK
PiA+ID4gICAgICAgICAgcmV0dXJuIC1FQUdBSU47Cj4gPiA+IHRvIHN1bnJwY19jYWNoZV9w
aXBlX3VwY2FsbCgpIGluIFNMRVMxMS1TUDEuCj4gPiA+Cj4gPiA+IERvZXMgdGhhdCB3b3Jr
IGZvciB5b3U/Cj4gPgo+ID4gWWVzLCB0aGF0IGxvb2tzIGdvb2QuIE15IHRlc3Qgd2l0aCB0
aGlzIGZpeCBpcyBydW5uaW5nIHN1Y2Nlc3NmdWxseQo+ID4gc2luY2UgNSBob3Vycy4gSSds
bCBsZXQgaXQgcnVuIHVudGlsIE1vbmRheS4KPgo+IEFwb2xvZ2llcywgSSd2ZSBjb21wbGV0
ZWx5IGxvc3QgdHJhY2sgb2YgdGhpcyB0aHJlYWQ6IGRvIHdlIGtub3cgd2hhdAo+IG1haW5s
aW5lIG5lZWRzIG5vdz8KPgo+IC0tYi4KPgoKVGhlcmUgaXMgbm8gcmVhc29uIGZvciBhcG9s
b2dpZXMuIFRoZSB0aHJlYWQgbWVhbndoaWxlIHNlZW1zIHRvIGJlIGEgYml0CmNvbmZ1c2lu
ZyA6LSkKCkN1cnJlbnQgc3RhdGUgaXM6CgotIE5laWwgQnJvd24gaGFzIGNyZWF0ZWQgdHdv
IHNlcmllcyBvZiBwYXRjaGVzLiBPbmUgZm9yIFNMRVMxMS1TUDEgYW5kIGEKICBzZWNvbmQg
b25lIGZvciAtU1AyCgotIEFGQUlDUywgdGhlIHNlcmllcyBmb3IgLVNQMiB3aWxsIG1hdGNo
IHdpdGggbWFpbmxpbmUgYWxzby4KCi0gVG9kYXkgSSBmb3VuZCBhbmQgZml4ZWQgdGhlICho
b3BlZnVsbHkpIGxhc3QgcHJvYmxlbSBpbiB0aGUgLVNQMSBzZXJpZXMuCiAgTXkgdGVzdCB1
c2luZyB0aGlzIHBhdGNoc2V0IHdpbGwgcnVuIHVudGlsIE1vbmRheS4KCi0gUHJvdmlkZWQg
dGhlIHRlc3Qgb24gU1AxIHN1Y2NlZWRzLCBwcm9iYWJseSBvbiBUdWVzZGF5IEknbGwgc3Rh
cnQgdG8gdGVzdAogIHRoZSBwYXRjaGVzIGZvciBTUDIgKGFuZCBtYWlubGluZSkuIElmIGl0
IHJ1bnMgZmluZSwgd2UnbGwgaGF2ZSBhIHRlc3RlZAogIHBhdGNoc2V0IG5vdCBsYXRlciB0
aGFuIE1vbiAxNXRoLgoKQm9kbwo=

Re: sunrpc/cache.c: races while updating cache entries

[J. Bruce Fields]

On Thu, Mar 21, 2013 at 05:41:35PM +0100, Bodo Stroesser wrote:
> On 21 Mar 2013 00:34:00 +0100 NeilBrown <neilb@suse.de> wrote:
> > This applies only to SLES11-SP1 (2.6.32+) right?
> > In mainline the request won't be dropped because the cache item isn't assumed
> > to still be valid.
> 
> I agree, there will be no drop in mainline.
> 
> But in mainline there will be a useless upcall. That upcall might even be
> processed by the reader, if cache_clean isn't faster then the reader.
> The answer from the reader will again replace the current cache entry
> (which is just a few microseconds old) by a new one. The new one must be
> allocated, the replaced one must later be cleaned. The single item cache
> is invalidated again.
> 
> In worst case, the unnecessary replacement also could trigger the next
> round of the game ...
> 
> So in my opinion it would be better to add the patch you suggested below
> to mainline also.
> 
> > 
> > So we need to make sure that sunrpc_cache_pipe_upcall doesn't make an upcall
> > on a cache item that has been replaced.  I'd rather not use the CACHE_CLEAN
> > bit (whether renamed or not) as it has a well defined meaning "has been
> > removed from cache" and I'd rather not blur that meaning.
> > We already have a state that means "this has been replace"- ->expiry_time is
> > 0.
> > So how about adding
> >    if (h->expiry_time == 0)
> >          return -EAGAIN;
> > to sunrpc_cache_pipe_upcall() in SLES11-SP1.
> > 
> > Does that work for you?
> 
> Yes, that looks good. My test with this fix is running successfully
> since 5 hours. I'll let it run until Monday.

Apologies, I've completely lost track of this thread: do we know what
mainline needs now?

--b.

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMjEgTWFyIDIwMTMgMDA6MzQ6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRl
PiB3cm90ZToKCj4gT24gMjAgTWFyIDIwMTMgMTk6NDU6NDggKzAxMDAgQm9kbyBTdHJvZXNz
ZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+Cj4gd3JvdGU6Cj4gCj4gPiBPbiAxOSBN
YXIgMjAxMyAwNDoyNDowMCArMDEwMCBOZWlsQnJvd24gPG5laWxiQHN1c2UuZGU+IHdyb3Rl
Ogo+ID4gCj4gPiA+IE9uIDE0IE1hciAyMDEzIDE4OjMxOjM4ICswMTAwIEJvZG8gU3Ryb2Vz
c2VyIDxic3Ryb2Vzc2VyQHRzLmZ1aml0c3UuY29tPgo+ID4gPiB3cm90ZToKPiA+ID4gCj4g
PiA+ID4gT24gMTMgTWFyIDIwMTMgMDc6NTU6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBz
dXNlLmRlPiB3cm90ZToKPiA+ID4gPiAKPiA+ID4gPiA+IE9uIDExIE1hciAyMDEzIDE3OjEz
OjQxICswMTAwIEJvZG8gU3Ryb2Vzc2VyIDxic3Ryb2Vzc2VyQHRzLmZ1aml0c3UuY29tPgo+
ID4gPiA+ID4gd3JvdGU6Cj4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gSGksCj4gPiA+ID4gPiA+
IAo+ID4gPiA+ID4gPiBBRkFJQ1MsIHRoZXJlIGlzIG9uZSBtb3JlIHJhY2UgaW4gUlBDIGNh
Y2hlLgo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gVGhlIHByb2JsZW0gc2hvd2VkIHVwIGZv
ciB0aGUgImF1dGgudW5peC5pcCIgY2FjaGUsIHRoYXQKPiA+ID4gPiA+ID4gaGFzIGEgcmVh
ZGVyLgo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gSWYgYSBzZXJ2ZXIgdGhyZWFkIHRyaWVz
IHRvIGZpbmQgYSBjYWNoZSBlbnRyeSwgaXQgZmlyc3QKPiA+ID4gPiA+ID4gZG9lcyBhIGxv
b2t1cCAob3IgY2FsbHMgaXBfbWFwX2NhY2hlZF9nZXQoKSBpbiB0aGlzIHNwZWNpZmljCj4g
PiA+ID4gPiA+IGNhc2UpLiBUaGVuLCBpdCBjYWxscyBjYWNoZV9jaGVjaygpIGZvciB0aGlz
IGVudHJ5Lgo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gSWYgdGhlIHJlYWRlciB1cGRhdGVz
IHRoZSBjYWNoZSBlbnRyeSBqdXN0IGJldHdlZW4gdGhlCj4gPiA+ID4gPiA+IHRocmVhZCdz
IGxvb2t1cCBhbmQgY2FjaGVfY2hlY2soKSBleGVjdXRpb24sIGNhY2hlX2NoZWNrKCkKPiA+
ID4gPiA+ID4gd2lsbCBkbyBhbiB1cGNhbGwgZm9yIHRoaXMgY2FjaGUgZW50cnkuIFRoaXMg
aXMsIGJlY2F1c2UKPiA+ID4gPiA+ID4gc3VucnBjX2NhY2hlX3VwZGF0ZSgpIGNhbGxzIGNh
Y2hlX2ZyZXNoX2xvY2tlZChvbGQsIDApLAo+ID4gPiA+ID4gPiB3aGljaCBzZXRzIGV4cGly
eV90aW1lIHRvIDAuCj4gPiA+ID4gPiA+IAo+ID4gPiA+ID4gPiBVbmZvcnR1bmF0ZWx5LCB0
aGUgcmVwbHkgdG8gdGhlIHVwY2FsbCB3aWxsIG5vdCBkZXF1ZXVlCj4gPiA+ID4gPiA+IHRo
ZSBxdWV1ZWQgY2FjaGVfcmVxdWVzdCwgYXMgdGhlIHJlcGx5IHdpbGwgYmUgYXNzaWduZWQg
dG8KPiA+ID4gPiA+ID4gdGhlIGNhY2hlIGVudHJ5IG5ld2x5IGNyZWF0ZWQgYnkgdGhlIGFi
b3ZlIGNhY2hlIHVwZGF0ZS4KPiA+ID4gPiA+ID4gCj4gPiA+ID4gPiA+IFRoZSByZXN1bHQg
aXMgYSBncm93aW5nIHF1ZXVlIG9mIG9ycGhhbmVkIGNhY2hlX3JlcXVlc3QKPiA+ID4gPiA+
ID4gc3RydWN0dXJlcyAtLT4gbWVtb3J5IGxlYWsuCj4gPiA+ID4gPiA+IAo+ID4gPiA+ID4g
PiBJIGZvdW5kIHRoaXMgb24gYSBTTEVTMTEgU1AxIHdpdGggYSBiYWNrcG9ydCBvZiB0aGUg
bGF0ZXN0Cj4gPiA+ID4gPiA+IHBhdGNoZXMgdGhhdCBmaXggdGhlIG90aGVyIFJQQyByYWNl
cy4gT24gdGhpcyBvbGQga2VybmVsLAo+ID4gPiA+ID4gPiB0aGUgcHJvYmxlbSBhbHNvIGxl
YWRzIHRvIHN2Y19kcm9wKCkgYmVpbmcgY2FsbGVkIGZvciB0aGUKPiA+ID4gPiA+ID4gYWZm
ZWN0ZWQgUlBDIHJlcXVlc3QgKGFmdGVyIHN2Y19kZWZlcigpKS4KPiA+ID4gPiA+ID4gCj4g
PiA+ID4gPiA+IEJlc3QgUmVnYXJkcwo+ID4gPiA+ID4gPiBCb2RvCj4gPiA+ID4gPiAKPiA+
ID4gPiA+IEkgZG9uJ3QgdGhpbmsgdGhpcyBpcyBhIHJlYWwgcHJvYmxlbS4KPiA+ID4gPiA+
IFRoZSBwZXJpb2RpYyBjYWxsIHRvICJjYWNoZV9jbGVhbiIgc2hvdWxkIGZpbmQgdGhlc2Ug
b3JwaGFuZWQgcmVxdWVzdHMgYW5kCj4gPiA+ID4gPiBwdXJnZSB0aGVtLiAgU28geW91IGNv
dWxkIGdldCBhIHNob3J0IHRlcm0gbWVtb3J5IGxlYWssIGJ1dCBpdCBzaG91bGQKPiA+ID4g
PiA+IGNvcnJlY3QgaXRzZWxmLgo+ID4gPiA+ID4gRG8geW91IGFncmVlPwo+ID4gPiA+ID4g
Cj4gPiA+ID4gPiBUaGFua3MsCj4gPiA+ID4gPiBOZWlsQnJvd24KPiA+ID4gPiAKPiA+ID4g
PiBNZWFud2hpbGUgSSBmb3VuZCB0aGUgbWlzc2luZyBwYXJ0IG9mIHRoZSByYWNlIQo+ID4g
PiA+IAo+ID4gPiA+IEl0J3MganVzdCB3aGF0IEkgd3JvdGUgYWJvdmUgYnV0IGFkZGl0aW9u
YWxseSwgaW1tZWRpYXRlbHkgYWZ0ZXIKPiA+ID4gPiB0aGUgcmVhZGVyIHVwZGF0ZWQgdGhl
IGNhY2hlLCBjYWNoZV9jbGVhbigpIHVuaGFzaGVzIHRoZSBvbGQgY2FjaGUKPiA+ID4gPiBl
bnRyeS4gSW4gdGhhdCBjYXNlOgo+ID4gPiA+IDEpIHRoZSB0aHJlYWQgd2lsbCBxdWV1ZSBh
IHJlcXVlc3QgZm9yIGEgY2FjaGUgZW50cnksIHRoYXQgaXNuJ3QKPiA+ID4gPiAgICBpbiB0
aGUgaGFzaCBsaXN0cy4KPiA+ID4gPiAyKSBjYWNoZV9jbGVhbigpIG5ldmVyIHdpbGwgYWNj
ZXNzIHRoaXMgb2xkIGNhY2hlIGVudHJ5IGFnYWluCj4gPiA+ID4gMykgZXZlcnkgZnVydGhl
ciBjYWNoZSB1cGRhdGUgd2lsbCBjYWxsIGNhY2hlX2RlcXVldWUoKSBmb3IgYSBuZXdlcgo+
ID4gPiA+ICAgIGNhY2hlIGVudHJ5LCB0aGUgcmVxdWVzdCBpcyBuZXZlciBkZXF1ZXVlZAo+
ID4gPiA+IAo+ID4gPiA+IC0tPiBtZW1vcnkgbGVhay4KPiA+ID4gCj4gPiA+IFllcywgSSBz
ZWUgdGhhdCBub3cuICBUaGFua3MuCj4gPiA+IAo+ID4gPiBJIHN1c3BlY3QgdGhhdCBidWcg
d2FzIGludHJvZHVjZWQgYnkgY29tbWl0IDNhZjQ5NzRlYjJjNzg2N2Q2ZTE2Lgo+ID4gPiBQ
cmlvciB0byB0aGVuLCBjYWNoZV9jbGVhbiB3b3VsZCBuZXZlciByZW1vdmUgYW55dGhpbmcg
d2l0aCBhbiBhY3RpdmUKPiA+ID4gcmVmZXJlbmNlLiAgSWYgc29tZXRoaW5nIHdhcyBhYm91
dCB0byBnZXQgQ0FDSEVfUEVORElORywgaXQgd291bGQgaGF2ZSBhCj4gPiA+IHJlZmVyZW5j
ZSBzbyBjYWNoZV9jbGVhbiB3b3VsZCBsZWF2ZSBpdCBhbG9uZS4KPiA+ID4gCj4gPiA+IEkg
d2FudGVkIHRvIGZpeCB0aGlzIGJ5IGdldHRpbmcgdGhlIGxhc3QgJ3B1dCcgdG8gY2FsbCBj
YWNoZV9kZXF1ZXVlKCkgaWYKPiA+ID4gQ0FDSEVfUEVORElORyB3YXMgc3RpbGwgc2V0LiAg
SG93ZXZlciBJIGNvdWxkbid0IGdldCBhY2Nlc3MgdG8gdGhlCj4gPiA+IENBQ0hFX1BFTkRJ
TkcgZmxhZyBhbmQgdGhlIGNhY2hlX2RldGFpbCBhdCB0aGUgc2FtZSBwbGFjZSAtIHNvIEkg
Z2F2ZSB1cCBvbgo+ID4gPiB0aGF0Lgo+ID4gPiAKPiA+ID4gVGhlIHBhdGNoIEkgaGF2ZSBp
bmNsdWRlZCBiZWxvdyBzZXRzIGEgZmxhZyB3aGVuIGFuIGNhY2hlIGl0ZW0gaXMgcmVtb3Zl
ZAo+ID4gPiBmcm9tIHRoZSBjYWNoZSAoYnkgY2FjaGVfY2xlYW4pIGFuZCByZWZ1c2VzIHRv
IGxvZGdlIGFuIHVwY2FsbCBpZiB0aGUgZmxhZyBpcwo+ID4gPiBzZXQuICBUaGF0IHdpbGwg
ZW5zdXJlIHRoZXJlIGlzIG5ldmVyIGEgcGVuZGluZyB1cGNhbGwgd2hlbiB0aGUgaXRlbSBp
cwo+ID4gPiBmaW5hbGx5IGZyZWVkLgo+ID4gPiAKPiA+ID4gWW91IHBhdGNoIG9ubHkgYWRk
cmVzc2VzIHRoZSBwYXJ0aWN1bGFyIHNpdHVhdGlvbiB0aGF0IHlvdSBoYWQgZm91bmQuICBJ
Cj4gPiA+IHRoaW5rIGl0IGlzIHBvc3NpYmxlIHRoYXQgdGhlcmUgbWlnaHQgYmUgb3RoZXIg
c2l0dWF0aW9ucyB0aGF0IGFsc28gbGVhZCB0bwo+ID4gPiBtZW1vcnkgbGVha3MsIHNvIEkg
d2FudGVkIGEgc29sdXRpb24gdGhhdCB3b3VsZCBndWFyYW50ZWUgdGhhdCB0aGVyZSBjb3Vs
ZG4ndAo+ID4gPiBiZSBhIGxlYWsuCj4gPiA+IAo+ID4gPiA+IAo+ID4gPiA+IEkgdmVyaWZp
ZWQgdGhpcyBpbnNlcnRpbmcgc29tZSBkZWJ1ZyBpbnN0cnVjdGlvbnMuIEluIGEgdGVzdHJ1
bgo+ID4gPiA+IHRoYXQgdHJpZ2dlcmVkIDYgdGltZXMsIGFuZCB0aGUgcXVldWUgcHJpbnRl
ZCBieSBjcmFzaCBhZnRlciB0aGUKPiA+ID4gPiBydW4gaGFkIDYgb3JwaGFuZWQgZW50cmll
cy4KPiA+ID4gPiAKPiA+ID4gPiBBcyBJIHdyb3RlIHllc3RlcmRheSwgSSBoYXZlIGEgcGF0
Y2ggdGhhdCBzb2x2ZXMgdGhlIHByb2JsZW0gYnkKPiA+ID4gPiByZXRlc3RpbmcgdGhlIHN0
YXRlIG9mIHRoZSBjYWNoZSBlbnRyeSBhZnRlciBzZXR0aW5nIENBQ0hFX1BFTkRJTkcKPiA+
ID4gPiBpbiBjYWNoZV9jaGVjaygpLiBJIGNhbiBzZW5kIGl0IGlmIHlvdSBsaWtlLgo+ID4g
PiA+IAo+ID4gPiA+IEJvZG8KPiA+ID4gPiAKPiA+ID4gPiBQLlMuOgo+ID4gPiA+IE1heWJl
IEknbSB3cm9uZywgYnV0IEFGQUlDUywgdGhlcmUgYXJlIHR3byBmdXJ0aGVyIG1pbm9yIHBy
b2JsZW1zCj4gPiA+ID4gcmVnYXJkaW5nIChuZWFybHkpIGV4cGlyZWQgY2FjaGUgZW50cmll
czoKPiA+ID4gPiBhKSBpcF9tYXBfY2FjaGVkX2dldCgpIGluIHNvbWUgc2l0dWF0aW9ucyBj
YW4gcmV0dXJuIGFuIGFscmVhZHkKPiA+ID4gPiAgICBvdXRkYXRlZCAoaXQgdXNlcyBjYWNo
ZV92YWxpZCB0aGF0IG5vdCBmdWxseSBjaGVja3MgdGhlIHN0YXRlKQo+ID4gPiA+ICAgIGNh
Y2hlIGVudHJ5LiBJZiBjYWNoZV9jaGVjaygpIGlzIGNhY2xsZWQgZm9yIHRoYXQgZW50cnks
IGl0IHdpbGwKPiA+ID4gPiAgICBmYWlsLCBJIHRoaW5rCj4gPiA+ID4gYikgR2VuZXJhbGx5
LCBpZiBhIGNhY2hlIGVudHJ5IGlzIHJldHVybmVkIGJ5IHN1bnJwY19jYWNoZV9sb29rdXAo
KQo+ID4gPiA+ICAgIGFuZCB0aGUgZW50cnkgaXMgaW4gdGhlIGxhc3Qgc2Vjb25kIG9mIGl0
J3MgZXhwaXJ5X3RpbWUsIHRoZSAKPiA+ID4gPiAgICBjbG9jayBtaWdodCBtb3ZlIHRvIHRo
ZSBuZXh0IHNlY29uZCBiZWZvcmUgY2FjaGVfY2hlY2sgaXMgY2FsbGVkLgo+ID4gPiA+ICAg
IElmIHRoaXMgaGFwcGVucywgY2FjaGVfY2hlY2sgd2lsbCBmYWlsLCBJIHRoaW5rLgo+ID4g
PiA+IERvIHlvdSBhZ3JlZT8KPiA+ID4gCj4gPiA+IFllcywgYnV0IEknbSBub3Qgc3VyZSBo
b3cgaW1wb3J0YW50IHRoaXMgaXMuCj4gPiA+IE5vcm1hbGx5IGNhY2hlIGVudHJpZXMgc2hv
dWxkIGJlIHJlZnJlc2hlZCB3ZWxsIGJlZm9yZSB0aGV5IGV4cGlyZSwgc28gd2UKPiA+ID4g
c2hvdWxkIG5vcm1hbGx5IGZpbmQgYW4gZW50cnkgd2l0aCBtb3JlIHRoYW4gaGFsZiBvZiBp
dHMgbGlmZXRpbWUgbGVmdC4KPiA+ID4gCj4gPiA+IEluIHRoZSByYXJlIGNhc2Ugd2hlcmUg
dGhlIHNjZW5hcmlvIHlvdSBkZXNjcmliZSBoYXBwZW5zLCBjYWNoZV9jaGVjaygpIHdpbGwK
PiA+ID4gcmV0dXJuIC1FVElNRURPVVQuCj4gPiA+IEluIG1haW5saW5lLCB0aGF0IHdpbGwg
Y2F1c2UgdGhlIHJlcXVlc3QgdG8gYmUgZHJvcHBlZCBhbmQgdGhlIGNvbm5lY3Rpb24KPiA+
ID4gY2xvc2VkIHNvIHRoZSBjbGllbnQgd2lsbCB0cnkgYWdhaW4gYW5kIHdvbid0IGhpdCBh
bnkgcmFjZSBhbmQgc28gc2hvdWxkIGdldAo+ID4gPiBhIGNvcnJlY3QgcmVzdWx0Lgo+ID4g
PiBJbiBTTEVTMTFTUDEsIHdlIHJldHJ5IHRoZSBsb29rdXAgYW5kIHdpbGwgaG9wZWZ1bGx5
IGdldCBhIGJldHRlciByZXN1bHQKPiA+ID4gd2l0aG91dCBoYXZpbmcgdG8gY2xvc2UgdGhl
IGNvbm5lY3Rpb24uCj4gPiA+IAo+ID4gPiBTbyB0aGlzIHNob3VsZCBiZSByYXJlIGFuZCBz
aG91bGQgZmFpbC1zYWZlLgo+ID4gPiAKPiA+ID4gRG9lcyB0aGF0IGFncmVlIHdpdGggeW91
ciB1bmRlcnN0YW5kaW5nPwo+ID4gPiAKPiA+ID4gCj4gPiA+IFRoYW5rcywKPiA+ID4gTmVp
bEJyb3duCj4gPiA+IAo+ID4gPiAKPiA+ID4gRnJvbSBlNzZlNjU4MzQwNWEzYjVmZjljOGQy
YWUxMTg0NzA0ZWZiMjA5ZWYwIE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQo+ID4gPiBGcm9t
OiBOZWlsQnJvd24gPG5laWxiQHN1c2UuZGU+Cj4gPiA+IERhdGU6IFR1ZSwgMTkgTWFyIDIw
MTMgMTM6NTg6NTggKzExMDAKPiA+ID4gU3ViamVjdDogW1BBVENIXSBzdW5ycGMvY2FjaGU6
IGVuc3VyZSBpdGVtcyByZW1vdmVkIGZyb20gY2FjaGUgZG8gbm90IGhhdmUKPiA+ID4gIHBl
bmRpbmcgdXBjYWxscy4KPiA+ID4gCj4gPiA+IEl0IGlzIHBvc3NpYmxlIGZvciBhIHJhY2Ug
dG8gc2V0IENBQ0hFX1BFTkRJTkcgYWZ0ZXIgY2FjaGVfY2xlYW4oKQo+ID4gPiBoYXMgcmVt
b3ZlZCBhIGNhY2hlIGVudHJ5IGZyb20gdGhlIGNhY2hlLgo+ID4gPiBJZiBDQUNIRV9QRU5E
SU5HIGlzIHN0aWxsIHNldCB3aGVuIHRoZSBlbnRyeSBpcyBmaW5hbGx5ICdwdXQnLAo+ID4g
PiB0aGUgY2FjaGVfZGVxdWV1ZSgpIHdpbGwgbmV2ZXIgaGFwcGVuIGFuZCB3ZSBjYW4gbGVh
ayBtZW1vcnkuCj4gPiA+IAo+ID4gPiBTbyBzZXQgYSBuZXcgZmxhZyAnQ0FDSEVfQ0xFQU5F
RCcgd2hlbiB3ZSByZW1vdmUgc29tZXRoaW5nIGZyb20KPiA+ID4gdGhlIGNhY2hlLCBhbmQg
ZG9uJ3QgcXVldWUgYW5kIHVwY2FsbCBpZiBpdCBpcyBzZXQuCj4gPiA+IAo+ID4gPiBJZiBD
QUNIRV9QRU5ESU5HIGlzIHNldCBiZWZvcmUgQ0FDSEVfQ0xFQU5FRCwgdGhlIGNhbGwgdGhh
dAo+ID4gPiBjYWNoZV9jbGVhbigpIG1ha2VzIHRvIGNhY2hlX2ZyZXNoX3VubG9ja2VkKCkg
d2lsbCBmcmVlIG1lbW9yeQo+ID4gPiBhcyBuZWVkZWQuICBJZiBDQUNIRV9QRU5ESU5HIGlz
IHNldCBhZnRlciBDQUNIRV9DTEVBTkVELCB0aGUKPiA+ID4gdGVzdCBpbiBzdW5ycGNfY2Fj
aGVfcGlwZV91cGNhbGwgd2lsbCBlbnN1cmUgdGhhdCB0aGUgbWVtb3J5Cj4gPiA+IGlzIG5v
dCBhbGxvY2F0ZWQuCj4gPiA+IAo+ID4gPiBSZXBvcnRlZC1ieTogPGJzdHJvZXNzZXJAdHMu
ZnVqaXRzdS5jb20+Cj4gPiA+IFNpZ25lZC1vZmYtYnk6IE5laWxCcm93biA8bmVpbGJAc3Vz
ZS5kZT4KPiA+ID4gCj4gPiA+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9j
YWNoZS5oIGIvaW5jbHVkZS9saW51eC9zdW5ycGMvY2FjaGUuaAo+ID4gPiBpbmRleCAzMDMz
OTliLi44NDE5ZjdkIDEwMDY0NAo+ID4gPiAtLS0gYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9j
YWNoZS5oCj4gPiA+ICsrKyBiL2luY2x1ZGUvbGludXgvc3VucnBjL2NhY2hlLmgKPiA+ID4g
QEAgLTU3LDYgKzU3LDcgQEAgc3RydWN0IGNhY2hlX2hlYWQgewo+ID4gPiAgI2RlZmluZQlD
QUNIRV9WQUxJRAkwCS8qIEVudHJ5IGNvbnRhaW5zIHZhbGlkIGRhdGEgKi8KPiA+ID4gICNk
ZWZpbmUJQ0FDSEVfTkVHQVRJVkUJMQkvKiBOZWdhdGl2ZSBlbnRyeSAtIHRoZXJlIGlzIG5v
IG1hdGNoIGZvciB0aGUga2V5ICovCj4gPiA+ICAjZGVmaW5lCUNBQ0hFX1BFTkRJTkcJMgkv
KiBBbiB1cGNhbGwgaGFzIGJlZW4gc2VudCBidXQgbm8gcmVwbHkgcmVjZWl2ZWQgeWV0Ki8K
PiA+ID4gKyNkZWZpbmUJQ0FDSEVfQ0xFQU5FRAkzCS8qIEVudHJ5IGhhcyBiZWVuIGNsZWFu
ZWQgZnJvbSBjYWNoZSAqLwo+ID4gPiAgCj4gPiA+ICAjZGVmaW5lCUNBQ0hFX05FV19FWFBJ
UlkgMTIwCS8qIGtlZXAgbmV3IHRoaW5ncyBwZW5kaW5nIGNvbmZpcm1hdGlvbiBmb3IgMTIw
IHNlY29uZHMgKi8KPiA+ID4gIAo+ID4gPiBkaWZmIC0tZ2l0IGEvbmV0L3N1bnJwYy9jYWNo
ZS5jIGIvbmV0L3N1bnJwYy9jYWNoZS5jCj4gPiA+IGluZGV4IDE2NzdjZmUuLjhlN2NjYmIg
MTAwNjQ0Cj4gPiA+IC0tLSBhL25ldC9zdW5ycGMvY2FjaGUuYwo+ID4gPiArKysgYi9uZXQv
c3VucnBjL2NhY2hlLmMKPiA+ID4gQEAgLTMwNiw3ICszMDYsNyBAQCBFWFBPUlRfU1lNQk9M
X0dQTChjYWNoZV9jaGVjayk7Cj4gPiA+ICAgKiBhIGN1cnJlbnQgcG9pbnRlciBpbnRvIHRo
YXQgbGlzdCBhbmQgaW50byB0aGUgdGFibGUKPiA+ID4gICAqIGZvciB0aGF0IGVudHJ5Lgo+
ID4gPiAgICoKPiA+ID4gLSAqIEVhY2ggdGltZSBjbGVhbl9jYWNoZSBpcyBjYWxsZWQgaXQg
ZmluZHMgdGhlIG5leHQgbm9uLWVtcHR5IGVudHJ5Cj4gPiA+ICsgKiBFYWNoIHRpbWUgY2Fj
aGVfY2xlYW4gaXMgY2FsbGVkIGl0IGZpbmRzIHRoZSBuZXh0IG5vbi1lbXB0eSBlbnRyeQo+
ID4gPiAgICogaW4gdGhlIGN1cnJlbnQgdGFibGUgYW5kIHdhbGtzIHRoZSBsaXN0IGluIHRo
YXQgZW50cnkKPiA+ID4gICAqIGxvb2tpbmcgZm9yIGVudHJpZXMgdGhhdCBjYW4gYmUgcmVt
b3ZlZC4KPiA+ID4gICAqCj4gPiA+IEBAIC00NTMsNiArNDUzLDcgQEAgc3RhdGljIGludCBj
YWNoZV9jbGVhbih2b2lkKQo+ID4gPiAgCQkJY3VycmVudF9pbmRleCArKzsKPiA+ID4gIAkJ
c3Bpbl91bmxvY2soJmNhY2hlX2xpc3RfbG9jayk7Cj4gPiA+ICAJCWlmIChjaCkgewo+ID4g
PiArCQkJc2V0X2JpdChDQUNIRV9DTEVBTkVELCAmY2gtPmZsYWdzKTsKPiA+ID4gIAkJCWNh
Y2hlX2ZyZXNoX3VubG9ja2VkKGNoLCBkKTsKPiA+ID4gIAkJCWNhY2hlX3B1dChjaCwgZCk7
Cj4gPiA+ICAJCX0KPiA+ID4gQEAgLTExNzYsNiArMTE3Nyw5IEBAIGludCBzdW5ycGNfY2Fj
aGVfcGlwZV91cGNhbGwoc3RydWN0IGNhY2hlX2RldGFpbCAqZGV0YWlsLCBzdHJ1Y3QgY2Fj
aGVfaGVhZCAqaCkKPiA+ID4gIAkJd2Fybl9ub19saXN0ZW5lcihkZXRhaWwpOwo+ID4gPiAg
CQlyZXR1cm4gLUVJTlZBTDsKPiA+ID4gIAl9Cj4gPiA+ICsJaWYgKHRlc3RfYml0KENBQ0hF
X0NMRUFORUQsICZoLT5mbGFncykpCj4gPiA+ICsJCS8qIFRvbyBsYXRlIHRvIG1ha2UgYW4g
dXBjYWxsICovCj4gPiA+ICsJCXJldHVybiAtRUFHQUlOOwo+ID4gPiAgCj4gPiA+ICAJYnVm
ID0ga21hbGxvYyhQQUdFX1NJWkUsIEdGUF9LRVJORUwpOwo+ID4gPiAgCWlmICghYnVmKQo+
ID4gPiAKPiA+IAo+ID4gSSBhZGRlZCB0aGlzIHBhdGNoIHRvIG15IFNMRVMxMSBTUDEgYW5k
IHN0aWxsIGhhZCBkcm9wcGVkIFJQQyByZXF1ZXN0cywKPiA+IGJ1dCBubyBsZWFrZWQgbWVt
b3J5Lgo+ID4gCj4gPiBUaGUgZHJvcHMgb2NjdXIsIGJlY2F1c2UgY2FjaGVfY2hlY2soKSBh
bGxvd3MgdG8gbWFrZSB1cGNhbGxzIGZvciBjYWNoZQo+ID4gZW50cmllcywgdGhhdCBhcmUg
c3Vic3RpdHV0ZWQgd2l0aCBuZXcgb25lcyBieSBzdW5ycGNfY2FjaGVfdXBkYXRlKCkuCj4g
PiBJZiBjYWNoZV9jbGVhbigpIGNvbWVzIHRvbyBsYXRlIHRvIHdha2UgdXAgdGhlIHRocmVh
ZCB3YWl0aW5nIGZvciBhbgo+ID4gdXBkYXRlIG9mIHRoZSBvbGQgY2FjaGUgZW50cnksIGEg
dGltZW91dCB3aWxsIG9jY3VyIGFuZCB0aGUgUlBDIHJlcXVlc3QKPiA+IGlzIGRyb3BwZWQu
Cj4gPiAKPiA+IFRoaXMgd2lsbCBub3QgaGFwcGVuIG9uIG1haW5saW5lLCBidXQgd2h5IHNo
b3VsZCB3ZSBhbGxvdyB1cGNhbGxzIHRvIGJlCj4gPiBxdWV1ZWQgZm9yIHJlcGxhY2VkIGNh
Y2hlIGVudHJpZXM/IFRoaXMgd2lsbCBjcmVhdGUgdW5uZWNlc3NhcnkgdHJhZmZpYwo+ID4g
b24gdGhlIGNoYW5uZWwgdG8gdGhlIHJlYWRlciBhbmQgYXMgYSByZXN1bHQgdW5uZWNlc3Nh
cnkgY2FsbHMgdG8KPiA+IHN1bnJwY19jYWNoZV91cGRhdGUoKQo+ID4gCj4gPiBCeSBhZGRp
dGlvbmFsbHkgdG8gdGhlIGFib3ZlIHBhdGNoIGluc2VydGluZwo+ID4gICAgICJzZXRiaXQo
Q0FDSEVfQ0xFQU5FRCwgJm9sZC0+ZmxhZ3MpOyIKPiA+IGludG8gc3VucnBjX2NhY2hlX3Vw
ZGF0ZSgpIGJldHdlZW4gdGhlIHR3byBsaW5lcwo+ID4gICAgICJjYWNoZV9mcmVzaF91bmxv
Y2tlZCh0bXAsIGRldGFpbCk7Igo+ID4gICAgICJjYWNoZV9mcmVzaF91bmxvY2tlZChvbGQs
IGRldGFpbCk7Igo+ID4gdGhpcyB1bmNsZWFuIGJlaGF2aW9yIGNhbiBiZSBhdm9pZGVkLgo+
ID4gCj4gPiBJZiB3ZSBkbyBzbywgbWF5YmUgQ0FDSEVfQ0xFQU4gc2hvdWxkIGJldHRlciBi
ZSBuYW1lZCBDQUNIRV9PTEQgb3IKPiA+IHNvbWV0aGluZyBlbHNlIHRoYXQgcmVmbGVjdHMg
Ym90aCB1c2VzIG9mIHRoZSBiaXQuCj4gPiAKPiA+IERvIHlvdSBhZ3JlZT8KPiAKPiBUaGlz
IGFwcGxpZXMgb25seSB0byBTTEVTMTEtU1AxICgyLjYuMzIrKSByaWdodD8KPiBJbiBtYWlu
bGluZSB0aGUgcmVxdWVzdCB3b24ndCBiZSBkcm9wcGVkIGJlY2F1c2UgdGhlIGNhY2hlIGl0
ZW0gaXNuJ3QgYXNzdW1lZAo+IHRvIHN0aWxsIGJlIHZhbGlkLgoKSSBhZ3JlZSwgdGhlcmUg
d2lsbCBiZSBubyBkcm9wIGluIG1haW5saW5lLgoKQnV0IGluIG1haW5saW5lIHRoZXJlIHdp
bGwgYmUgYSB1c2VsZXNzIHVwY2FsbC4gVGhhdCB1cGNhbGwgbWlnaHQgZXZlbiBiZQpwcm9j
ZXNzZWQgYnkgdGhlIHJlYWRlciwgaWYgY2FjaGVfY2xlYW4gaXNuJ3QgZmFzdGVyIHRoZW4g
dGhlIHJlYWRlci4KVGhlIGFuc3dlciBmcm9tIHRoZSByZWFkZXIgd2lsbCBhZ2FpbiByZXBs
YWNlIHRoZSBjdXJyZW50IGNhY2hlIGVudHJ5Cih3aGljaCBpcyBqdXN0IGEgZmV3IG1pY3Jv
c2Vjb25kcyBvbGQpIGJ5IGEgbmV3IG9uZS4gVGhlIG5ldyBvbmUgbXVzdCBiZQphbGxvY2F0
ZWQsIHRoZSByZXBsYWNlZCBvbmUgbXVzdCBsYXRlciBiZSBjbGVhbmVkLiBUaGUgc2luZ2xl
IGl0ZW0gY2FjaGUKaXMgaW52YWxpZGF0ZWQgYWdhaW4uCgpJbiB3b3JzdCBjYXNlLCB0aGUg
dW5uZWNlc3NhcnkgcmVwbGFjZW1lbnQgYWxzbyBjb3VsZCB0cmlnZ2VyIHRoZSBuZXh0CnJv
dW5kIG9mIHRoZSBnYW1lIC4uLgoKU28gaW4gbXkgb3BpbmlvbiBpdCB3b3VsZCBiZSBiZXR0
ZXIgdG8gYWRkIHRoZSBwYXRjaCB5b3Ugc3VnZ2VzdGVkIGJlbG93CnRvIG1haW5saW5lIGFs
c28uCgo+IAo+IFNvIHdlIG5lZWQgdG8gbWFrZSBzdXJlIHRoYXQgc3VucnBjX2NhY2hlX3Bp
cGVfdXBjYWxsIGRvZXNuJ3QgbWFrZSBhbiB1cGNhbGwKPiBvbiBhIGNhY2hlIGl0ZW0gdGhh
dCBoYXMgYmVlbiByZXBsYWNlZC4gIEknZCByYXRoZXIgbm90IHVzZSB0aGUgQ0FDSEVfQ0xF
QU4KPiBiaXQgKHdoZXRoZXIgcmVuYW1lZCBvciBub3QpIGFzIGl0IGhhcyBhIHdlbGwgZGVm
aW5lZCBtZWFuaW5nICJoYXMgYmVlbgo+IHJlbW92ZWQgZnJvbSBjYWNoZSIgYW5kIEknZCBy
YXRoZXIgbm90IGJsdXIgdGhhdCBtZWFuaW5nLgo+IFdlIGFscmVhZHkgaGF2ZSBhIHN0YXRl
IHRoYXQgbWVhbnMgInRoaXMgaGFzIGJlZW4gcmVwbGFjZSItIC0+ZXhwaXJ5X3RpbWUgaXMK
PiAwLgo+IFNvIGhvdyBhYm91dCBhZGRpbmcKPiAgICBpZiAoaC0+ZXhwaXJ5X3RpbWUgPT0g
MCkKPiAgICAgICAgICByZXR1cm4gLUVBR0FJTjsKPiB0byBzdW5ycGNfY2FjaGVfcGlwZV91
cGNhbGwoKSBpbiBTTEVTMTEtU1AxLgo+IAo+IERvZXMgdGhhdCB3b3JrIGZvciB5b3U/CgpZ
ZXMsIHRoYXQgbG9va3MgZ29vZC4gTXkgdGVzdCB3aXRoIHRoaXMgZml4IGlzIHJ1bm5pbmcg
c3VjY2Vzc2Z1bGx5CnNpbmNlIDUgaG91cnMuIEknbGwgbGV0IGl0IHJ1biB1bnRpbCBNb25k
YXkuCgpUaGFuayB5b3UgZm9yIHlvdXIgaGVscCwKQm9kbwoKPiAKPiBUaGFua3MsCj4gTmVp
bEJyb3duCg==

Re: sunrpc/cache.c: races while updating cache entries

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 10383 bytes --]

On 20 Mar 2013 19:45:48 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
wrote:

> On 19 Mar 2013 04:24:00 +0100 NeilBrown <neilb@suse.de> wrote:
> 
> > On 14 Mar 2013 18:31:38 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
> > wrote:
> > 
> > > On 13 Mar 2013 07:55:00 +0100 NeilBrown <neilb@suse.de> wrote:
> > > 
> > > > On 11 Mar 2013 17:13:41 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
> > > > wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > AFAICS, there is one more race in RPC cache.
> > > > > 
> > > > > The problem showed up for the "auth.unix.ip" cache, that
> > > > > has a reader.
> > > > > 
> > > > > If a server thread tries to find a cache entry, it first
> > > > > does a lookup (or calls ip_map_cached_get() in this specific
> > > > > case). Then, it calls cache_check() for this entry.
> > > > > 
> > > > > If the reader updates the cache entry just between the
> > > > > thread's lookup and cache_check() execution, cache_check()
> > > > > will do an upcall for this cache entry. This is, because
> > > > > sunrpc_cache_update() calls cache_fresh_locked(old, 0),
> > > > > which sets expiry_time to 0.
> > > > > 
> > > > > Unfortunately, the reply to the upcall will not dequeue
> > > > > the queued cache_request, as the reply will be assigned to
> > > > > the cache entry newly created by the above cache update.
> > > > > 
> > > > > The result is a growing queue of orphaned cache_request
> > > > > structures --> memory leak.
> > > > > 
> > > > > I found this on a SLES11 SP1 with a backport of the latest
> > > > > patches that fix the other RPC races. On this old kernel,
> > > > > the problem also leads to svc_drop() being called for the
> > > > > affected RPC request (after svc_defer()).
> > > > > 
> > > > > Best Regards
> > > > > Bodo
> > > > 
> > > > I don't think this is a real problem.
> > > > The periodic call to "cache_clean" should find these orphaned requests and
> > > > purge them.  So you could get a short term memory leak, but it should
> > > > correct itself.
> > > > Do you agree?
> > > > 
> > > > Thanks,
> > > > NeilBrown
> > > 
> > > Meanwhile I found the missing part of the race!
> > > 
> > > It's just what I wrote above but additionally, immediately after
> > > the reader updated the cache, cache_clean() unhashes the old cache
> > > entry. In that case:
> > > 1) the thread will queue a request for a cache entry, that isn't
> > >    in the hash lists.
> > > 2) cache_clean() never will access this old cache entry again
> > > 3) every further cache update will call cache_dequeue() for a newer
> > >    cache entry, the request is never dequeued
> > > 
> > > --> memory leak.
> > 
> > Yes, I see that now.  Thanks.
> > 
> > I suspect that bug was introduced by commit 3af4974eb2c7867d6e16.
> > Prior to then, cache_clean would never remove anything with an active
> > reference.  If something was about to get CACHE_PENDING, it would have a
> > reference so cache_clean would leave it alone.
> > 
> > I wanted to fix this by getting the last 'put' to call cache_dequeue() if
> > CACHE_PENDING was still set.  However I couldn't get access to the
> > CACHE_PENDING flag and the cache_detail at the same place - so I gave up on
> > that.
> > 
> > The patch I have included below sets a flag when an cache item is removed
> > from the cache (by cache_clean) and refuses to lodge an upcall if the flag is
> > set.  That will ensure there is never a pending upcall when the item is
> > finally freed.
> > 
> > You patch only addresses the particular situation that you had found.  I
> > think it is possible that there might be other situations that also lead to
> > memory leaks, so I wanted a solution that would guarantee that there couldn't
> > be a leak.
> > 
> > > 
> > > I verified this inserting some debug instructions. In a testrun
> > > that triggered 6 times, and the queue printed by crash after the
> > > run had 6 orphaned entries.
> > > 
> > > As I wrote yesterday, I have a patch that solves the problem by
> > > retesting the state of the cache entry after setting CACHE_PENDING
> > > in cache_check(). I can send it if you like.
> > > 
> > > Bodo
> > > 
> > > P.S.:
> > > Maybe I'm wrong, but AFAICS, there are two further minor problems
> > > regarding (nearly) expired cache entries:
> > > a) ip_map_cached_get() in some situations can return an already
> > >    outdated (it uses cache_valid that not fully checks the state)
> > >    cache entry. If cache_check() is caclled for that entry, it will
> > >    fail, I think
> > > b) Generally, if a cache entry is returned by sunrpc_cache_lookup()
> > >    and the entry is in the last second of it's expiry_time, the 
> > >    clock might move to the next second before cache_check is called.
> > >    If this happens, cache_check will fail, I think.
> > > Do you agree?
> > 
> > Yes, but I'm not sure how important this is.
> > Normally cache entries should be refreshed well before they expire, so we
> > should normally find an entry with more than half of its lifetime left.
> > 
> > In the rare case where the scenario you describe happens, cache_check() will
> > return -ETIMEDOUT.
> > In mainline, that will cause the request to be dropped and the connection
> > closed so the client will try again and won't hit any race and so should get
> > a correct result.
> > In SLES11SP1, we retry the lookup and will hopefully get a better result
> > without having to close the connection.
> > 
> > So this should be rare and should fail-safe.
> > 
> > Does that agree with your understanding?
> > 
> > 
> > Thanks,
> > NeilBrown
> > 
> > 
> > From e76e6583405a3b5ff9c8d2ae1184704efb209ef0 Mon Sep 17 00:00:00 2001
> > From: NeilBrown <neilb@suse.de>
> > Date: Tue, 19 Mar 2013 13:58:58 +1100
> > Subject: [PATCH] sunrpc/cache: ensure items removed from cache do not have
> >  pending upcalls.
> > 
> > It is possible for a race to set CACHE_PENDING after cache_clean()
> > has removed a cache entry from the cache.
> > If CACHE_PENDING is still set when the entry is finally 'put',
> > the cache_dequeue() will never happen and we can leak memory.
> > 
> > So set a new flag 'CACHE_CLEANED' when we remove something from
> > the cache, and don't queue and upcall if it is set.
> > 
> > If CACHE_PENDING is set before CACHE_CLEANED, the call that
> > cache_clean() makes to cache_fresh_unlocked() will free memory
> > as needed.  If CACHE_PENDING is set after CACHE_CLEANED, the
> > test in sunrpc_cache_pipe_upcall will ensure that the memory
> > is not allocated.
> > 
> > Reported-by: <bstroesser@ts.fujitsu.com>
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > 
> > diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
> > index 303399b..8419f7d 100644
> > --- a/include/linux/sunrpc/cache.h
> > +++ b/include/linux/sunrpc/cache.h
> > @@ -57,6 +57,7 @@ struct cache_head {
> >  #define	CACHE_VALID	0	/* Entry contains valid data */
> >  #define	CACHE_NEGATIVE	1	/* Negative entry - there is no match for the key */
> >  #define	CACHE_PENDING	2	/* An upcall has been sent but no reply received yet*/
> > +#define	CACHE_CLEANED	3	/* Entry has been cleaned from cache */
> >  
> >  #define	CACHE_NEW_EXPIRY 120	/* keep new things pending confirmation for 120 seconds */
> >  
> > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> > index 1677cfe..8e7ccbb 100644
> > --- a/net/sunrpc/cache.c
> > +++ b/net/sunrpc/cache.c
> > @@ -306,7 +306,7 @@ EXPORT_SYMBOL_GPL(cache_check);
> >   * a current pointer into that list and into the table
> >   * for that entry.
> >   *
> > - * Each time clean_cache is called it finds the next non-empty entry
> > + * Each time cache_clean is called it finds the next non-empty entry
> >   * in the current table and walks the list in that entry
> >   * looking for entries that can be removed.
> >   *
> > @@ -453,6 +453,7 @@ static int cache_clean(void)
> >  			current_index ++;
> >  		spin_unlock(&cache_list_lock);
> >  		if (ch) {
> > +			set_bit(CACHE_CLEANED, &ch->flags);
> >  			cache_fresh_unlocked(ch, d);
> >  			cache_put(ch, d);
> >  		}
> > @@ -1176,6 +1177,9 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h)
> >  		warn_no_listener(detail);
> >  		return -EINVAL;
> >  	}
> > +	if (test_bit(CACHE_CLEANED, &h->flags))
> > +		/* Too late to make an upcall */
> > +		return -EAGAIN;
> >  
> >  	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> >  	if (!buf)
> > 
> 
> I added this patch to my SLES11 SP1 and still had dropped RPC requests,
> but no leaked memory.
> 
> The drops occur, because cache_check() allows to make upcalls for cache
> entries, that are substituted with new ones by sunrpc_cache_update().
> If cache_clean() comes too late to wake up the thread waiting for an
> update of the old cache entry, a timeout will occur and the RPC request
> is dropped.
> 
> This will not happen on mainline, but why should we allow upcalls to be
> queued for replaced cache entries? This will create unnecessary traffic
> on the channel to the reader and as a result unnecessary calls to
> sunrpc_cache_update()
> 
> By additionally to the above patch inserting
>     "setbit(CACHE_CLEANED, &old->flags);"
> into sunrpc_cache_update() between the two lines
>     "cache_fresh_unlocked(tmp, detail);"
>     "cache_fresh_unlocked(old, detail);"
> this unclean behavior can be avoided.
> 
> If we do so, maybe CACHE_CLEAN should better be named CACHE_OLD or
> something else that reflects both uses of the bit.
> 
> Do you agree?

This applies only to SLES11-SP1 (2.6.32+) right?
In mainline the request won't be dropped because the cache item isn't assumed
to still be valid.

So we need to make sure that sunrpc_cache_pipe_upcall doesn't make an upcall
on a cache item that has been replaced.  I'd rather not use the CACHE_CLEAN
bit (whether renamed or not) as it has a well defined meaning "has been
removed from cache" and I'd rather not blur that meaning.
We already have a state that means "this has been replace"- ->expiry_time is
0.
So how about adding
   if (h->expiry_time == 0)
         return -EAGAIN;
to sunrpc_cache_pipe_upcall() in SLES11-SP1.

Does that work for you?

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMTkgTWFyIDIwMTMgMDQ6MjQ6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRl
PiB3cm90ZToKCj4gT24gMTQgTWFyIDIwMTMgMTg6MzE6MzggKzAxMDAgQm9kbyBTdHJvZXNz
ZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+Cj4gd3JvdGU6Cj4gCj4gPiBPbiAxMyBN
YXIgMjAxMyAwNzo1NTowMCArMDEwMCBOZWlsQnJvd24gPG5laWxiQHN1c2UuZGU+IHdyb3Rl
Ogo+ID4gCj4gPiA+IE9uIDExIE1hciAyMDEzIDE3OjEzOjQxICswMTAwIEJvZG8gU3Ryb2Vz
c2VyIDxic3Ryb2Vzc2VyQHRzLmZ1aml0c3UuY29tPgo+ID4gPiB3cm90ZToKPiA+ID4gCj4g
PiA+ID4gSGksCj4gPiA+ID4gCj4gPiA+ID4gQUZBSUNTLCB0aGVyZSBpcyBvbmUgbW9yZSBy
YWNlIGluIFJQQyBjYWNoZS4KPiA+ID4gPiAKPiA+ID4gPiBUaGUgcHJvYmxlbSBzaG93ZWQg
dXAgZm9yIHRoZSAiYXV0aC51bml4LmlwIiBjYWNoZSwgdGhhdAo+ID4gPiA+IGhhcyBhIHJl
YWRlci4KPiA+ID4gPiAKPiA+ID4gPiBJZiBhIHNlcnZlciB0aHJlYWQgdHJpZXMgdG8gZmlu
ZCBhIGNhY2hlIGVudHJ5LCBpdCBmaXJzdAo+ID4gPiA+IGRvZXMgYSBsb29rdXAgKG9yIGNh
bGxzIGlwX21hcF9jYWNoZWRfZ2V0KCkgaW4gdGhpcyBzcGVjaWZpYwo+ID4gPiA+IGNhc2Up
LiBUaGVuLCBpdCBjYWxscyBjYWNoZV9jaGVjaygpIGZvciB0aGlzIGVudHJ5Lgo+ID4gPiA+
IAo+ID4gPiA+IElmIHRoZSByZWFkZXIgdXBkYXRlcyB0aGUgY2FjaGUgZW50cnkganVzdCBi
ZXR3ZWVuIHRoZQo+ID4gPiA+IHRocmVhZCdzIGxvb2t1cCBhbmQgY2FjaGVfY2hlY2soKSBl
eGVjdXRpb24sIGNhY2hlX2NoZWNrKCkKPiA+ID4gPiB3aWxsIGRvIGFuIHVwY2FsbCBmb3Ig
dGhpcyBjYWNoZSBlbnRyeS4gVGhpcyBpcywgYmVjYXVzZQo+ID4gPiA+IHN1bnJwY19jYWNo
ZV91cGRhdGUoKSBjYWxscyBjYWNoZV9mcmVzaF9sb2NrZWQob2xkLCAwKSwKPiA+ID4gPiB3
aGljaCBzZXRzIGV4cGlyeV90aW1lIHRvIDAuCj4gPiA+ID4gCj4gPiA+ID4gVW5mb3J0dW5h
dGVseSwgdGhlIHJlcGx5IHRvIHRoZSB1cGNhbGwgd2lsbCBub3QgZGVxdWV1ZQo+ID4gPiA+
IHRoZSBxdWV1ZWQgY2FjaGVfcmVxdWVzdCwgYXMgdGhlIHJlcGx5IHdpbGwgYmUgYXNzaWdu
ZWQgdG8KPiA+ID4gPiB0aGUgY2FjaGUgZW50cnkgbmV3bHkgY3JlYXRlZCBieSB0aGUgYWJv
dmUgY2FjaGUgdXBkYXRlLgo+ID4gPiA+IAo+ID4gPiA+IFRoZSByZXN1bHQgaXMgYSBncm93
aW5nIHF1ZXVlIG9mIG9ycGhhbmVkIGNhY2hlX3JlcXVlc3QKPiA+ID4gPiBzdHJ1Y3R1cmVz
IC0tPiBtZW1vcnkgbGVhay4KPiA+ID4gPiAKPiA+ID4gPiBJIGZvdW5kIHRoaXMgb24gYSBT
TEVTMTEgU1AxIHdpdGggYSBiYWNrcG9ydCBvZiB0aGUgbGF0ZXN0Cj4gPiA+ID4gcGF0Y2hl
cyB0aGF0IGZpeCB0aGUgb3RoZXIgUlBDIHJhY2VzLiBPbiB0aGlzIG9sZCBrZXJuZWwsCj4g
PiA+ID4gdGhlIHByb2JsZW0gYWxzbyBsZWFkcyB0byBzdmNfZHJvcCgpIGJlaW5nIGNhbGxl
ZCBmb3IgdGhlCj4gPiA+ID4gYWZmZWN0ZWQgUlBDIHJlcXVlc3QgKGFmdGVyIHN2Y19kZWZl
cigpKS4KPiA+ID4gPiAKPiA+ID4gPiBCZXN0IFJlZ2FyZHMKPiA+ID4gPiBCb2RvCj4gPiA+
IAo+ID4gPiBJIGRvbid0IHRoaW5rIHRoaXMgaXMgYSByZWFsIHByb2JsZW0uCj4gPiA+IFRo
ZSBwZXJpb2RpYyBjYWxsIHRvICJjYWNoZV9jbGVhbiIgc2hvdWxkIGZpbmQgdGhlc2Ugb3Jw
aGFuZWQgcmVxdWVzdHMgYW5kCj4gPiA+IHB1cmdlIHRoZW0uICBTbyB5b3UgY291bGQgZ2V0
IGEgc2hvcnQgdGVybSBtZW1vcnkgbGVhaywgYnV0IGl0IHNob3VsZAo+ID4gPiBjb3JyZWN0
IGl0c2VsZi4KPiA+ID4gRG8geW91IGFncmVlPwo+ID4gPiAKPiA+ID4gVGhhbmtzLAo+ID4g
PiBOZWlsQnJvd24KPiA+IAo+ID4gTWVhbndoaWxlIEkgZm91bmQgdGhlIG1pc3NpbmcgcGFy
dCBvZiB0aGUgcmFjZSEKPiA+IAo+ID4gSXQncyBqdXN0IHdoYXQgSSB3cm90ZSBhYm92ZSBi
dXQgYWRkaXRpb25hbGx5LCBpbW1lZGlhdGVseSBhZnRlcgo+ID4gdGhlIHJlYWRlciB1cGRh
dGVkIHRoZSBjYWNoZSwgY2FjaGVfY2xlYW4oKSB1bmhhc2hlcyB0aGUgb2xkIGNhY2hlCj4g
PiBlbnRyeS4gSW4gdGhhdCBjYXNlOgo+ID4gMSkgdGhlIHRocmVhZCB3aWxsIHF1ZXVlIGEg
cmVxdWVzdCBmb3IgYSBjYWNoZSBlbnRyeSwgdGhhdCBpc24ndAo+ID4gICAgaW4gdGhlIGhh
c2ggbGlzdHMuCj4gPiAyKSBjYWNoZV9jbGVhbigpIG5ldmVyIHdpbGwgYWNjZXNzIHRoaXMg
b2xkIGNhY2hlIGVudHJ5IGFnYWluCj4gPiAzKSBldmVyeSBmdXJ0aGVyIGNhY2hlIHVwZGF0
ZSB3aWxsIGNhbGwgY2FjaGVfZGVxdWV1ZSgpIGZvciBhIG5ld2VyCj4gPiAgICBjYWNoZSBl
bnRyeSwgdGhlIHJlcXVlc3QgaXMgbmV2ZXIgZGVxdWV1ZWQKPiA+IAo+ID4gLS0+IG1lbW9y
eSBsZWFrLgo+IAo+IFllcywgSSBzZWUgdGhhdCBub3cuICBUaGFua3MuCj4gCj4gSSBzdXNw
ZWN0IHRoYXQgYnVnIHdhcyBpbnRyb2R1Y2VkIGJ5IGNvbW1pdCAzYWY0OTc0ZWIyYzc4Njdk
NmUxNi4KPiBQcmlvciB0byB0aGVuLCBjYWNoZV9jbGVhbiB3b3VsZCBuZXZlciByZW1vdmUg
YW55dGhpbmcgd2l0aCBhbiBhY3RpdmUKPiByZWZlcmVuY2UuICBJZiBzb21ldGhpbmcgd2Fz
IGFib3V0IHRvIGdldCBDQUNIRV9QRU5ESU5HLCBpdCB3b3VsZCBoYXZlIGEKPiByZWZlcmVu
Y2Ugc28gY2FjaGVfY2xlYW4gd291bGQgbGVhdmUgaXQgYWxvbmUuCj4gCj4gSSB3YW50ZWQg
dG8gZml4IHRoaXMgYnkgZ2V0dGluZyB0aGUgbGFzdCAncHV0JyB0byBjYWxsIGNhY2hlX2Rl
cXVldWUoKSBpZgo+IENBQ0hFX1BFTkRJTkcgd2FzIHN0aWxsIHNldC4gIEhvd2V2ZXIgSSBj
b3VsZG4ndCBnZXQgYWNjZXNzIHRvIHRoZQo+IENBQ0hFX1BFTkRJTkcgZmxhZyBhbmQgdGhl
IGNhY2hlX2RldGFpbCBhdCB0aGUgc2FtZSBwbGFjZSAtIHNvIEkgZ2F2ZSB1cCBvbgo+IHRo
YXQuCj4gCj4gVGhlIHBhdGNoIEkgaGF2ZSBpbmNsdWRlZCBiZWxvdyBzZXRzIGEgZmxhZyB3
aGVuIGFuIGNhY2hlIGl0ZW0gaXMgcmVtb3ZlZAo+IGZyb20gdGhlIGNhY2hlIChieSBjYWNo
ZV9jbGVhbikgYW5kIHJlZnVzZXMgdG8gbG9kZ2UgYW4gdXBjYWxsIGlmIHRoZSBmbGFnIGlz
Cj4gc2V0LiAgVGhhdCB3aWxsIGVuc3VyZSB0aGVyZSBpcyBuZXZlciBhIHBlbmRpbmcgdXBj
YWxsIHdoZW4gdGhlIGl0ZW0gaXMKPiBmaW5hbGx5IGZyZWVkLgo+IAo+IFlvdSBwYXRjaCBv
bmx5IGFkZHJlc3NlcyB0aGUgcGFydGljdWxhciBzaXR1YXRpb24gdGhhdCB5b3UgaGFkIGZv
dW5kLiAgSQo+IHRoaW5rIGl0IGlzIHBvc3NpYmxlIHRoYXQgdGhlcmUgbWlnaHQgYmUgb3Ro
ZXIgc2l0dWF0aW9ucyB0aGF0IGFsc28gbGVhZCB0bwo+IG1lbW9yeSBsZWFrcywgc28gSSB3
YW50ZWQgYSBzb2x1dGlvbiB0aGF0IHdvdWxkIGd1YXJhbnRlZSB0aGF0IHRoZXJlIGNvdWxk
bid0Cj4gYmUgYSBsZWFrLgo+IAo+ID4gCj4gPiBJIHZlcmlmaWVkIHRoaXMgaW5zZXJ0aW5n
IHNvbWUgZGVidWcgaW5zdHJ1Y3Rpb25zLiBJbiBhIHRlc3RydW4KPiA+IHRoYXQgdHJpZ2dl
cmVkIDYgdGltZXMsIGFuZCB0aGUgcXVldWUgcHJpbnRlZCBieSBjcmFzaCBhZnRlciB0aGUK
PiA+IHJ1biBoYWQgNiBvcnBoYW5lZCBlbnRyaWVzLgo+ID4gCj4gPiBBcyBJIHdyb3RlIHll
c3RlcmRheSwgSSBoYXZlIGEgcGF0Y2ggdGhhdCBzb2x2ZXMgdGhlIHByb2JsZW0gYnkKPiA+
IHJldGVzdGluZyB0aGUgc3RhdGUgb2YgdGhlIGNhY2hlIGVudHJ5IGFmdGVyIHNldHRpbmcg
Q0FDSEVfUEVORElORwo+ID4gaW4gY2FjaGVfY2hlY2soKS4gSSBjYW4gc2VuZCBpdCBpZiB5
b3UgbGlrZS4KPiA+IAo+ID4gQm9kbwo+ID4gCj4gPiBQLlMuOgo+ID4gTWF5YmUgSSdtIHdy
b25nLCBidXQgQUZBSUNTLCB0aGVyZSBhcmUgdHdvIGZ1cnRoZXIgbWlub3IgcHJvYmxlbXMK
PiA+IHJlZ2FyZGluZyAobmVhcmx5KSBleHBpcmVkIGNhY2hlIGVudHJpZXM6Cj4gPiBhKSBp
cF9tYXBfY2FjaGVkX2dldCgpIGluIHNvbWUgc2l0dWF0aW9ucyBjYW4gcmV0dXJuIGFuIGFs
cmVhZHkKPiA+ICAgIG91dGRhdGVkIChpdCB1c2VzIGNhY2hlX3ZhbGlkIHRoYXQgbm90IGZ1
bGx5IGNoZWNrcyB0aGUgc3RhdGUpCj4gPiAgICBjYWNoZSBlbnRyeS4gSWYgY2FjaGVfY2hl
Y2soKSBpcyBjYWNsbGVkIGZvciB0aGF0IGVudHJ5LCBpdCB3aWxsCj4gPiAgICBmYWlsLCBJ
IHRoaW5rCj4gPiBiKSBHZW5lcmFsbHksIGlmIGEgY2FjaGUgZW50cnkgaXMgcmV0dXJuZWQg
Ynkgc3VucnBjX2NhY2hlX2xvb2t1cCgpCj4gPiAgICBhbmQgdGhlIGVudHJ5IGlzIGluIHRo
ZSBsYXN0IHNlY29uZCBvZiBpdCdzIGV4cGlyeV90aW1lLCB0aGUgCj4gPiAgICBjbG9jayBt
aWdodCBtb3ZlIHRvIHRoZSBuZXh0IHNlY29uZCBiZWZvcmUgY2FjaGVfY2hlY2sgaXMgY2Fs
bGVkLgo+ID4gICAgSWYgdGhpcyBoYXBwZW5zLCBjYWNoZV9jaGVjayB3aWxsIGZhaWwsIEkg
dGhpbmsuCj4gPiBEbyB5b3UgYWdyZWU/Cj4gCj4gWWVzLCBidXQgSSdtIG5vdCBzdXJlIGhv
dyBpbXBvcnRhbnQgdGhpcyBpcy4KPiBOb3JtYWxseSBjYWNoZSBlbnRyaWVzIHNob3VsZCBi
ZSByZWZyZXNoZWQgd2VsbCBiZWZvcmUgdGhleSBleHBpcmUsIHNvIHdlCj4gc2hvdWxkIG5v
cm1hbGx5IGZpbmQgYW4gZW50cnkgd2l0aCBtb3JlIHRoYW4gaGFsZiBvZiBpdHMgbGlmZXRp
bWUgbGVmdC4KPiAKPiBJbiB0aGUgcmFyZSBjYXNlIHdoZXJlIHRoZSBzY2VuYXJpbyB5b3Ug
ZGVzY3JpYmUgaGFwcGVucywgY2FjaGVfY2hlY2soKSB3aWxsCj4gcmV0dXJuIC1FVElNRURP
VVQuCj4gSW4gbWFpbmxpbmUsIHRoYXQgd2lsbCBjYXVzZSB0aGUgcmVxdWVzdCB0byBiZSBk
cm9wcGVkIGFuZCB0aGUgY29ubmVjdGlvbgo+IGNsb3NlZCBzbyB0aGUgY2xpZW50IHdpbGwg
dHJ5IGFnYWluIGFuZCB3b24ndCBoaXQgYW55IHJhY2UgYW5kIHNvIHNob3VsZCBnZXQKPiBh
IGNvcnJlY3QgcmVzdWx0Lgo+IEluIFNMRVMxMVNQMSwgd2UgcmV0cnkgdGhlIGxvb2t1cCBh
bmQgd2lsbCBob3BlZnVsbHkgZ2V0IGEgYmV0dGVyIHJlc3VsdAo+IHdpdGhvdXQgaGF2aW5n
IHRvIGNsb3NlIHRoZSBjb25uZWN0aW9uLgo+IAo+IFNvIHRoaXMgc2hvdWxkIGJlIHJhcmUg
YW5kIHNob3VsZCBmYWlsLXNhZmUuCj4gCj4gRG9lcyB0aGF0IGFncmVlIHdpdGggeW91ciB1
bmRlcnN0YW5kaW5nPwo+IAo+IAo+IFRoYW5rcywKPiBOZWlsQnJvd24KPiAKPiAKPiBGcm9t
IGU3NmU2NTgzNDA1YTNiNWZmOWM4ZDJhZTExODQ3MDRlZmIyMDllZjAgTW9uIFNlcCAxNyAw
MDowMDowMCAyMDAxCj4gRnJvbTogTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRlPgo+IERhdGU6
IFR1ZSwgMTkgTWFyIDIwMTMgMTM6NTg6NTggKzExMDAKPiBTdWJqZWN0OiBbUEFUQ0hdIHN1
bnJwYy9jYWNoZTogZW5zdXJlIGl0ZW1zIHJlbW92ZWQgZnJvbSBjYWNoZSBkbyBub3QgaGF2
ZQo+ICBwZW5kaW5nIHVwY2FsbHMuCj4gCj4gSXQgaXMgcG9zc2libGUgZm9yIGEgcmFjZSB0
byBzZXQgQ0FDSEVfUEVORElORyBhZnRlciBjYWNoZV9jbGVhbigpCj4gaGFzIHJlbW92ZWQg
YSBjYWNoZSBlbnRyeSBmcm9tIHRoZSBjYWNoZS4KPiBJZiBDQUNIRV9QRU5ESU5HIGlzIHN0
aWxsIHNldCB3aGVuIHRoZSBlbnRyeSBpcyBmaW5hbGx5ICdwdXQnLAo+IHRoZSBjYWNoZV9k
ZXF1ZXVlKCkgd2lsbCBuZXZlciBoYXBwZW4gYW5kIHdlIGNhbiBsZWFrIG1lbW9yeS4KPiAK
PiBTbyBzZXQgYSBuZXcgZmxhZyAnQ0FDSEVfQ0xFQU5FRCcgd2hlbiB3ZSByZW1vdmUgc29t
ZXRoaW5nIGZyb20KPiB0aGUgY2FjaGUsIGFuZCBkb24ndCBxdWV1ZSBhbmQgdXBjYWxsIGlm
IGl0IGlzIHNldC4KPiAKPiBJZiBDQUNIRV9QRU5ESU5HIGlzIHNldCBiZWZvcmUgQ0FDSEVf
Q0xFQU5FRCwgdGhlIGNhbGwgdGhhdAo+IGNhY2hlX2NsZWFuKCkgbWFrZXMgdG8gY2FjaGVf
ZnJlc2hfdW5sb2NrZWQoKSB3aWxsIGZyZWUgbWVtb3J5Cj4gYXMgbmVlZGVkLiAgSWYgQ0FD
SEVfUEVORElORyBpcyBzZXQgYWZ0ZXIgQ0FDSEVfQ0xFQU5FRCwgdGhlCj4gdGVzdCBpbiBz
dW5ycGNfY2FjaGVfcGlwZV91cGNhbGwgd2lsbCBlbnN1cmUgdGhhdCB0aGUgbWVtb3J5Cj4g
aXMgbm90IGFsbG9jYXRlZC4KPiAKPiBSZXBvcnRlZC1ieTogPGJzdHJvZXNzZXJAdHMuZnVq
aXRzdS5jb20+Cj4gU2lnbmVkLW9mZi1ieTogTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRlPgo+
IAo+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9jYWNoZS5oIGIvaW5jbHVk
ZS9saW51eC9zdW5ycGMvY2FjaGUuaAo+IGluZGV4IDMwMzM5OWIuLjg0MTlmN2QgMTAwNjQ0
Cj4gLS0tIGEvaW5jbHVkZS9saW51eC9zdW5ycGMvY2FjaGUuaAo+ICsrKyBiL2luY2x1ZGUv
bGludXgvc3VucnBjL2NhY2hlLmgKPiBAQCAtNTcsNiArNTcsNyBAQCBzdHJ1Y3QgY2FjaGVf
aGVhZCB7Cj4gICNkZWZpbmUJQ0FDSEVfVkFMSUQJMAkvKiBFbnRyeSBjb250YWlucyB2YWxp
ZCBkYXRhICovCj4gICNkZWZpbmUJQ0FDSEVfTkVHQVRJVkUJMQkvKiBOZWdhdGl2ZSBlbnRy
eSAtIHRoZXJlIGlzIG5vIG1hdGNoIGZvciB0aGUga2V5ICovCj4gICNkZWZpbmUJQ0FDSEVf
UEVORElORwkyCS8qIEFuIHVwY2FsbCBoYXMgYmVlbiBzZW50IGJ1dCBubyByZXBseSByZWNl
aXZlZCB5ZXQqLwo+ICsjZGVmaW5lCUNBQ0hFX0NMRUFORUQJMwkvKiBFbnRyeSBoYXMgYmVl
biBjbGVhbmVkIGZyb20gY2FjaGUgKi8KPiAgCj4gICNkZWZpbmUJQ0FDSEVfTkVXX0VYUElS
WSAxMjAJLyoga2VlcCBuZXcgdGhpbmdzIHBlbmRpbmcgY29uZmlybWF0aW9uIGZvciAxMjAg
c2Vjb25kcyAqLwo+ICAKPiBkaWZmIC0tZ2l0IGEvbmV0L3N1bnJwYy9jYWNoZS5jIGIvbmV0
L3N1bnJwYy9jYWNoZS5jCj4gaW5kZXggMTY3N2NmZS4uOGU3Y2NiYiAxMDA2NDQKPiAtLS0g
YS9uZXQvc3VucnBjL2NhY2hlLmMKPiArKysgYi9uZXQvc3VucnBjL2NhY2hlLmMKPiBAQCAt
MzA2LDcgKzMwNiw3IEBAIEVYUE9SVF9TWU1CT0xfR1BMKGNhY2hlX2NoZWNrKTsKPiAgICog
YSBjdXJyZW50IHBvaW50ZXIgaW50byB0aGF0IGxpc3QgYW5kIGludG8gdGhlIHRhYmxlCj4g
ICAqIGZvciB0aGF0IGVudHJ5Lgo+ICAgKgo+IC0gKiBFYWNoIHRpbWUgY2xlYW5fY2FjaGUg
aXMgY2FsbGVkIGl0IGZpbmRzIHRoZSBuZXh0IG5vbi1lbXB0eSBlbnRyeQo+ICsgKiBFYWNo
IHRpbWUgY2FjaGVfY2xlYW4gaXMgY2FsbGVkIGl0IGZpbmRzIHRoZSBuZXh0IG5vbi1lbXB0
eSBlbnRyeQo+ICAgKiBpbiB0aGUgY3VycmVudCB0YWJsZSBhbmQgd2Fsa3MgdGhlIGxpc3Qg
aW4gdGhhdCBlbnRyeQo+ICAgKiBsb29raW5nIGZvciBlbnRyaWVzIHRoYXQgY2FuIGJlIHJl
bW92ZWQuCj4gICAqCj4gQEAgLTQ1Myw2ICs0NTMsNyBAQCBzdGF0aWMgaW50IGNhY2hlX2Ns
ZWFuKHZvaWQpCj4gIAkJCWN1cnJlbnRfaW5kZXggKys7Cj4gIAkJc3Bpbl91bmxvY2soJmNh
Y2hlX2xpc3RfbG9jayk7Cj4gIAkJaWYgKGNoKSB7Cj4gKwkJCXNldF9iaXQoQ0FDSEVfQ0xF
QU5FRCwgJmNoLT5mbGFncyk7Cj4gIAkJCWNhY2hlX2ZyZXNoX3VubG9ja2VkKGNoLCBkKTsK
PiAgCQkJY2FjaGVfcHV0KGNoLCBkKTsKPiAgCQl9Cj4gQEAgLTExNzYsNiArMTE3Nyw5IEBA
IGludCBzdW5ycGNfY2FjaGVfcGlwZV91cGNhbGwoc3RydWN0IGNhY2hlX2RldGFpbCAqZGV0
YWlsLCBzdHJ1Y3QgY2FjaGVfaGVhZCAqaCkKPiAgCQl3YXJuX25vX2xpc3RlbmVyKGRldGFp
bCk7Cj4gIAkJcmV0dXJuIC1FSU5WQUw7Cj4gIAl9Cj4gKwlpZiAodGVzdF9iaXQoQ0FDSEVf
Q0xFQU5FRCwgJmgtPmZsYWdzKSkKPiArCQkvKiBUb28gbGF0ZSB0byBtYWtlIGFuIHVwY2Fs
bCAqLwo+ICsJCXJldHVybiAtRUFHQUlOOwo+ICAKPiAgCWJ1ZiA9IGttYWxsb2MoUEFHRV9T
SVpFLCBHRlBfS0VSTkVMKTsKPiAgCWlmICghYnVmKQo+IAoKSSBhZGRlZCB0aGlzIHBhdGNo
IHRvIG15IFNMRVMxMSBTUDEgYW5kIHN0aWxsIGhhZCBkcm9wcGVkIFJQQyByZXF1ZXN0cywK
YnV0IG5vIGxlYWtlZCBtZW1vcnkuCgpUaGUgZHJvcHMgb2NjdXIsIGJlY2F1c2UgY2FjaGVf
Y2hlY2soKSBhbGxvd3MgdG8gbWFrZSB1cGNhbGxzIGZvciBjYWNoZQplbnRyaWVzLCB0aGF0
IGFyZSBzdWJzdGl0dXRlZCB3aXRoIG5ldyBvbmVzIGJ5IHN1bnJwY19jYWNoZV91cGRhdGUo
KS4KSWYgY2FjaGVfY2xlYW4oKSBjb21lcyB0b28gbGF0ZSB0byB3YWtlIHVwIHRoZSB0aHJl
YWQgd2FpdGluZyBmb3IgYW4KdXBkYXRlIG9mIHRoZSBvbGQgY2FjaGUgZW50cnksIGEgdGlt
ZW91dCB3aWxsIG9jY3VyIGFuZCB0aGUgUlBDIHJlcXVlc3QKaXMgZHJvcHBlZC4KClRoaXMg
d2lsbCBub3QgaGFwcGVuIG9uIG1haW5saW5lLCBidXQgd2h5IHNob3VsZCB3ZSBhbGxvdyB1
cGNhbGxzIHRvIGJlCnF1ZXVlZCBmb3IgcmVwbGFjZWQgY2FjaGUgZW50cmllcz8gVGhpcyB3
aWxsIGNyZWF0ZSB1bm5lY2Vzc2FyeSB0cmFmZmljCm9uIHRoZSBjaGFubmVsIHRvIHRoZSBy
ZWFkZXIgYW5kIGFzIGEgcmVzdWx0IHVubmVjZXNzYXJ5IGNhbGxzIHRvCnN1bnJwY19jYWNo
ZV91cGRhdGUoKQoKQnkgYWRkaXRpb25hbGx5IHRvIHRoZSBhYm92ZSBwYXRjaCBpbnNlcnRp
bmcKICAgICJzZXRiaXQoQ0FDSEVfQ0xFQU5FRCwgJm9sZC0+ZmxhZ3MpOyIKaW50byBzdW5y
cGNfY2FjaGVfdXBkYXRlKCkgYmV0d2VlbiB0aGUgdHdvIGxpbmVzCiAgICAiY2FjaGVfZnJl
c2hfdW5sb2NrZWQodG1wLCBkZXRhaWwpOyIKICAgICJjYWNoZV9mcmVzaF91bmxvY2tlZChv
bGQsIGRldGFpbCk7Igp0aGlzIHVuY2xlYW4gYmVoYXZpb3IgY2FuIGJlIGF2b2lkZWQuCgpJ
ZiB3ZSBkbyBzbywgbWF5YmUgQ0FDSEVfQ0xFQU4gc2hvdWxkIGJldHRlciBiZSBuYW1lZCBD
QUNIRV9PTEQgb3IKc29tZXRoaW5nIGVsc2UgdGhhdCByZWZsZWN0cyBib3RoIHVzZXMgb2Yg
dGhlIGJpdC4KCkRvIHlvdSBhZ3JlZT8KCkJvZG8KCg==

Re: sunrpc/cache.c: races while updating cache entries

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 11930 bytes --]

On 19 Mar 2013 20:58:53 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
wrote:

> On 19 Mar 2013 04:24:00 +0100 NeilBrown <neilb@suse.de> wrote:
> 
> > On 14 Mar 2013 18:31:38 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
> > wrote:
> > 
> > > On 13 Mar 2013 07:55:00 +0100 NeilBrown <neilb@suse.de> wrote:
> > > 
> > > > On 11 Mar 2013 17:13:41 +0100 Bodo Stroesser 
> > > > <bstroesser@ts.fujitsu.com>
> > > > wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > AFAICS, there is one more race in RPC cache.
> > > > > 
> > > > > The problem showed up for the "auth.unix.ip" cache, that has a 
> > > > > reader.
> > > > > 
> > > > > If a server thread tries to find a cache entry, it first does a 
> > > > > lookup (or calls ip_map_cached_get() in this specific case). Then, 
> > > > > it calls cache_check() for this entry.
> > > > > 
> > > > > If the reader updates the cache entry just between the thread's 
> > > > > lookup and cache_check() execution, cache_check() will do an 
> > > > > upcall for this cache entry. This is, because
> > > > > sunrpc_cache_update() calls cache_fresh_locked(old, 0), which sets 
> > > > > expiry_time to 0.
> > > > > 
> > > > > Unfortunately, the reply to the upcall will not dequeue the queued 
> > > > > cache_request, as the reply will be assigned to the cache entry 
> > > > > newly created by the above cache update.
> > > > > 
> > > > > The result is a growing queue of orphaned cache_request structures 
> > > > > --> memory leak.
> > > > > 
> > > > > I found this on a SLES11 SP1 with a backport of the latest patches 
> > > > > that fix the other RPC races. On this old kernel, the problem also 
> > > > > leads to svc_drop() being called for the affected RPC request 
> > > > > (after svc_defer()).
> > > > > 
> > > > > Best Regards
> > > > > Bodo
> > > > 
> > > > I don't think this is a real problem.
> > > > The periodic call to "cache_clean" should find these orphaned 
> > > > requests and purge them.  So you could get a short term memory leak, 
> > > > but it should correct itself.
> > > > Do you agree?
> > > > 
> > > > Thanks,
> > > > NeilBrown
> > > 
> > > Meanwhile I found the missing part of the race!
> > > 
> > > It's just what I wrote above but additionally, immediately after the 
> > > reader updated the cache, cache_clean() unhashes the old cache entry. 
> > > In that case:
> > > 1) the thread will queue a request for a cache entry, that isn't
> > >    in the hash lists.
> > > 2) cache_clean() never will access this old cache entry again
> > > 3) every further cache update will call cache_dequeue() for a newer
> > >    cache entry, the request is never dequeued
> > > 
> > > --> memory leak.
> > 
> > Yes, I see that now.  Thanks.
> > 
> > I suspect that bug was introduced by commit 3af4974eb2c7867d6e16.
> > Prior to then, cache_clean would never remove anything with an active reference.  If something was about to get CACHE_PENDING, it would have a reference so cache_clean would leave it alone.
> > 
> > I wanted to fix this by getting the last 'put' to call cache_dequeue() if CACHE_PENDING was still set.  However I couldn't get access to the CACHE_PENDING flag and the cache_detail at the same place - so I gave up on that.
> > 
> > The patch I have included below sets a flag when an cache item is removed from the cache (by cache_clean) and refuses to lodge an upcall if the flag is set.  That will ensure there is never a pending upcall when the item is finally freed.
> > 
> > You patch only addresses the particular situation that you had found.  I think it is possible that there might be other situations that also lead to memory leaks, so I wanted a solution that would guarantee that there couldn't be a leak.
> > 
> 
> I think, your patch is safe (And I've tried to think of a remaining race,
> but couldn't find ;-)
> I've inserted it into SLES11 SP1 instead of my patch and started the test.
> 
> 
> > > 
> > > I verified this inserting some debug instructions. In a testrun that 
> > > triggered 6 times, and the queue printed by crash after the run had 6 
> > > orphaned entries.
> > > 
> > > As I wrote yesterday, I have a patch that solves the problem by 
> > > retesting the state of the cache entry after setting CACHE_PENDING in 
> > > cache_check(). I can send it if you like.
> > > 
> > > Bodo
> > > 
> > > P.S.:
> > > Maybe I'm wrong, but AFAICS, there are two further minor problems 
> > > regarding (nearly) expired cache entries:
> > > a) ip_map_cached_get() in some situations can return an already
> > >    outdated (it uses cache_valid that not fully checks the state)
> > >    cache entry. If cache_check() is caclled for that entry, it will
> > >    fail, I think
> > > b) Generally, if a cache entry is returned by sunrpc_cache_lookup()
> > >    and the entry is in the last second of it's expiry_time, the 
> > >    clock might move to the next second before cache_check is called.
> > >    If this happens, cache_check will fail, I think.
> > > Do you agree?
> > 
> > Yes, but I'm not sure how important this is.
> > Normally cache entries should be refreshed well before they expire, so we should normally find an entry with more than half of its lifetime left.
> > 
> > In the rare case where the scenario you describe happens, cache_check() will return -ETIMEDOUT.
> > In mainline, that will cause the request to be dropped and the connection closed so the client will try again and won't hit any race and so should get a correct result.
> > In SLES11SP1, we retry the lookup and will hopefully get a better result without having to close the connection.
> > 
> > So this should be rare and should fail-safe.
> > 
> > Does that agree with your understanding?
> 
> Only partly, because I was wrong.
> 
> On SLES11 SP1, as you say, cache_is_valid() examines expiry_time, but
> the retry will avoid to make a RPC request fail.
> 
> On a mainline kernel, cache_is_valid() does not evaluate the expiry_time.
> So, an old entry might start an upcall, but as the result of cache_is_valid
> still is 0, nothing will fail on mainline.
> 
> The only little issue remaining is the fact, that ip_map_cached_get() will
> accept outdated cache entries as long as they are not yet cleaned.
> In mainline this entry will even be accepted by cache_check().
> Do you think it would be a good idea to replace cache_valid() by
> cache_is_valid()?
> 


I think my problem is that I remember how this all used to work, rather than
how it has been changed to work over the years (largely by me or with my
oversight).  So I analyse the code against an out-of-date understanding :-(
I'm glad you aren't weighed down by that and so can see the code clearly!

Yes, in mainline an entry cached by ip_map_cached_put() will never be tested
for expiry.  I think we should change it to use cache_is_expired as below.

Thanks,
NeilBrown

From 0262882daee7ebef8f04bb40404695ba524e0bd1 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Wed, 20 Mar 2013 15:32:10 +1100
Subject: [PATCH] net/sunrpc: xpt_auth_cache should be ignored when expired.

commit d202cce8963d9268ff355a386e20243e8332b308
    sunrpc: never return expired entries in sunrpc_cache_lookup

moved the 'entry is expired' test from cache_check to
sunrpc_cache_lookup, so that it happened early and some races could
safely be ignored.

However the ip_map (in svcauth_unix.c) has a separate single-item
cache which allows quick lookup without locking.  An entry in this
case would not be subject to the expiry test and so could be used
well after it has expired.

This is not normally a big problem because the first time it is used
after it is expired an up-call will be scheduled to refresh the entry
(if it hasn't been scheduled already) and the old entry will then
be invalidated.  So on the second attempt to use it after it has
expired, ip_map_cached_get will discard it.

However that is subtle and not ideal, so replace the "!cache_valid"
test with "cache_is_expired".
In doing this we drop the test on the "CACHE_VALID" bit.  This is
unnecessary as the bit is never cleared, and an entry will only
be cached if the bit is set.

Reported-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 8419f7d..6ce690d 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -149,6 +149,24 @@ struct cache_deferred_req {
 					   int too_many);
 };
 
+/*
+ * timestamps kept in the cache are expressed in seconds
+ * since boot.  This is the best for measuring differences in
+ * real time.
+ */
+static inline time_t seconds_since_boot(void)
+{
+	struct timespec boot;
+	getboottime(&boot);
+	return get_seconds() - boot.tv_sec;
+}
+
+static inline time_t convert_to_wallclock(time_t sinceboot)
+{
+	struct timespec boot;
+	getboottime(&boot);
+	return boot.tv_sec + sinceboot;
+}
 
 extern const struct file_operations cache_file_operations_pipefs;
 extern const struct file_operations content_file_operations_pipefs;
@@ -182,15 +200,10 @@ static inline void cache_put(struct cache_head *h, struct cache_detail *cd)
 	kref_put(&h->ref, cd->cache_put);
 }
 
-static inline int cache_valid(struct cache_head *h)
+static inline int cache_is_expired(struct cache_detail *detail, struct cache_head *h)
 {
-	/* If an item has been unhashed pending removal when
-	 * the refcount drops to 0, the expiry_time will be
-	 * set to 0.  We don't want to consider such items
-	 * valid in this context even though CACHE_VALID is
-	 * set.
-	 */
-	return (h->expiry_time != 0 && test_bit(CACHE_VALID, &h->flags));
+	return  (h->expiry_time < seconds_since_boot()) ||
+		(detail->flush_time > h->last_refresh);
 }
 
 extern int cache_check(struct cache_detail *detail,
@@ -251,25 +264,6 @@ static inline int get_uint(char **bpp, unsigned int *anint)
 	return 0;
 }
 
-/*
- * timestamps kept in the cache are expressed in seconds
- * since boot.  This is the best for measuring differences in
- * real time.
- */
-static inline time_t seconds_since_boot(void)
-{
-	struct timespec boot;
-	getboottime(&boot);
-	return get_seconds() - boot.tv_sec;
-}
-
-static inline time_t convert_to_wallclock(time_t sinceboot)
-{
-	struct timespec boot;
-	getboottime(&boot);
-	return boot.tv_sec + sinceboot;
-}
-
 static inline time_t get_expiry(char **bpp)
 {
 	int rv;
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 8e7ccbb..81defe6 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -50,12 +50,6 @@ static void cache_init(struct cache_head *h)
 	h->last_refresh = now;
 }
 
-static inline int cache_is_expired(struct cache_detail *detail, struct cache_head *h)
-{
-	return  (h->expiry_time < seconds_since_boot()) ||
-		(detail->flush_time > h->last_refresh);
-}
-
 struct cache_head *sunrpc_cache_lookup(struct cache_detail *detail,
 				       struct cache_head *key, int hash)
 {
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index c3f9e1e..8d7a7b9 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -347,13 +347,13 @@ ip_map_cached_get(struct svc_xprt *xprt)
 		spin_lock(&xprt->xpt_lock);
 		ipm = xprt->xpt_auth_cache;
 		if (ipm != NULL) {
-			if (!cache_valid(&ipm->h)) {
+			sn = net_generic(xprt->xpt_net, sunrpc_net_id);
+			if (cache_is_expired(sn->ip_map_cache, &ipm->h)) {
 				/*
 				 * The entry has been invalidated since it was
 				 * remembered, e.g. by a second mount from the
 				 * same IP address.
 				 */
-				sn = net_generic(xprt->xpt_net, sunrpc_net_id);
 				xprt->xpt_auth_cache = NULL;
 				spin_unlock(&xprt->xpt_lock);
 				cache_put(&ipm->h, sn->ip_map_cache);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMTkgTWFyIDIwMTMgMDQ6MjQ6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRl
PiB3cm90ZToKCj4gT24gMTQgTWFyIDIwMTMgMTg6MzE6MzggKzAxMDAgQm9kbyBTdHJvZXNz
ZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+Cj4gd3JvdGU6Cj4gCj4gPiBPbiAxMyBN
YXIgMjAxMyAwNzo1NTowMCArMDEwMCBOZWlsQnJvd24gPG5laWxiQHN1c2UuZGU+IHdyb3Rl
Ogo+ID4gCj4gPiA+IE9uIDExIE1hciAyMDEzIDE3OjEzOjQxICswMTAwIEJvZG8gU3Ryb2Vz
c2VyIAo+ID4gPiA8YnN0cm9lc3NlckB0cy5mdWppdHN1LmNvbT4KPiA+ID4gd3JvdGU6Cj4g
PiA+IAo+ID4gPiA+IEhpLAo+ID4gPiA+IAo+ID4gPiA+IEFGQUlDUywgdGhlcmUgaXMgb25l
IG1vcmUgcmFjZSBpbiBSUEMgY2FjaGUuCj4gPiA+ID4gCj4gPiA+ID4gVGhlIHByb2JsZW0g
c2hvd2VkIHVwIGZvciB0aGUgImF1dGgudW5peC5pcCIgY2FjaGUsIHRoYXQgaGFzIGEgCj4g
PiA+ID4gcmVhZGVyLgo+ID4gPiA+IAo+ID4gPiA+IElmIGEgc2VydmVyIHRocmVhZCB0cmll
cyB0byBmaW5kIGEgY2FjaGUgZW50cnksIGl0IGZpcnN0IGRvZXMgYSAKPiA+ID4gPiBsb29r
dXAgKG9yIGNhbGxzIGlwX21hcF9jYWNoZWRfZ2V0KCkgaW4gdGhpcyBzcGVjaWZpYyBjYXNl
KS4gVGhlbiwgCj4gPiA+ID4gaXQgY2FsbHMgY2FjaGVfY2hlY2soKSBmb3IgdGhpcyBlbnRy
eS4KPiA+ID4gPiAKPiA+ID4gPiBJZiB0aGUgcmVhZGVyIHVwZGF0ZXMgdGhlIGNhY2hlIGVu
dHJ5IGp1c3QgYmV0d2VlbiB0aGUgdGhyZWFkJ3MgCj4gPiA+ID4gbG9va3VwIGFuZCBjYWNo
ZV9jaGVjaygpIGV4ZWN1dGlvbiwgY2FjaGVfY2hlY2soKSB3aWxsIGRvIGFuIAo+ID4gPiA+
IHVwY2FsbCBmb3IgdGhpcyBjYWNoZSBlbnRyeS4gVGhpcyBpcywgYmVjYXVzZQo+ID4gPiA+
IHN1bnJwY19jYWNoZV91cGRhdGUoKSBjYWxscyBjYWNoZV9mcmVzaF9sb2NrZWQob2xkLCAw
KSwgd2hpY2ggc2V0cyAKPiA+ID4gPiBleHBpcnlfdGltZSB0byAwLgo+ID4gPiA+IAo+ID4g
PiA+IFVuZm9ydHVuYXRlbHksIHRoZSByZXBseSB0byB0aGUgdXBjYWxsIHdpbGwgbm90IGRl
cXVldWUgdGhlIHF1ZXVlZCAKPiA+ID4gPiBjYWNoZV9yZXF1ZXN0LCBhcyB0aGUgcmVwbHkg
d2lsbCBiZSBhc3NpZ25lZCB0byB0aGUgY2FjaGUgZW50cnkgCj4gPiA+ID4gbmV3bHkgY3Jl
YXRlZCBieSB0aGUgYWJvdmUgY2FjaGUgdXBkYXRlLgo+ID4gPiA+IAo+ID4gPiA+IFRoZSBy
ZXN1bHQgaXMgYSBncm93aW5nIHF1ZXVlIG9mIG9ycGhhbmVkIGNhY2hlX3JlcXVlc3Qgc3Ry
dWN0dXJlcyAKPiA+ID4gPiAtLT4gbWVtb3J5IGxlYWsuCj4gPiA+ID4gCj4gPiA+ID4gSSBm
b3VuZCB0aGlzIG9uIGEgU0xFUzExIFNQMSB3aXRoIGEgYmFja3BvcnQgb2YgdGhlIGxhdGVz
dCBwYXRjaGVzIAo+ID4gPiA+IHRoYXQgZml4IHRoZSBvdGhlciBSUEMgcmFjZXMuIE9uIHRo
aXMgb2xkIGtlcm5lbCwgdGhlIHByb2JsZW0gYWxzbyAKPiA+ID4gPiBsZWFkcyB0byBzdmNf
ZHJvcCgpIGJlaW5nIGNhbGxlZCBmb3IgdGhlIGFmZmVjdGVkIFJQQyByZXF1ZXN0IAo+ID4g
PiA+IChhZnRlciBzdmNfZGVmZXIoKSkuCj4gPiA+ID4gCj4gPiA+ID4gQmVzdCBSZWdhcmRz
Cj4gPiA+ID4gQm9kbwo+ID4gPiAKPiA+ID4gSSBkb24ndCB0aGluayB0aGlzIGlzIGEgcmVh
bCBwcm9ibGVtLgo+ID4gPiBUaGUgcGVyaW9kaWMgY2FsbCB0byAiY2FjaGVfY2xlYW4iIHNo
b3VsZCBmaW5kIHRoZXNlIG9ycGhhbmVkIAo+ID4gPiByZXF1ZXN0cyBhbmQgcHVyZ2UgdGhl
bS4gIFNvIHlvdSBjb3VsZCBnZXQgYSBzaG9ydCB0ZXJtIG1lbW9yeSBsZWFrLCAKPiA+ID4g
YnV0IGl0IHNob3VsZCBjb3JyZWN0IGl0c2VsZi4KPiA+ID4gRG8geW91IGFncmVlPwo+ID4g
PiAKPiA+ID4gVGhhbmtzLAo+ID4gPiBOZWlsQnJvd24KPiA+IAo+ID4gTWVhbndoaWxlIEkg
Zm91bmQgdGhlIG1pc3NpbmcgcGFydCBvZiB0aGUgcmFjZSEKPiA+IAo+ID4gSXQncyBqdXN0
IHdoYXQgSSB3cm90ZSBhYm92ZSBidXQgYWRkaXRpb25hbGx5LCBpbW1lZGlhdGVseSBhZnRl
ciB0aGUgCj4gPiByZWFkZXIgdXBkYXRlZCB0aGUgY2FjaGUsIGNhY2hlX2NsZWFuKCkgdW5o
YXNoZXMgdGhlIG9sZCBjYWNoZSBlbnRyeS4gCj4gPiBJbiB0aGF0IGNhc2U6Cj4gPiAxKSB0
aGUgdGhyZWFkIHdpbGwgcXVldWUgYSByZXF1ZXN0IGZvciBhIGNhY2hlIGVudHJ5LCB0aGF0
IGlzbid0Cj4gPiAgICBpbiB0aGUgaGFzaCBsaXN0cy4KPiA+IDIpIGNhY2hlX2NsZWFuKCkg
bmV2ZXIgd2lsbCBhY2Nlc3MgdGhpcyBvbGQgY2FjaGUgZW50cnkgYWdhaW4KPiA+IDMpIGV2
ZXJ5IGZ1cnRoZXIgY2FjaGUgdXBkYXRlIHdpbGwgY2FsbCBjYWNoZV9kZXF1ZXVlKCkgZm9y
IGEgbmV3ZXIKPiA+ICAgIGNhY2hlIGVudHJ5LCB0aGUgcmVxdWVzdCBpcyBuZXZlciBkZXF1
ZXVlZAo+ID4gCj4gPiAtLT4gbWVtb3J5IGxlYWsuCj4gCj4gWWVzLCBJIHNlZSB0aGF0IG5v
dy4gIFRoYW5rcy4KPiAKPiBJIHN1c3BlY3QgdGhhdCBidWcgd2FzIGludHJvZHVjZWQgYnkg
Y29tbWl0IDNhZjQ5NzRlYjJjNzg2N2Q2ZTE2Lgo+IFByaW9yIHRvIHRoZW4sIGNhY2hlX2Ns
ZWFuIHdvdWxkIG5ldmVyIHJlbW92ZSBhbnl0aGluZyB3aXRoIGFuIGFjdGl2ZSByZWZlcmVu
Y2UuICBJZiBzb21ldGhpbmcgd2FzIGFib3V0IHRvIGdldCBDQUNIRV9QRU5ESU5HLCBpdCB3
b3VsZCBoYXZlIGEgcmVmZXJlbmNlIHNvIGNhY2hlX2NsZWFuIHdvdWxkIGxlYXZlIGl0IGFs
b25lLgo+IAo+IEkgd2FudGVkIHRvIGZpeCB0aGlzIGJ5IGdldHRpbmcgdGhlIGxhc3QgJ3B1
dCcgdG8gY2FsbCBjYWNoZV9kZXF1ZXVlKCkgaWYgQ0FDSEVfUEVORElORyB3YXMgc3RpbGwg
c2V0LiAgSG93ZXZlciBJIGNvdWxkbid0IGdldCBhY2Nlc3MgdG8gdGhlIENBQ0hFX1BFTkRJ
TkcgZmxhZyBhbmQgdGhlIGNhY2hlX2RldGFpbCBhdCB0aGUgc2FtZSBwbGFjZSAtIHNvIEkg
Z2F2ZSB1cCBvbiB0aGF0Lgo+IAo+IFRoZSBwYXRjaCBJIGhhdmUgaW5jbHVkZWQgYmVsb3cg
c2V0cyBhIGZsYWcgd2hlbiBhbiBjYWNoZSBpdGVtIGlzIHJlbW92ZWQgZnJvbSB0aGUgY2Fj
aGUgKGJ5IGNhY2hlX2NsZWFuKSBhbmQgcmVmdXNlcyB0byBsb2RnZSBhbiB1cGNhbGwgaWYg
dGhlIGZsYWcgaXMgc2V0LiAgVGhhdCB3aWxsIGVuc3VyZSB0aGVyZSBpcyBuZXZlciBhIHBl
bmRpbmcgdXBjYWxsIHdoZW4gdGhlIGl0ZW0gaXMgZmluYWxseSBmcmVlZC4KPiAKPiBZb3Ug
cGF0Y2ggb25seSBhZGRyZXNzZXMgdGhlIHBhcnRpY3VsYXIgc2l0dWF0aW9uIHRoYXQgeW91
IGhhZCBmb3VuZC4gIEkgdGhpbmsgaXQgaXMgcG9zc2libGUgdGhhdCB0aGVyZSBtaWdodCBi
ZSBvdGhlciBzaXR1YXRpb25zIHRoYXQgYWxzbyBsZWFkIHRvIG1lbW9yeSBsZWFrcywgc28g
SSB3YW50ZWQgYSBzb2x1dGlvbiB0aGF0IHdvdWxkIGd1YXJhbnRlZSB0aGF0IHRoZXJlIGNv
dWxkbid0IGJlIGEgbGVhay4KPiAKCkkgdGhpbmssIHlvdXIgcGF0Y2ggaXMgc2FmZSAoQW5k
IEkndmUgdHJpZWQgdG8gdGhpbmsgb2YgYSByZW1haW5pbmcgcmFjZSwKYnV0IGNvdWxkbid0
IGZpbmQgOy0pCkkndmUgaW5zZXJ0ZWQgaXQgaW50byBTTEVTMTEgU1AxIGluc3RlYWQgb2Yg
bXkgcGF0Y2ggYW5kIHN0YXJ0ZWQgdGhlIHRlc3QuCgoKPiA+IAo+ID4gSSB2ZXJpZmllZCB0
aGlzIGluc2VydGluZyBzb21lIGRlYnVnIGluc3RydWN0aW9ucy4gSW4gYSB0ZXN0cnVuIHRo
YXQgCj4gPiB0cmlnZ2VyZWQgNiB0aW1lcywgYW5kIHRoZSBxdWV1ZSBwcmludGVkIGJ5IGNy
YXNoIGFmdGVyIHRoZSBydW4gaGFkIDYgCj4gPiBvcnBoYW5lZCBlbnRyaWVzLgo+ID4gCj4g
PiBBcyBJIHdyb3RlIHllc3RlcmRheSwgSSBoYXZlIGEgcGF0Y2ggdGhhdCBzb2x2ZXMgdGhl
IHByb2JsZW0gYnkgCj4gPiByZXRlc3RpbmcgdGhlIHN0YXRlIG9mIHRoZSBjYWNoZSBlbnRy
eSBhZnRlciBzZXR0aW5nIENBQ0hFX1BFTkRJTkcgaW4gCj4gPiBjYWNoZV9jaGVjaygpLiBJ
IGNhbiBzZW5kIGl0IGlmIHlvdSBsaWtlLgo+ID4gCj4gPiBCb2RvCj4gPiAKPiA+IFAuUy46
Cj4gPiBNYXliZSBJJ20gd3JvbmcsIGJ1dCBBRkFJQ1MsIHRoZXJlIGFyZSB0d28gZnVydGhl
ciBtaW5vciBwcm9ibGVtcyAKPiA+IHJlZ2FyZGluZyAobmVhcmx5KSBleHBpcmVkIGNhY2hl
IGVudHJpZXM6Cj4gPiBhKSBpcF9tYXBfY2FjaGVkX2dldCgpIGluIHNvbWUgc2l0dWF0aW9u
cyBjYW4gcmV0dXJuIGFuIGFscmVhZHkKPiA+ICAgIG91dGRhdGVkIChpdCB1c2VzIGNhY2hl
X3ZhbGlkIHRoYXQgbm90IGZ1bGx5IGNoZWNrcyB0aGUgc3RhdGUpCj4gPiAgICBjYWNoZSBl
bnRyeS4gSWYgY2FjaGVfY2hlY2soKSBpcyBjYWNsbGVkIGZvciB0aGF0IGVudHJ5LCBpdCB3
aWxsCj4gPiAgICBmYWlsLCBJIHRoaW5rCj4gPiBiKSBHZW5lcmFsbHksIGlmIGEgY2FjaGUg
ZW50cnkgaXMgcmV0dXJuZWQgYnkgc3VucnBjX2NhY2hlX2xvb2t1cCgpCj4gPiAgICBhbmQg
dGhlIGVudHJ5IGlzIGluIHRoZSBsYXN0IHNlY29uZCBvZiBpdCdzIGV4cGlyeV90aW1lLCB0
aGUgCj4gPiAgICBjbG9jayBtaWdodCBtb3ZlIHRvIHRoZSBuZXh0IHNlY29uZCBiZWZvcmUg
Y2FjaGVfY2hlY2sgaXMgY2FsbGVkLgo+ID4gICAgSWYgdGhpcyBoYXBwZW5zLCBjYWNoZV9j
aGVjayB3aWxsIGZhaWwsIEkgdGhpbmsuCj4gPiBEbyB5b3UgYWdyZWU/Cj4gCj4gWWVzLCBi
dXQgSSdtIG5vdCBzdXJlIGhvdyBpbXBvcnRhbnQgdGhpcyBpcy4KPiBOb3JtYWxseSBjYWNo
ZSBlbnRyaWVzIHNob3VsZCBiZSByZWZyZXNoZWQgd2VsbCBiZWZvcmUgdGhleSBleHBpcmUs
IHNvIHdlIHNob3VsZCBub3JtYWxseSBmaW5kIGFuIGVudHJ5IHdpdGggbW9yZSB0aGFuIGhh
bGYgb2YgaXRzIGxpZmV0aW1lIGxlZnQuCj4gCj4gSW4gdGhlIHJhcmUgY2FzZSB3aGVyZSB0
aGUgc2NlbmFyaW8geW91IGRlc2NyaWJlIGhhcHBlbnMsIGNhY2hlX2NoZWNrKCkgd2lsbCBy
ZXR1cm4gLUVUSU1FRE9VVC4KPiBJbiBtYWlubGluZSwgdGhhdCB3aWxsIGNhdXNlIHRoZSBy
ZXF1ZXN0IHRvIGJlIGRyb3BwZWQgYW5kIHRoZSBjb25uZWN0aW9uIGNsb3NlZCBzbyB0aGUg
Y2xpZW50IHdpbGwgdHJ5IGFnYWluIGFuZCB3b24ndCBoaXQgYW55IHJhY2UgYW5kIHNvIHNo
b3VsZCBnZXQgYSBjb3JyZWN0IHJlc3VsdC4KPiBJbiBTTEVTMTFTUDEsIHdlIHJldHJ5IHRo
ZSBsb29rdXAgYW5kIHdpbGwgaG9wZWZ1bGx5IGdldCBhIGJldHRlciByZXN1bHQgd2l0aG91
dCBoYXZpbmcgdG8gY2xvc2UgdGhlIGNvbm5lY3Rpb24uCj4gCj4gU28gdGhpcyBzaG91bGQg
YmUgcmFyZSBhbmQgc2hvdWxkIGZhaWwtc2FmZS4KPiAKPiBEb2VzIHRoYXQgYWdyZWUgd2l0
aCB5b3VyIHVuZGVyc3RhbmRpbmc/CgpPbmx5IHBhcnRseSwgYmVjYXVzZSBJIHdhcyB3cm9u
Zy4KCk9uIFNMRVMxMSBTUDEsIGFzIHlvdSBzYXksIGNhY2hlX2lzX3ZhbGlkKCkgZXhhbWlu
ZXMgZXhwaXJ5X3RpbWUsIGJ1dAp0aGUgcmV0cnkgd2lsbCBhdm9pZCB0byBtYWtlIGEgUlBD
IHJlcXVlc3QgZmFpbC4KCk9uIGEgbWFpbmxpbmUga2VybmVsLCBjYWNoZV9pc192YWxpZCgp
IGRvZXMgbm90IGV2YWx1YXRlIHRoZSBleHBpcnlfdGltZS4KU28sIGFuIG9sZCBlbnRyeSBt
aWdodCBzdGFydCBhbiB1cGNhbGwsIGJ1dCBhcyB0aGUgcmVzdWx0IG9mIGNhY2hlX2lzX3Zh
bGlkCnN0aWxsIGlzIDAsIG5vdGhpbmcgd2lsbCBmYWlsIG9uIG1haW5saW5lLgoKVGhlIG9u
bHkgbGl0dGxlIGlzc3VlIHJlbWFpbmluZyBpcyB0aGUgZmFjdCwgdGhhdCBpcF9tYXBfY2Fj
aGVkX2dldCgpIHdpbGwKYWNjZXB0IG91dGRhdGVkIGNhY2hlIGVudHJpZXMgYXMgbG9uZyBh
cyB0aGV5IGFyZSBub3QgeWV0IGNsZWFuZWQuCkluIG1haW5saW5lIHRoaXMgZW50cnkgd2ls
bCBldmVuIGJlIGFjY2VwdGVkIGJ5IGNhY2hlX2NoZWNrKCkuCkRvIHlvdSB0aGluayBpdCB3
b3VsZCBiZSBhIGdvb2QgaWRlYSB0byByZXBsYWNlIGNhY2hlX3ZhbGlkKCkgYnkKY2FjaGVf
aXNfdmFsaWQoKT8KClRoYW5rIHlvdQpCb2RvCiAKCj4gCj4gCj4gVGhhbmtzLAo+IE5laWxC
cm93bgo+IAo+IAo+IEZyb20gZTc2ZTY1ODM0MDVhM2I1ZmY5YzhkMmFlMTE4NDcwNGVmYjIw
OWVmMCBNb24gU2VwIDE3IDAwOjAwOjAwIDIwMDEKPiBGcm9tOiBOZWlsQnJvd24gPG5laWxi
QHN1c2UuZGU+Cj4gRGF0ZTogVHVlLCAxOSBNYXIgMjAxMyAxMzo1ODo1OCArMTEwMAo+IFN1
YmplY3Q6IFtQQVRDSF0gc3VucnBjL2NhY2hlOiBlbnN1cmUgaXRlbXMgcmVtb3ZlZCBmcm9t
IGNhY2hlIGRvIG5vdCBoYXZlICBwZW5kaW5nIHVwY2FsbHMuCj4gCj4gSXQgaXMgcG9zc2li
bGUgZm9yIGEgcmFjZSB0byBzZXQgQ0FDSEVfUEVORElORyBhZnRlciBjYWNoZV9jbGVhbigp
IGhhcyByZW1vdmVkIGEgY2FjaGUgZW50cnkgZnJvbSB0aGUgY2FjaGUuCj4gSWYgQ0FDSEVf
UEVORElORyBpcyBzdGlsbCBzZXQgd2hlbiB0aGUgZW50cnkgaXMgZmluYWxseSAncHV0Jywg
dGhlIGNhY2hlX2RlcXVldWUoKSB3aWxsIG5ldmVyIGhhcHBlbiBhbmQgd2UgY2FuIGxlYWsg
bWVtb3J5Lgo+IAo+IFNvIHNldCBhIG5ldyBmbGFnICdDQUNIRV9DTEVBTkVEJyB3aGVuIHdl
IHJlbW92ZSBzb21ldGhpbmcgZnJvbSB0aGUgY2FjaGUsIGFuZCBkb24ndCBxdWV1ZSBhbmQg
dXBjYWxsIGlmIGl0IGlzIHNldC4KPiAKPiBJZiBDQUNIRV9QRU5ESU5HIGlzIHNldCBiZWZv
cmUgQ0FDSEVfQ0xFQU5FRCwgdGhlIGNhbGwgdGhhdAo+IGNhY2hlX2NsZWFuKCkgbWFrZXMg
dG8gY2FjaGVfZnJlc2hfdW5sb2NrZWQoKSB3aWxsIGZyZWUgbWVtb3J5IGFzIG5lZWRlZC4g
IElmIENBQ0hFX1BFTkRJTkcgaXMgc2V0IGFmdGVyIENBQ0hFX0NMRUFORUQsIHRoZSB0ZXN0
IGluIHN1bnJwY19jYWNoZV9waXBlX3VwY2FsbCB3aWxsIGVuc3VyZSB0aGF0IHRoZSBtZW1v
cnkgaXMgbm90IGFsbG9jYXRlZC4KPiAKPiBSZXBvcnRlZC1ieTogPGJzdHJvZXNzZXJAdHMu
ZnVqaXRzdS5jb20+Cj4gU2lnbmVkLW9mZi1ieTogTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRl
Pgo+IAo+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L3N1bnJwYy9jYWNoZS5oIGIvaW5j
bHVkZS9saW51eC9zdW5ycGMvY2FjaGUuaCBpbmRleCAzMDMzOTliLi44NDE5ZjdkIDEwMDY0
NAo+IC0tLSBhL2luY2x1ZGUvbGludXgvc3VucnBjL2NhY2hlLmgKPiArKysgYi9pbmNsdWRl
L2xpbnV4L3N1bnJwYy9jYWNoZS5oCj4gQEAgLTU3LDYgKzU3LDcgQEAgc3RydWN0IGNhY2hl
X2hlYWQgewo+ICAjZGVmaW5lCUNBQ0hFX1ZBTElECTAJLyogRW50cnkgY29udGFpbnMgdmFs
aWQgZGF0YSAqLwo+ICAjZGVmaW5lCUNBQ0hFX05FR0FUSVZFCTEJLyogTmVnYXRpdmUgZW50
cnkgLSB0aGVyZSBpcyBubyBtYXRjaCBmb3IgdGhlIGtleSAqLwo+ICAjZGVmaW5lCUNBQ0hF
X1BFTkRJTkcJMgkvKiBBbiB1cGNhbGwgaGFzIGJlZW4gc2VudCBidXQgbm8gcmVwbHkgcmVj
ZWl2ZWQgeWV0Ki8KPiArI2RlZmluZQlDQUNIRV9DTEVBTkVECTMJLyogRW50cnkgaGFzIGJl
ZW4gY2xlYW5lZCBmcm9tIGNhY2hlICovCj4gIAo+ICAjZGVmaW5lCUNBQ0hFX05FV19FWFBJ
UlkgMTIwCS8qIGtlZXAgbmV3IHRoaW5ncyBwZW5kaW5nIGNvbmZpcm1hdGlvbiBmb3IgMTIw
IHNlY29uZHMgKi8KPiAgCj4gZGlmZiAtLWdpdCBhL25ldC9zdW5ycGMvY2FjaGUuYyBiL25l
dC9zdW5ycGMvY2FjaGUuYyBpbmRleCAxNjc3Y2ZlLi44ZTdjY2JiIDEwMDY0NAo+IC0tLSBh
L25ldC9zdW5ycGMvY2FjaGUuYwo+ICsrKyBiL25ldC9zdW5ycGMvY2FjaGUuYwo+IEBAIC0z
MDYsNyArMzA2LDcgQEAgRVhQT1JUX1NZTUJPTF9HUEwoY2FjaGVfY2hlY2spOwo+ICAgKiBh
IGN1cnJlbnQgcG9pbnRlciBpbnRvIHRoYXQgbGlzdCBhbmQgaW50byB0aGUgdGFibGUKPiAg
ICogZm9yIHRoYXQgZW50cnkuCj4gICAqCj4gLSAqIEVhY2ggdGltZSBjbGVhbl9jYWNoZSBp
cyBjYWxsZWQgaXQgZmluZHMgdGhlIG5leHQgbm9uLWVtcHR5IGVudHJ5Cj4gKyAqIEVhY2gg
dGltZSBjYWNoZV9jbGVhbiBpcyBjYWxsZWQgaXQgZmluZHMgdGhlIG5leHQgbm9uLWVtcHR5
IGVudHJ5Cj4gICAqIGluIHRoZSBjdXJyZW50IHRhYmxlIGFuZCB3YWxrcyB0aGUgbGlzdCBp
biB0aGF0IGVudHJ5Cj4gICAqIGxvb2tpbmcgZm9yIGVudHJpZXMgdGhhdCBjYW4gYmUgcmVt
b3ZlZC4KPiAgICoKPiBAQCAtNDUzLDYgKzQ1Myw3IEBAIHN0YXRpYyBpbnQgY2FjaGVfY2xl
YW4odm9pZCkKPiAgCQkJY3VycmVudF9pbmRleCArKzsKPiAgCQlzcGluX3VubG9jaygmY2Fj
aGVfbGlzdF9sb2NrKTsKPiAgCQlpZiAoY2gpIHsKPiArCQkJc2V0X2JpdChDQUNIRV9DTEVB
TkVELCAmY2gtPmZsYWdzKTsKPiAgCQkJY2FjaGVfZnJlc2hfdW5sb2NrZWQoY2gsIGQpOwo+
ICAJCQljYWNoZV9wdXQoY2gsIGQpOwo+ICAJCX0KPiBAQCAtMTE3Niw2ICsxMTc3LDkgQEAg
aW50IHN1bnJwY19jYWNoZV9waXBlX3VwY2FsbChzdHJ1Y3QgY2FjaGVfZGV0YWlsICpkZXRh
aWwsIHN0cnVjdCBjYWNoZV9oZWFkICpoKQo+ICAJCXdhcm5fbm9fbGlzdGVuZXIoZGV0YWls
KTsKPiAgCQlyZXR1cm4gLUVJTlZBTDsKPiAgCX0KPiArCWlmICh0ZXN0X2JpdChDQUNIRV9D
TEVBTkVELCAmaC0+ZmxhZ3MpKQo+ICsJCS8qIFRvbyBsYXRlIHRvIG1ha2UgYW4gdXBjYWxs
ICovCj4gKwkJcmV0dXJuIC1FQUdBSU47Cj4gIAo+ICAJYnVmID0ga21hbGxvYyhQQUdFX1NJ
WkUsIEdGUF9LRVJORUwpOwo+ICAJaWYgKCFidWYpCj4gCg==

Re: sunrpc/cache.c: races while updating cache entries

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 7657 bytes --]

On 14 Mar 2013 18:31:38 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
wrote:

> On 13 Mar 2013 07:55:00 +0100 NeilBrown <neilb@suse.de> wrote:
> 
> > On 11 Mar 2013 17:13:41 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
> > wrote:
> > 
> > > Hi,
> > > 
> > > AFAICS, there is one more race in RPC cache.
> > > 
> > > The problem showed up for the "auth.unix.ip" cache, that
> > > has a reader.
> > > 
> > > If a server thread tries to find a cache entry, it first
> > > does a lookup (or calls ip_map_cached_get() in this specific
> > > case). Then, it calls cache_check() for this entry.
> > > 
> > > If the reader updates the cache entry just between the
> > > thread's lookup and cache_check() execution, cache_check()
> > > will do an upcall for this cache entry. This is, because
> > > sunrpc_cache_update() calls cache_fresh_locked(old, 0),
> > > which sets expiry_time to 0.
> > > 
> > > Unfortunately, the reply to the upcall will not dequeue
> > > the queued cache_request, as the reply will be assigned to
> > > the cache entry newly created by the above cache update.
> > > 
> > > The result is a growing queue of orphaned cache_request
> > > structures --> memory leak.
> > > 
> > > I found this on a SLES11 SP1 with a backport of the latest
> > > patches that fix the other RPC races. On this old kernel,
> > > the problem also leads to svc_drop() being called for the
> > > affected RPC request (after svc_defer()).
> > > 
> > > Best Regards
> > > Bodo
> > 
> > I don't think this is a real problem.
> > The periodic call to "cache_clean" should find these orphaned requests and
> > purge them.  So you could get a short term memory leak, but it should
> > correct itself.
> > Do you agree?
> > 
> > Thanks,
> > NeilBrown
> 
> Meanwhile I found the missing part of the race!
> 
> It's just what I wrote above but additionally, immediately after
> the reader updated the cache, cache_clean() unhashes the old cache
> entry. In that case:
> 1) the thread will queue a request for a cache entry, that isn't
>    in the hash lists.
> 2) cache_clean() never will access this old cache entry again
> 3) every further cache update will call cache_dequeue() for a newer
>    cache entry, the request is never dequeued
> 
> --> memory leak.

Yes, I see that now.  Thanks.

I suspect that bug was introduced by commit 3af4974eb2c7867d6e16.
Prior to then, cache_clean would never remove anything with an active
reference.  If something was about to get CACHE_PENDING, it would have a
reference so cache_clean would leave it alone.

I wanted to fix this by getting the last 'put' to call cache_dequeue() if
CACHE_PENDING was still set.  However I couldn't get access to the
CACHE_PENDING flag and the cache_detail at the same place - so I gave up on
that.

The patch I have included below sets a flag when an cache item is removed
from the cache (by cache_clean) and refuses to lodge an upcall if the flag is
set.  That will ensure there is never a pending upcall when the item is
finally freed.

You patch only addresses the particular situation that you had found.  I
think it is possible that there might be other situations that also lead to
memory leaks, so I wanted a solution that would guarantee that there couldn't
be a leak.

> 
> I verified this inserting some debug instructions. In a testrun
> that triggered 6 times, and the queue printed by crash after the
> run had 6 orphaned entries.
> 
> As I wrote yesterday, I have a patch that solves the problem by
> retesting the state of the cache entry after setting CACHE_PENDING
> in cache_check(). I can send it if you like.
> 
> Bodo
> 
> P.S.:
> Maybe I'm wrong, but AFAICS, there are two further minor problems
> regarding (nearly) expired cache entries:
> a) ip_map_cached_get() in some situations can return an already
>    outdated (it uses cache_valid that not fully checks the state)
>    cache entry. If cache_check() is caclled for that entry, it will
>    fail, I think
> b) Generally, if a cache entry is returned by sunrpc_cache_lookup()
>    and the entry is in the last second of it's expiry_time, the 
>    clock might move to the next second before cache_check is called.
>    If this happens, cache_check will fail, I think.
> Do you agree?

Yes, but I'm not sure how important this is.
Normally cache entries should be refreshed well before they expire, so we
should normally find an entry with more than half of its lifetime left.

In the rare case where the scenario you describe happens, cache_check() will
return -ETIMEDOUT.
In mainline, that will cause the request to be dropped and the connection
closed so the client will try again and won't hit any race and so should get
a correct result.
In SLES11SP1, we retry the lookup and will hopefully get a better result
without having to close the connection.

So this should be rare and should fail-safe.

Does that agree with your understanding?


Thanks,
NeilBrown


From e76e6583405a3b5ff9c8d2ae1184704efb209ef0 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Tue, 19 Mar 2013 13:58:58 +1100
Subject: [PATCH] sunrpc/cache: ensure items removed from cache do not have
 pending upcalls.

It is possible for a race to set CACHE_PENDING after cache_clean()
has removed a cache entry from the cache.
If CACHE_PENDING is still set when the entry is finally 'put',
the cache_dequeue() will never happen and we can leak memory.

So set a new flag 'CACHE_CLEANED' when we remove something from
the cache, and don't queue and upcall if it is set.

If CACHE_PENDING is set before CACHE_CLEANED, the call that
cache_clean() makes to cache_fresh_unlocked() will free memory
as needed.  If CACHE_PENDING is set after CACHE_CLEANED, the
test in sunrpc_cache_pipe_upcall will ensure that the memory
is not allocated.

Reported-by: <bstroesser@ts.fujitsu.com>
Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 303399b..8419f7d 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -57,6 +57,7 @@ struct cache_head {
 #define	CACHE_VALID	0	/* Entry contains valid data */
 #define	CACHE_NEGATIVE	1	/* Negative entry - there is no match for the key */
 #define	CACHE_PENDING	2	/* An upcall has been sent but no reply received yet*/
+#define	CACHE_CLEANED	3	/* Entry has been cleaned from cache */
 
 #define	CACHE_NEW_EXPIRY 120	/* keep new things pending confirmation for 120 seconds */
 
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 1677cfe..8e7ccbb 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -306,7 +306,7 @@ EXPORT_SYMBOL_GPL(cache_check);
  * a current pointer into that list and into the table
  * for that entry.
  *
- * Each time clean_cache is called it finds the next non-empty entry
+ * Each time cache_clean is called it finds the next non-empty entry
  * in the current table and walks the list in that entry
  * looking for entries that can be removed.
  *
@@ -453,6 +453,7 @@ static int cache_clean(void)
 			current_index ++;
 		spin_unlock(&cache_list_lock);
 		if (ch) {
+			set_bit(CACHE_CLEANED, &ch->flags);
 			cache_fresh_unlocked(ch, d);
 			cache_put(ch, d);
 		}
@@ -1176,6 +1177,9 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h)
 		warn_no_listener(detail);
 		return -EINVAL;
 	}
+	if (test_bit(CACHE_CLEANED, &h->flags))
+		/* Too late to make an upcall */
+		return -EAGAIN;
 
 	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!buf)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMTMgTWFyIDIwMTMgMDc6NTU6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRl
PiB3cm90ZToKCj4gT24gMTEgTWFyIDIwMTMgMTc6MTM6NDEgKzAxMDAgQm9kbyBTdHJvZXNz
ZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+Cj4gd3JvdGU6Cj4gCj4gPiBIaSwKPiA+
IAo+ID4gQUZBSUNTLCB0aGVyZSBpcyBvbmUgbW9yZSByYWNlIGluIFJQQyBjYWNoZS4KPiA+
IAo+ID4gVGhlIHByb2JsZW0gc2hvd2VkIHVwIGZvciB0aGUgImF1dGgudW5peC5pcCIgY2Fj
aGUsIHRoYXQKPiA+IGhhcyBhIHJlYWRlci4KPiA+IAo+ID4gSWYgYSBzZXJ2ZXIgdGhyZWFk
IHRyaWVzIHRvIGZpbmQgYSBjYWNoZSBlbnRyeSwgaXQgZmlyc3QKPiA+IGRvZXMgYSBsb29r
dXAgKG9yIGNhbGxzIGlwX21hcF9jYWNoZWRfZ2V0KCkgaW4gdGhpcyBzcGVjaWZpYwo+ID4g
Y2FzZSkuIFRoZW4sIGl0IGNhbGxzIGNhY2hlX2NoZWNrKCkgZm9yIHRoaXMgZW50cnkuCj4g
PiAKPiA+IElmIHRoZSByZWFkZXIgdXBkYXRlcyB0aGUgY2FjaGUgZW50cnkganVzdCBiZXR3
ZWVuIHRoZQo+ID4gdGhyZWFkJ3MgbG9va3VwIGFuZCBjYWNoZV9jaGVjaygpIGV4ZWN1dGlv
biwgY2FjaGVfY2hlY2soKQo+ID4gd2lsbCBkbyBhbiB1cGNhbGwgZm9yIHRoaXMgY2FjaGUg
ZW50cnkuIFRoaXMgaXMsIGJlY2F1c2UKPiA+IHN1bnJwY19jYWNoZV91cGRhdGUoKSBjYWxs
cyBjYWNoZV9mcmVzaF9sb2NrZWQob2xkLCAwKSwKPiA+IHdoaWNoIHNldHMgZXhwaXJ5X3Rp
bWUgdG8gMC4KPiA+IAo+ID4gVW5mb3J0dW5hdGVseSwgdGhlIHJlcGx5IHRvIHRoZSB1cGNh
bGwgd2lsbCBub3QgZGVxdWV1ZQo+ID4gdGhlIHF1ZXVlZCBjYWNoZV9yZXF1ZXN0LCBhcyB0
aGUgcmVwbHkgd2lsbCBiZSBhc3NpZ25lZCB0bwo+ID4gdGhlIGNhY2hlIGVudHJ5IG5ld2x5
IGNyZWF0ZWQgYnkgdGhlIGFib3ZlIGNhY2hlIHVwZGF0ZS4KPiA+IAo+ID4gVGhlIHJlc3Vs
dCBpcyBhIGdyb3dpbmcgcXVldWUgb2Ygb3JwaGFuZWQgY2FjaGVfcmVxdWVzdAo+ID4gc3Ry
dWN0dXJlcyAtLT4gbWVtb3J5IGxlYWsuCj4gPiAKPiA+IEkgZm91bmQgdGhpcyBvbiBhIFNM
RVMxMSBTUDEgd2l0aCBhIGJhY2twb3J0IG9mIHRoZSBsYXRlc3QKPiA+IHBhdGNoZXMgdGhh
dCBmaXggdGhlIG90aGVyIFJQQyByYWNlcy4gT24gdGhpcyBvbGQga2VybmVsLAo+ID4gdGhl
IHByb2JsZW0gYWxzbyBsZWFkcyB0byBzdmNfZHJvcCgpIGJlaW5nIGNhbGxlZCBmb3IgdGhl
Cj4gPiBhZmZlY3RlZCBSUEMgcmVxdWVzdCAoYWZ0ZXIgc3ZjX2RlZmVyKCkpLgo+ID4gCj4g
PiBCZXN0IFJlZ2FyZHMKPiA+IEJvZG8KPiAKPiBJIGRvbid0IHRoaW5rIHRoaXMgaXMgYSBy
ZWFsIHByb2JsZW0uCj4gVGhlIHBlcmlvZGljIGNhbGwgdG8gImNhY2hlX2NsZWFuIiBzaG91
bGQgZmluZCB0aGVzZSBvcnBoYW5lZCByZXF1ZXN0cyBhbmQKPiBwdXJnZSB0aGVtLiAgU28g
eW91IGNvdWxkIGdldCBhIHNob3J0IHRlcm0gbWVtb3J5IGxlYWssIGJ1dCBpdCBzaG91bGQK
PiBjb3JyZWN0IGl0c2VsZi4KPiBEbyB5b3UgYWdyZWU/Cj4gCj4gVGhhbmtzLAo+IE5laWxC
cm93bgoKTWVhbndoaWxlIEkgZm91bmQgdGhlIG1pc3NpbmcgcGFydCBvZiB0aGUgcmFjZSEK
Ckl0J3MganVzdCB3aGF0IEkgd3JvdGUgYWJvdmUgYnV0IGFkZGl0aW9uYWxseSwgaW1tZWRp
YXRlbHkgYWZ0ZXIKdGhlIHJlYWRlciB1cGRhdGVkIHRoZSBjYWNoZSwgY2FjaGVfY2xlYW4o
KSB1bmhhc2hlcyB0aGUgb2xkIGNhY2hlCmVudHJ5LiBJbiB0aGF0IGNhc2U6CjEpIHRoZSB0
aHJlYWQgd2lsbCBxdWV1ZSBhIHJlcXVlc3QgZm9yIGEgY2FjaGUgZW50cnksIHRoYXQgaXNu
J3QKICAgaW4gdGhlIGhhc2ggbGlzdHMuCjIpIGNhY2hlX2NsZWFuKCkgbmV2ZXIgd2lsbCBh
Y2Nlc3MgdGhpcyBvbGQgY2FjaGUgZW50cnkgYWdhaW4KMykgZXZlcnkgZnVydGhlciBjYWNo
ZSB1cGRhdGUgd2lsbCBjYWxsIGNhY2hlX2RlcXVldWUoKSBmb3IgYSBuZXdlcgogICBjYWNo
ZSBlbnRyeSwgdGhlIHJlcXVlc3QgaXMgbmV2ZXIgZGVxdWV1ZWQKCi0tPiBtZW1vcnkgbGVh
ay4KCkkgdmVyaWZpZWQgdGhpcyBpbnNlcnRpbmcgc29tZSBkZWJ1ZyBpbnN0cnVjdGlvbnMu
IEluIGEgdGVzdHJ1bgp0aGF0IHRyaWdnZXJlZCA2IHRpbWVzLCBhbmQgdGhlIHF1ZXVlIHBy
aW50ZWQgYnkgY3Jhc2ggYWZ0ZXIgdGhlCnJ1biBoYWQgNiBvcnBoYW5lZCBlbnRyaWVzLgoK
QXMgSSB3cm90ZSB5ZXN0ZXJkYXksIEkgaGF2ZSBhIHBhdGNoIHRoYXQgc29sdmVzIHRoZSBw
cm9ibGVtIGJ5CnJldGVzdGluZyB0aGUgc3RhdGUgb2YgdGhlIGNhY2hlIGVudHJ5IGFmdGVy
IHNldHRpbmcgQ0FDSEVfUEVORElORwppbiBjYWNoZV9jaGVjaygpLiBJIGNhbiBzZW5kIGl0
IGlmIHlvdSBsaWtlLgoKQm9kbwoKUC5TLjoKTWF5YmUgSSdtIHdyb25nLCBidXQgQUZBSUNT
LCB0aGVyZSBhcmUgdHdvIGZ1cnRoZXIgbWlub3IgcHJvYmxlbXMKcmVnYXJkaW5nIChuZWFy
bHkpIGV4cGlyZWQgY2FjaGUgZW50cmllczoKYSkgaXBfbWFwX2NhY2hlZF9nZXQoKSBpbiBz
b21lIHNpdHVhdGlvbnMgY2FuIHJldHVybiBhbiBhbHJlYWR5CiAgIG91dGRhdGVkIChpdCB1
c2VzIGNhY2hlX3ZhbGlkIHRoYXQgbm90IGZ1bGx5IGNoZWNrcyB0aGUgc3RhdGUpCiAgIGNh
Y2hlIGVudHJ5LiBJZiBjYWNoZV9jaGVjaygpIGlzIGNhY2xsZWQgZm9yIHRoYXQgZW50cnks
IGl0IHdpbGwKICAgZmFpbCwgSSB0aGluawpiKSBHZW5lcmFsbHksIGlmIGEgY2FjaGUgZW50
cnkgaXMgcmV0dXJuZWQgYnkgc3VucnBjX2NhY2hlX2xvb2t1cCgpCiAgIGFuZCB0aGUgZW50
cnkgaXMgaW4gdGhlIGxhc3Qgc2Vjb25kIG9mIGl0J3MgZXhwaXJ5X3RpbWUsIHRoZSAKICAg
Y2xvY2sgbWlnaHQgbW92ZSB0byB0aGUgbmV4dCBzZWNvbmQgYmVmb3JlIGNhY2hlX2NoZWNr
IGlzIGNhbGxlZC4KICAgSWYgdGhpcyBoYXBwZW5zLCBjYWNoZV9jaGVjayB3aWxsIGZhaWws
IEkgdGhpbmsuCkRvIHlvdSBhZ3JlZT8K

Re: sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

T24gMTMgTWFyIDIwMTMgMDc6NTU6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRl
PiB3cm90ZToKCj4gT24gMTEgTWFyIDIwMTMgMTc6MTM6NDEgKzAxMDAgQm9kbyBTdHJvZXNz
ZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+Cj4gd3JvdGU6Cj4gCj4gPiBIaSwKPiA+
IAo+ID4gQUZBSUNTLCB0aGVyZSBpcyBvbmUgbW9yZSByYWNlIGluIFJQQyBjYWNoZS4KPiA+
IAo+ID4gVGhlIHByb2JsZW0gc2hvd2VkIHVwIGZvciB0aGUgImF1dGgudW5peC5pcCIgY2Fj
aGUsIHRoYXQKPiA+IGhhcyBhIHJlYWRlci4KPiA+IAo+ID4gSWYgYSBzZXJ2ZXIgdGhyZWFk
IHRyaWVzIHRvIGZpbmQgYSBjYWNoZSBlbnRyeSwgaXQgZmlyc3QKPiA+IGRvZXMgYSBsb29r
dXAgKG9yIGNhbGxzIGlwX21hcF9jYWNoZWRfZ2V0KCkgaW4gdGhpcyBzcGVjaWZpYwo+ID4g
Y2FzZSkuIFRoZW4sIGl0IGNhbGxzIGNhY2hlX2NoZWNrKCkgZm9yIHRoaXMgZW50cnkuCj4g
PiAKPiA+IElmIHRoZSByZWFkZXIgdXBkYXRlcyB0aGUgY2FjaGUgZW50cnkganVzdCBiZXR3
ZWVuIHRoZQo+ID4gdGhyZWFkJ3MgbG9va3VwIGFuZCBjYWNoZV9jaGVjaygpIGV4ZWN1dGlv
biwgY2FjaGVfY2hlY2soKQo+ID4gd2lsbCBkbyBhbiB1cGNhbGwgZm9yIHRoaXMgY2FjaGUg
ZW50cnkuIFRoaXMgaXMsIGJlY2F1c2UKPiA+IHN1bnJwY19jYWNoZV91cGRhdGUoKSBjYWxs
cyBjYWNoZV9mcmVzaF9sb2NrZWQob2xkLCAwKSwKPiA+IHdoaWNoIHNldHMgZXhwaXJ5X3Rp
bWUgdG8gMC4KPiA+IAo+ID4gVW5mb3J0dW5hdGVseSwgdGhlIHJlcGx5IHRvIHRoZSB1cGNh
bGwgd2lsbCBub3QgZGVxdWV1ZQo+ID4gdGhlIHF1ZXVlZCBjYWNoZV9yZXF1ZXN0LCBhcyB0
aGUgcmVwbHkgd2lsbCBiZSBhc3NpZ25lZCB0bwo+ID4gdGhlIGNhY2hlIGVudHJ5IG5ld2x5
IGNyZWF0ZWQgYnkgdGhlIGFib3ZlIGNhY2hlIHVwZGF0ZS4KPiA+IAo+ID4gVGhlIHJlc3Vs
dCBpcyBhIGdyb3dpbmcgcXVldWUgb2Ygb3JwaGFuZWQgY2FjaGVfcmVxdWVzdAo+ID4gc3Ry
dWN0dXJlcyAtLT4gbWVtb3J5IGxlYWsuCj4gPiAKPiA+IEkgZm91bmQgdGhpcyBvbiBhIFNM
RVMxMSBTUDEgd2l0aCBhIGJhY2twb3J0IG9mIHRoZSBsYXRlc3QKPiA+IHBhdGNoZXMgdGhh
dCBmaXggdGhlIG90aGVyIFJQQyByYWNlcy4gT24gdGhpcyBvbGQga2VybmVsLAo+ID4gdGhl
IHByb2JsZW0gYWxzbyBsZWFkcyB0byBzdmNfZHJvcCgpIGJlaW5nIGNhbGxlZCBmb3IgdGhl
Cj4gPiBhZmZlY3RlZCBSUEMgcmVxdWVzdCAoYWZ0ZXIgc3ZjX2RlZmVyKCkpLgo+ID4gCj4g
PiBCZXN0IFJlZ2FyZHMKPiA+IEJvZG8KPiAKPiBJIGRvbid0IHRoaW5rIHRoaXMgaXMgYSBy
ZWFsIHByb2JsZW0uCj4gVGhlIHBlcmlvZGljIGNhbGwgdG8gImNhY2hlX2NsZWFuIiBzaG91
bGQgZmluZCB0aGVzZSBvcnBoYW5lZCByZXF1ZXN0cyBhbmQKPiBwdXJnZSB0aGVtLiAgU28g
eW91IGNvdWxkIGdldCBhIHNob3J0IHRlcm0gbWVtb3J5IGxlYWssIGJ1dCBpdCBzaG91bGQK
PiBjb3JyZWN0IGl0c2VsZi4KPiBEbyB5b3UgYWdyZWU/Cj4gCj4gVGhhbmtzLAo+IE5laWxC
cm93bgoKSSBhZ3JlZSB0aGF0IGl0IF9zaG91bGRfIGNvcnJlY3QgaXRzZWxmLiBJIHJlYWQg
dGhlIHNvdXJjZSBhZ2FpbgphbmQgSSBub3cgc2VlIHRoYXQgY2FjaGVfY2xlYW4gc2hvdWxk
IGRvIHRoZSB3b3JrIGZpbmUuCgpPVE9ILCBvbiBteSBTTEVTMTEgU1AxLCB0aGVyZSAqaXMq
IGEgbWVtb3J5IGxlYWsgYW5kIGFsc28gUlBDCnJlcXVlc3RzIGFyZSBkcm9wcGVkICh3aGlj
aCBjYW4ndCBoYXBwZW4gb24gbWFpbmxpbmUsIGJ1dCB0aGF0Cm1hZGUgbWUgbG9vayBpbnRv
IGl0KS4KUGxlYXNlIHNlZSB0aGUgb3V0cHV0IGZyb20gY3Jhc2ggb24gdGhlIGxpZmUgc3lz
dGVtIGJlbG93LiBUaGF0Cm91dHB1dCBpcyBmcm9tIGEgdGltZSBmYXIgYWZ0ZXIgYWxsIE5G
UyBhY3Rpdml0eSB3YXMgc3RvcHBlZCBhbmQKdGhlIG5leHQgY3ljbGUgb2YgY2FjaGVfY2xl
YW4oKSBoYWQgcnVuIChpcF9tYXBfY2FjaGUubmV4dGNoZWNrCmhhcyBjaGFuZ2VkIHNldmVy
YWwgdGltZXMgZnJvbSAzMTczIHRvIHRoZSBjdXJyZW50IHZhbHVlIG9mIDEwMzczKS4KCkF0
IHRoZSBlbmQgb2YgdGhlIGNyYXNoIG91dHB1dCB5b3UnbGwgZmluZCB0d28gZXhhbXBsZXMg
b2Ygd2hhdApjYWNoZV9yZXF1ZXN0Lml0ZW0gcG9pbnRzIHRvLiBJJ3ZlIGxvb2tlZCBpbnRv
IHNvbWUgbW9yZSBvZiB0aGVtLgpUaGV5IGFsbCBoYXZlIG5leHQgPT0gMCBhbmQgcmVmY291
bnQuY291bnRlciA+PSAxIChUaGUgbWF4IGNvdW50ZXIKSSBzYXcgd2FzIDMpLgoKU28sIHRo
ZSBjYWNoZV9oZWFkIGl0ZW1zIGFsbCBhcmUgYWxyZWFkeSB1bmhhc2hlZCwgYnV0IHRoZQpj
YWNoZV9yZXF1ZXN0cyBzdGlsbCBhcmUgcXVldWVkLCByaWdodD8KCkkgaGF2ZSBubyBpZGVh
LCBob3cgdGhpcyBjb3VsZCBoYXBwZW4uIE1heWJlIG15IHBhdGNoZXMgZm9yIFNMRVMxMQpT
UDEgYXJlIG5vdCBvay4KClNvLCBiZWhpbmQgdGhlIGNyYXNoIG91dHB1dCwgSSBhbHNvIGFk
ZGVkIG15IHBhdGNoZXMuIE1heWJlIHlvdQpjb3VsZCBoYXZlIGEgbG9vayBvbnRvIGl0PwoK
VGhlcmUgaXMgb25lIG1vcmUgc21hbGwgcG9pbnQsIHRoYXQgaXNuJ3QgY2xlYXIgdG8gbWU6
IHdoZW4gdGhlCiJvbGQiIHJhY2VzIGluIGF1dGgudW5peC5naWQgY2F1c2VkIGEgTkZTIHJl
cXVlc3QgdG8gYmUgZHJvcHBlZCwKdGhlIFRDUCBjb25uZWN0aW9uIGFmdGVyIGEgZmV3IG1p
bnV0ZXMgd2FzIG1hcmtlZCBieQpzdmNfYWdlX3RlbXBfeHBydHMoKSB0byBiZSBjbG9zZWQu
CldoZW4gYSByZXF1ZXN0IGlzIGRyb3BwZWQgYmVjYXVzZSBvZiB0aGUgcmFjZSBpbiBhdXRo
LnVuaXguaXAsCnN2Y19hZ2VfdGVtcF94cHJ0cygpIGRvZXMgbm90IG1hcmsgaXQgdG8gYmUg
Y2xvc2VkLiBXaHkgZG9lcyBpdApiZWhhdmUgc28gZGlmZmVyZW50bHk/CgpUaGFuayB5b3Ug
YW5kIGJlc3QgcmVnYXJkcwpCb2RvCgpQUzoKSSBhbHNvIGhhdmUgb25lIGZ1cnRoZXIgcGF0
Y2gsIHRoYXQgZml4ZXMgdGhlIHByb2JsZW0gYnkgcmV0ZXN0aW5nCnRoZSBjYWNoZSBlbnRy
eSdzIHN0YXRlIGFmdGVyIGhhdmluZyBzZXQgQ0FDSEVfUEVORElORyBpbgpjYWNoZV9jaGVj
aygpIGFuZCBieSBleGFtaW5pbmcgZXhwaXJ5X3RpbWUgb25seSBpZiBpdCBpcyBub3QgMC4K
CgpjcmFzaD4gcHJpbnQgJmlwX21hcF9jYWNoZS5xdWV1ZQokNDMgPSAoc3RydWN0IGxpc3Rf
aGVhZCAqKSAweGZmZmZmZmZmYTA0YTE0NzgKY3Jhc2g+IGxpc3QgMCAtSCAweGZmZmZmZmZm
YTA0YTE0NzggLXMgY2FjaGVfcmVxdWVzdApmZmZmODgwMGJiZjgwMTgwCnN0cnVjdCBjYWNo
ZV9yZXF1ZXN0IHsKICBxID0gewogICAgbGlzdCA9IHsKICAgICAgbmV4dCA9IDB4ZmZmZjg4
MDBiYTI4ZTk0MCwKICAgICAgcHJldiA9IDB4ZmZmZmZmZmZhMDRhMTQ3OAogICAgfSwKICAg
IHJlYWRlciA9IDAKICB9LAogIGl0ZW0gPSAweGZmZmY4ODAwYmI4MjJjNDAsCiAgYnVmID0g
MHhmZmZmODgwMGJhYWRmMDAwICJuZnNkIDE5Mi4xNjguMTAuMlxuIiwKICBsZW4gPSAxOCwK
ICByZWFkZXJzID0gMAp9CmZmZmY4ODAwYmEyOGU5NDAKc3RydWN0IGNhY2hlX3JlcXVlc3Qg
ewogIHEgPSB7CiAgICBsaXN0ID0gewogICAgICBuZXh0ID0gMHhmZmZmODgwMGJiNWQ5Yzgw
LAogICAgICBwcmV2ID0gMHhmZmZmODgwMGJiZjgwMTgwCiAgICB9LAogICAgcmVhZGVyID0g
MAogIH0sCiAgaXRlbSA9IDB4ZmZmZjg4MDBiYmQ4ODU0MCwKICBidWYgPSAweGZmZmY4ODAw
YmVkY2IwMDAgIm5mc2QgMTkyLjE2OC4xMS4yXG4iLAogIGxlbiA9IDE4LAogIHJlYWRlcnMg
PSAwCn0KZmZmZjg4MDBiYjVkOWM4MApzdHJ1Y3QgY2FjaGVfcmVxdWVzdCB7CiAgcSA9IHsK
ICAgIGxpc3QgPSB7CiAgICAgIG5leHQgPSAweGZmZmY4ODAwYmJkMWI4MDAsCiAgICAgIHBy
ZXYgPSAweGZmZmY4ODAwYmEyOGU5NDAKICAgIH0sCiAgICByZWFkZXIgPSAwCiAgfSwKICBp
dGVtID0gMHhmZmZmODgwMjA0YjFiMzQwLAogIGJ1ZiA9IDB4ZmZmZjg4MDBiYjIxYzAwMCAi
bmZzZCAxOTIuMTY4LjEwLjJcbiIsCiAgbGVuID0gMTgsCiAgcmVhZGVycyA9IDAKfQpmZmZm
ODgwMGJiZDFiODAwCnN0cnVjdCBjYWNoZV9yZXF1ZXN0IHsKICBxID0gewogICAgbGlzdCA9
IHsKICAgICAgbmV4dCA9IDB4ZmZmZjg4MDBiNTQ0NGQwMCwKICAgICAgcHJldiA9IDB4ZmZm
Zjg4MDBiYjVkOWM4MAogICAgfSwKICAgIHJlYWRlciA9IDAKICB9LAogIGl0ZW0gPSAweGZm
ZmY4ODAwYmI5OTE5NDAsCiAgYnVmID0gMHhmZmZmODgwMGIyOGY5MDAwICJuZnNkIDE5Mi4x
NjguMTEuMlxuIiwKICBsZW4gPSAxOCwKICByZWFkZXJzID0gMAp9CmZmZmY4ODAwYjU0NDRk
MDAKc3RydWN0IGNhY2hlX3JlcXVlc3QgewogIHEgPSB7CiAgICBsaXN0ID0gewogICAgICBu
ZXh0ID0gMHhmZmZmODgwMGJmMjFjYzgwLAogICAgICBwcmV2ID0gMHhmZmZmODgwMGJiZDFi
ODAwCiAgICB9LAogICAgcmVhZGVyID0gMAogIH0sCiAgaXRlbSA9IDB4ZmZmZjg4MDIwYTM4
ODVjMCwKICBidWYgPSAweGZmZmY4ODAwYjI4ODAwMDAgIm5mc2QgMTkyLjE2OC4xMC4yXG4i
LAogIGxlbiA9IDE4LAogIHJlYWRlcnMgPSAwCn0KZmZmZjg4MDBiZjIxY2M4MApzdHJ1Y3Qg
Y2FjaGVfcmVxdWVzdCB7CiAgcSA9IHsKICAgIGxpc3QgPSB7CiAgICAgIG5leHQgPSAweGZm
ZmY4ODAwYmVkOWNiODAsCiAgICAgIHByZXYgPSAweGZmZmY4ODAwYjU0NDRkMDAKICAgIH0s
CiAgICByZWFkZXIgPSAwCiAgfSwKICBpdGVtID0gMHhmZmZmODgwMjA0MzcxZWMwLAogIGJ1
ZiA9IDB4ZmZmZjg4MDBiMTA5MzAwMCAibmZzZCAxOTIuMTY4LjExLjJcbiIsCiAgbGVuID0g
MTgsCiAgcmVhZGVycyA9IDAKfQpmZmZmODgwMGJlZDljYjgwCnN0cnVjdCBjYWNoZV9yZXF1
ZXN0IHsKICBxID0gewogICAgbGlzdCA9IHsKICAgICAgbmV4dCA9IDB4ZmZmZjg4MDBiYWMy
N2M0MCwKICAgICAgcHJldiA9IDB4ZmZmZjg4MDBiZjIxY2M4MAogICAgfSwKICAgIHJlYWRl
ciA9IDAKICB9LAogIGl0ZW0gPSAweGZmZmY4ODAyMzU5NzQxYzAsCiAgYnVmID0gMHhmZmZm
ODgwMGJiNTRjMDAwICJuZnNkIDE5Mi4xNjguMTEuMlxuIiwKICBsZW4gPSAxOCwKICByZWFk
ZXJzID0gMAp9CmZmZmY4ODAwYmFjMjdjNDAKc3RydWN0IGNhY2hlX3JlcXVlc3QgewogIHEg
PSB7CiAgICBsaXN0ID0gewogICAgICBuZXh0ID0gMHhmZmZmODgwMGJlZjFkNDgwLAogICAg
ICBwcmV2ID0gMHhmZmZmODgwMGJlZDljYjgwCiAgICB9LAogICAgcmVhZGVyID0gMAogIH0s
CiAgaXRlbSA9IDB4ZmZmZjg4MDBiYmQzYzE0MCwKICBidWYgPSAweGZmZmY4ODAwYmI4YWUw
MDAgIm5mc2QgMTkyLjE2OC4xMC4yXG4iLAogIGxlbiA9IDE4LAogIHJlYWRlcnMgPSAwCn0K
ZmZmZjg4MDBiZWYxZDQ4MApzdHJ1Y3QgY2FjaGVfcmVxdWVzdCB7CiAgcSA9IHsKICAgIGxp
c3QgPSB7CiAgICAgIG5leHQgPSAweGZmZmY4ODAwYmFhZTVmODAsCiAgICAgIHByZXYgPSAw
eGZmZmY4ODAwYmFjMjdjNDAKICAgIH0sCiAgICByZWFkZXIgPSAwCiAgfSwKICBpdGVtID0g
MHhmZmZmODgwMGJiZmQ2MmMwLAogIGJ1ZiA9IDB4ZmZmZjg4MDBiOWVjZDAwMCAibmZzZCAx
OTIuMTY4LjEwLjJcbiIsCiAgbGVuID0gMTgsCiAgcmVhZGVycyA9IDAKfQpmZmZmODgwMGJh
YWU1ZjgwCnN0cnVjdCBjYWNoZV9yZXF1ZXN0IHsKICBxID0gewogICAgbGlzdCA9IHsKICAg
ICAgbmV4dCA9IDB4ZmZmZjg4MDBiZjBjZDY4MCwKICAgICAgcHJldiA9IDB4ZmZmZjg4MDBi
ZWYxZDQ4MAogICAgfSwKICAgIHJlYWRlciA9IDAKICB9LAogIGl0ZW0gPSAweGZmZmY4ODAw
YmI4MjI0YzAsCiAgYnVmID0gMHhmZmZmODgwMGJmMjQ5MDAwICJuZnNkIDE5Mi4xNjguMTEu
MlxuIiwKICBsZW4gPSAxOCwKICByZWFkZXJzID0gMAp9CmZmZmY4ODAwYmYwY2Q2ODAKc3Ry
dWN0IGNhY2hlX3JlcXVlc3QgewogIHEgPSB7CiAgICBsaXN0ID0gewogICAgICBuZXh0ID0g
MHhmZmZmODgwMGJiOTFhNTAwLAogICAgICBwcmV2ID0gMHhmZmZmODgwMGJhYWU1ZjgwCiAg
ICB9LAogICAgcmVhZGVyID0gMAogIH0sCiAgaXRlbSA9IDB4ZmZmZjg4MDBiZjE4NjVjMCwK
ICBidWYgPSAweGZmZmY4ODAwYmIxYzgwMDAgIm5mc2QgMTkyLjE2OC4xMC4yXG4iLAogIGxl
biA9IDE4LAogIHJlYWRlcnMgPSAwCn0KZmZmZjg4MDBiYjkxYTUwMApzdHJ1Y3QgY2FjaGVf
cmVxdWVzdCB7CiAgcSA9IHsKICAgIGxpc3QgPSB7CiAgICAgIG5leHQgPSAweGZmZmY4ODAw
YjEyZjMyMDAsCiAgICAgIHByZXYgPSAweGZmZmY4ODAwYmYwY2Q2ODAKICAgIH0sCiAgICBy
ZWFkZXIgPSAwCiAgfSwKICBpdGVtID0gMHhmZmZmODgwMGJlYzdiOTQwLAogIGJ1ZiA9IDB4
ZmZmZjg4MDBiMjZiODAwMCAibmZzZCAxOTIuMTY4LjEwLjJcbiIsCiAgbGVuID0gMTgsCiAg
cmVhZGVycyA9IDAKfQpmZmZmODgwMGIxMmYzMjAwCnN0cnVjdCBjYWNoZV9yZXF1ZXN0IHsK
ICBxID0gewogICAgbGlzdCA9IHsKICAgICAgbmV4dCA9IDB4ZmZmZjg4MDIzNGM5Nzg0MCwK
ICAgICAgcHJldiA9IDB4ZmZmZjg4MDBiYjkxYTUwMAogICAgfSwKICAgIHJlYWRlciA9IDAK
ICB9LAogIGl0ZW0gPSAweGZmZmY4ODAwYjY1MTY0NDAsCiAgYnVmID0gMHhmZmZmODgwMGJi
OTMzMDAwICJuZnNkIDE5Mi4xNjguMTAuMlxuIiwKICBsZW4gPSAxOCwKICByZWFkZXJzID0g
MAp9CmZmZmY4ODAyMzRjOTc4NDAKc3RydWN0IGNhY2hlX3JlcXVlc3QgewogIHEgPSB7CiAg
ICBsaXN0ID0gewogICAgICBuZXh0ID0gMHhmZmZmZmZmZmEwNGExNDc4LAogICAgICBwcmV2
ID0gMHhmZmZmODgwMGIxMmYzMjAwCiAgICB9LAogICAgcmVhZGVyID0gMQogIH0sCiAgaXRl
bSA9IDB4MCwKICBidWYgPSAweDMxM2EzMzNhMzAzYTM4IDxBZGRyZXNzIDB4MzEzYTMzM2Ez
MDNhMzggb3V0IG9mIGJvdW5kcz4sCiAgbGVuID0gMCwKICByZWFkZXJzID0gMAp9CmNyYXNo
PiBwcmludCAqKHN0cnVjdCBjYWNoZV9oZWFkICopMHhmZmZmODgwMGI2NTE2NDQwCiQ0NCA9
IHsKICBuZXh0ID0gMHgwLAogIGV4cGlyeV90aW1lID0gMCwKICBsYXN0X3JlZnJlc2ggPSAx
MTQwLAogIHJlZiA9IHsKICAgIHJlZmNvdW50ID0gewogICAgICBjb3VudGVyID0gMQogICAg
fQogIH0sCiAgZmxhZ3MgPSA1Cn0KY3Jhc2g+IHByaW50ICooc3RydWN0IGNhY2hlX2hlYWQg
KikweGZmZmY4ODAwYmVjN2I5NDAKJDQ1ID0gewogIG5leHQgPSAweDAsCiAgZXhwaXJ5X3Rp
bWUgPSAwLAogIGxhc3RfcmVmcmVzaCA9IDExMzgsCiAgcmVmID0gewogICAgcmVmY291bnQg
PSB7CiAgICAgIGNvdW50ZXIgPSAyCiAgICB9CiAgfSwKICBmbGFncyA9IDUKfQpjcmFzaD4K
CgoKCiMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMKCgotLS0gYS9uZXQvc3VucnBjL2Nh
Y2hlLmMJMjAxMy0wMy0wNSAxMjo1NToyOC4wMDAwMDAwMDAgKzAxMDAKKysrIGIvbmV0L3N1
bnJwYy9jYWNoZS5jCTIwMTMtMDMtMDUgMTI6NTQ6NDIuMDAwMDAwMDAwICswMTAwCkBAIC05
NjIsMjMgKzk2MiwzMyBAQCBzdGF0aWMgaW50IGNhY2hlX3JlbGVhc2Uoc3RydWN0IGlub2Rl
ICppCiAKIHN0YXRpYyB2b2lkIGNhY2hlX2RlcXVldWUoc3RydWN0IGNhY2hlX2RldGFpbCAq
ZGV0YWlsLCBzdHJ1Y3QgY2FjaGVfaGVhZCAqY2gpCiB7Ci0Jc3RydWN0IGNhY2hlX3F1ZXVl
ICpjcTsKKwlzdHJ1Y3QgY2FjaGVfcXVldWUgKmNxLCAqdG1wOworCXN0cnVjdCBjYWNoZV9y
ZXF1ZXN0ICpjcjsKKwlzdHJ1Y3QgbGlzdF9oZWFkIGRlcXVldWVkOworCisJSU5JVF9MSVNU
X0hFQUQoJmRlcXVldWVkKTsKIAlzcGluX2xvY2soJnF1ZXVlX2xvY2spOwotCWxpc3RfZm9y
X2VhY2hfZW50cnkoY3EsICZkZXRhaWwtPnF1ZXVlLCBsaXN0KQorCWxpc3RfZm9yX2VhY2hf
ZW50cnlfc2FmZShjcSwgdG1wLCAmZGV0YWlsLT5xdWV1ZSwgbGlzdCkKIAkJaWYgKCFjcS0+
cmVhZGVyKSB7Ci0JCQlzdHJ1Y3QgY2FjaGVfcmVxdWVzdCAqY3IgPSBjb250YWluZXJfb2Yo
Y3EsIHN0cnVjdCBjYWNoZV9yZXF1ZXN0LCBxKTsKKwkJCWNyID0gY29udGFpbmVyX29mKGNx
LCBzdHJ1Y3QgY2FjaGVfcmVxdWVzdCwgcSk7CiAJCQlpZiAoY3ItPml0ZW0gIT0gY2gpCiAJ
CQkJY29udGludWU7CisJCQlpZiAodGVzdF9iaXQoQ0FDSEVfUEVORElORywgJmNoLT5mbGFn
cykpCisJCQkJLyogTG9zdCBhIHJhY2UgYW5kIGl0IGlzIHBlbmRpbmcgYWdhaW4gKi8KKwkJ
CQlicmVhazsKIAkJCWlmIChjci0+cmVhZGVycyAhPSAwKQogCQkJCWNvbnRpbnVlOwotCQkJ
bGlzdF9kZWwoJmNyLT5xLmxpc3QpOwotCQkJc3Bpbl91bmxvY2soJnF1ZXVlX2xvY2spOwot
CQkJY2FjaGVfcHV0KGNyLT5pdGVtLCBkZXRhaWwpOwotCQkJa2ZyZWUoY3ItPmJ1Zik7Ci0J
CQlrZnJlZShjcik7Ci0JCQlyZXR1cm47CisJCQlsaXN0X21vdmUoJmNyLT5xLmxpc3QsICZk
ZXF1ZXVlZCk7CiAJCX0KIAlzcGluX3VubG9jaygmcXVldWVfbG9jayk7CisKKwl3aGlsZSAo
IWxpc3RfZW1wdHkoJmRlcXVldWVkKSkgeworCQljciA9IGxpc3RfZW50cnkoZGVxdWV1ZWQu
bmV4dCwgc3RydWN0IGNhY2hlX3JlcXVlc3QsIHEubGlzdCk7CisJCWxpc3RfZGVsKCZjci0+
cS5saXN0KTsKKwkJY2FjaGVfcHV0KGNyLT5pdGVtLCBkZXRhaWwpOworCQlrZnJlZShjci0+
YnVmKTsKKwkJa2ZyZWUoY3IpOworCX0KIH0KIAogLyoKQEAgLTEwODEsNiArMTA5MSw3IEBA
IGludCBzdW5ycGNfY2FjaGVfcGlwZV91cGNhbGwoc3RydWN0IGNhY2gKIAlzdHJ1Y3QgY2Fj
aGVfcmVxdWVzdCAqY3JxOwogCWNoYXIgKmJwOwogCWludCBsZW47CisJaW50IHJldCA9IDA7
CiAKIAlpZiAoYXRvbWljX3JlYWQoJmRldGFpbC0+cmVhZGVycykgPT0gMCAmJgogCSAgICBk
ZXRhaWwtPmxhc3RfY2xvc2UgPCBnZXRfc2Vjb25kcygpIC0gMzApIHsKQEAgLTExMTMsMTAg
KzExMjQsMTggQEAgaW50IHN1bnJwY19jYWNoZV9waXBlX3VwY2FsbChzdHJ1Y3QgY2FjaAog
CWNycS0+bGVuID0gUEFHRV9TSVpFIC0gbGVuOwogCWNycS0+cmVhZGVycyA9IDA7CiAJc3Bp
bl9sb2NrKCZxdWV1ZV9sb2NrKTsKLQlsaXN0X2FkZF90YWlsKCZjcnEtPnEubGlzdCwgJmRl
dGFpbC0+cXVldWUpOworCWlmICh0ZXN0X2JpdChDQUNIRV9QRU5ESU5HLCAmaC0+ZmxhZ3Mp
KQorCQlsaXN0X2FkZF90YWlsKCZjcnEtPnEubGlzdCwgJmRldGFpbC0+cXVldWUpOworCWVs
c2UKKwkJLyogTG9zdCBhIHJhY2UsIG5vIGxvbmdlciBQRU5ESU5HLCBzbyBkb24ndCBlbnF1
ZXVlICovCisJCXJldCA9IC1FQUdBSU47CiAJc3Bpbl91bmxvY2soJnF1ZXVlX2xvY2spOwog
CXdha2VfdXAoJnF1ZXVlX3dhaXQpOwotCXJldHVybiAwOworCWlmIChyZXQgPT0gLUVBR0FJ
TikgeworCQlrZnJlZShidWYpOworCQlrZnJlZShjcnEpOworCX0KKwlyZXR1cm4gcmV0Owog
fQogRVhQT1JUX1NZTUJPTF9HUEwoc3VucnBjX2NhY2hlX3BpcGVfdXBjYWxsKTsKIAojIyMj
IyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjCgotLS0gYS9uZXQvc3VucnBjL2NhY2hlLmMJMjAx
My0wMy0wNSAxMzoxMDo1OC4wMDAwMDAwMDAgKzAxMDAKKysrIGIvbmV0L3N1bnJwYy9jYWNo
ZS5jCTIwMTMtMDMtMDUgMTI6NTQ6NDIuMDAwMDAwMDAwICswMTAwCkBAIC0yMzMsMTkgKzIz
MywxNiBAQCBpbnQgY2FjaGVfY2hlY2soc3RydWN0IGNhY2hlX2RldGFpbCAqZGV0CiAJCWlm
ICghdGVzdF9hbmRfc2V0X2JpdChDQUNIRV9QRU5ESU5HLCAmaC0+ZmxhZ3MpKSB7CiAJCQlz
d2l0Y2ggKGNhY2hlX21ha2VfdXBjYWxsKGRldGFpbCwgaCkpIHsKIAkJCWNhc2UgLUVJTlZB
TDoKLQkJCQljbGVhcl9iaXQoQ0FDSEVfUEVORElORywgJmgtPmZsYWdzKTsKLQkJCQljYWNo
ZV9yZXZpc2l0X3JlcXVlc3QoaCk7Ci0JCQkJaWYgKHJ2ID09IC1FQUdBSU4pIHsKKwkJCQl3
cml0ZV9sb2NrKCZkZXRhaWwtPmhhc2hfbG9jayk7CisJCQkJaWYgKHJ2KSB7CiAJCQkJCXNl
dF9iaXQoQ0FDSEVfTkVHQVRJVkUsICZoLT5mbGFncyk7CiAJCQkJCWNhY2hlX2ZyZXNoX2xv
Y2tlZChoLCBtb25vdG9uaWNfc2Vjb25kcygpK0NBQ0hFX05FV19FWFBJUlkpOwotCQkJCQlj
YWNoZV9mcmVzaF91bmxvY2tlZChoLCBkZXRhaWwpOwogCQkJCQlydiA9IC1FTk9FTlQ7CiAJ
CQkJfQotCQkJCWJyZWFrOwotCisJCQkJd3JpdGVfdW5sb2NrKCZkZXRhaWwtPmhhc2hfbG9j
ayk7CisJCQkJLyogRkFMTFRIUk9VR0ggKi8KIAkJCWNhc2UgLUVBR0FJTjoKLQkJCQljbGVh
cl9iaXQoQ0FDSEVfUEVORElORywgJmgtPmZsYWdzKTsKLQkJCQljYWNoZV9yZXZpc2l0X3Jl
cXVlc3QoaCk7CisJCQkJY2FjaGVfZnJlc2hfdW5sb2NrZWQoaCwgZGV0YWlsKTsKIAkJCQli
cmVhazsKIAkJCX0KIAkJfQpAQCAtNDA1LDE3ICs0MDIsMTIgQEAgc3RhdGljIGludCBjYWNo
ZV9jbGVhbih2b2lkKQogCQkJICAgICYmIGNoLT5sYXN0X3JlZnJlc2ggPj0gY3VycmVudF9k
ZXRhaWwtPmZsdXNoX3RpbWUKIAkJCQkpCiAJCQkJY29udGludWU7Ci0JCQlpZiAodGVzdF9h
bmRfY2xlYXJfYml0KENBQ0hFX1BFTkRJTkcsICZjaC0+ZmxhZ3MpKQotCQkJCWNhY2hlX2Rl
cXVldWUoY3VycmVudF9kZXRhaWwsIGNoKTsKIAotCQkJaWYgKGF0b21pY19yZWFkKCZjaC0+
cmVmLnJlZmNvdW50KSA9PSAxKQotCQkJCWJyZWFrOwotCQl9Ci0JCWlmIChjaCkgewogCQkJ
KmNwID0gY2gtPm5leHQ7CiAJCQljaC0+bmV4dCA9IE5VTEw7CiAJCQljdXJyZW50X2RldGFp
bC0+ZW50cmllcy0tOwogCQkJcnYgPSAxOworCQkJYnJlYWs7CiAJCX0KIAkJd3JpdGVfdW5s
b2NrKCZjdXJyZW50X2RldGFpbC0+aGFzaF9sb2NrKTsKIAkJZCA9IGN1cnJlbnRfZGV0YWls
OwpAQCAtNDIzLDcgKzQxNSw3IEBAIHN0YXRpYyBpbnQgY2FjaGVfY2xlYW4odm9pZCkKIAkJ
CWN1cnJlbnRfaW5kZXggKys7CiAJCXNwaW5fdW5sb2NrKCZjYWNoZV9saXN0X2xvY2spOwog
CQlpZiAoY2gpIHsKLQkJCWNhY2hlX3JldmlzaXRfcmVxdWVzdChjaCk7CisJCQljYWNoZV9m
cmVzaF91bmxvY2tlZChjaCwgZCk7CiAJCQljYWNoZV9wdXQoY2gsIGQpOwogCQl9CiAJfSBl
bHNlCg==

Re: sunrpc/cache.c: races while updating cache entries

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

On 11 Mar 2013 17:13:41 +0100 Bodo Stroesser <bstroesser@ts.fujitsu.com>
wrote:

> Hi,
> 
> AFAICS, there is one more race in RPC cache.
> 
> The problem showed up for the "auth.unix.ip" cache, that
> has a reader.
> 
> If a server thread tries to find a cache entry, it first
> does a lookup (or calls ip_map_cached_get() in this specific
> case). Then, it calls cache_check() for this entry.
> 
> If the reader updates the cache entry just between the
> thread's lookup and cache_check() execution, cache_check()
> will do an upcall for this cache entry. This is, because
> sunrpc_cache_update() calls cache_fresh_locked(old, 0),
> which sets expiry_time to 0.
> 
> Unfortunately, the reply to the upcall will not dequeue
> the queued cache_request, as the reply will be assigned to
> the cache entry newly created by the above cache update.
> 
> The result is a growing queue of orphaned cache_request
> structures --> memory leak.
> 
> I found this on a SLES11 SP1 with a backport of the latest
> patches that fix the other RPC races. On this old kernel,
> the problem also leads to svc_drop() being called for the
> affected RPC request (after svc_defer()).
> 
> Best Regards
> Bodo

I don't think this is a real problem.
The periodic call to "cache_clean" should find these orphaned requests and
purge them.  So you could get a short term memory leak, but it should
correct itself.
Do you agree?

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

sunrpc/cache.c: races while updating cache entries

[Bodo Stroesser]

SGksCgpBRkFJQ1MsIHRoZXJlIGlzIG9uZSBtb3JlIHJhY2UgaW4gUlBDIGNhY2hlLgoKVGhl
IHByb2JsZW0gc2hvd2VkIHVwIGZvciB0aGUgImF1dGgudW5peC5pcCIgY2FjaGUsIHRoYXQK
aGFzIGEgcmVhZGVyLgoKSWYgYSBzZXJ2ZXIgdGhyZWFkIHRyaWVzIHRvIGZpbmQgYSBjYWNo
ZSBlbnRyeSwgaXQgZmlyc3QKZG9lcyBhIGxvb2t1cCAob3IgY2FsbHMgaXBfbWFwX2NhY2hl
ZF9nZXQoKSBpbiB0aGlzIHNwZWNpZmljCmNhc2UpLiBUaGVuLCBpdCBjYWxscyBjYWNoZV9j
aGVjaygpIGZvciB0aGlzIGVudHJ5LgoKSWYgdGhlIHJlYWRlciB1cGRhdGVzIHRoZSBjYWNo
ZSBlbnRyeSBqdXN0IGJldHdlZW4gdGhlCnRocmVhZCdzIGxvb2t1cCBhbmQgY2FjaGVfY2hl
Y2soKSBleGVjdXRpb24sIGNhY2hlX2NoZWNrKCkKd2lsbCBkbyBhbiB1cGNhbGwgZm9yIHRo
aXMgY2FjaGUgZW50cnkuIFRoaXMgaXMsIGJlY2F1c2UKc3VucnBjX2NhY2hlX3VwZGF0ZSgp
IGNhbGxzIGNhY2hlX2ZyZXNoX2xvY2tlZChvbGQsIDApLAp3aGljaCBzZXRzIGV4cGlyeV90
aW1lIHRvIDAuCgpVbmZvcnR1bmF0ZWx5LCB0aGUgcmVwbHkgdG8gdGhlIHVwY2FsbCB3aWxs
IG5vdCBkZXF1ZXVlCnRoZSBxdWV1ZWQgY2FjaGVfcmVxdWVzdCwgYXMgdGhlIHJlcGx5IHdp
bGwgYmUgYXNzaWduZWQgdG8KdGhlIGNhY2hlIGVudHJ5IG5ld2x5IGNyZWF0ZWQgYnkgdGhl
IGFib3ZlIGNhY2hlIHVwZGF0ZS4KClRoZSByZXN1bHQgaXMgYSBncm93aW5nIHF1ZXVlIG9m
IG9ycGhhbmVkIGNhY2hlX3JlcXVlc3QKc3RydWN0dXJlcyAtLT4gbWVtb3J5IGxlYWsuCgpJ
IGZvdW5kIHRoaXMgb24gYSBTTEVTMTEgU1AxIHdpdGggYSBiYWNrcG9ydCBvZiB0aGUgbGF0
ZXN0CnBhdGNoZXMgdGhhdCBmaXggdGhlIG90aGVyIFJQQyByYWNlcy4gT24gdGhpcyBvbGQg
a2VybmVsLAp0aGUgcHJvYmxlbSBhbHNvIGxlYWRzIHRvIHN2Y19kcm9wKCkgYmVpbmcgY2Fs
bGVkIGZvciB0aGUKYWZmZWN0ZWQgUlBDIHJlcXVlc3QgKGFmdGVyIHN2Y19kZWZlcigpKS4K
CkJlc3QgUmVnYXJkcwpCb2RvCg==