2013-03-04 06:10:40

by NeilBrown

[permalink] [raw]
Subject: [PATCH 0/2] two fixes for races in SUNRPC. - Take 2.

hi,
these two patches fix (I believe) the races recently reported
by Bodo Stroesser.

This version removes the 'return' from cache_deqeue() as discussed.

Thanks,
NeilBrown


---

NeilBrown (2):
sunrpc/cache: remove races with queuing an upcall.
sunrpc/cache: use cache_fresh_unlocked consistently and correctly.


net/sunrpc/cache.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)

--
Signature



2013-03-04 06:10:47

by NeilBrown

[permalink] [raw]
Subject: [PATCH 1/2] sunrpc/cache: remove races with queuing an upcall.

We currently queue an upcall after setting CACHE_PENDING,
and dequeue after clearing CACHE_PENDING.
So a request should only be present when CACHE_PENDING is set.

However we don't combine the test and the enqueue/dequeue in
a protected region, so it is possible (if unlikely) for a race
to result in a request being queued without CACHE_PENDING set,
or a request to be absent despite CACHE_PENDING.

So: include a test for CACHE_PENDING inside the regions of
enqueue and dequeue where queue_lock is held, and abort
the operation if the value is not as expected.

Also remove the 'return' from cache_dequeue() to ensure it always
removes all entries: As there is no locking between setting
CACHE_PENDING and calling sunrpc_cache_pipe_upcall it is not
inconceivable for some other thread to clear CACHE_PENDING and then
someone else to set it can call sunrpc_cache_pipe_upcall, both before
the original thread completed the call.

With this, it perfectly safe and correct to:
- call cache_dequeue() if and only if we have just
cleared CACHE_PENDING
- call sunrpc_cache_pipe_upcall() (via cache_make_upcall)
if and only if we have just set CACHE_PENDING.

Reported-by: Bodo Stroesser <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
net/sunrpc/cache.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 9afa439..0400a92 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1022,6 +1022,9 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch)
struct cache_request *cr = container_of(cq, struct cache_request, q);
if (cr->item != ch)
continue;
+ if (test_bit(CACHE_PENDING, &ch->flags))
+ /* Lost a race and it is pending again */
+ break;
if (cr->readers != 0)
continue;
list_del(&cr->q.list);
@@ -1029,7 +1032,6 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch)
cache_put(cr->item, detail);
kfree(cr->buf);
kfree(cr);
- return;
}
spin_unlock(&queue_lock);
}
@@ -1151,6 +1153,7 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
struct cache_request *crq;
char *bp;
int len;
+ int ret = 0;

if (!cache_listeners_exist(detail)) {
warn_no_listener(detail);
@@ -1182,10 +1185,18 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
crq->len = PAGE_SIZE - len;
crq->readers = 0;
spin_lock(&queue_lock);
- list_add_tail(&crq->q.list, &detail->queue);
+ if (test_bit(CACHE_PENDING, &h->flags))
+ list_add_tail(&crq->q.list, &detail->queue);
+ else
+ /* Lost a race, no longer PENDING, so don't enqueue */
+ ret = -EAGAIN;
spin_unlock(&queue_lock);
wake_up(&queue_wait);
- return 0;
+ if (ret == -EAGAIN) {
+ kfree(buf);
+ kfree(crq);
+ }
+ return ret;
}
EXPORT_SYMBOL_GPL(sunrpc_cache_pipe_upcall);




2013-03-13 05:59:05

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 1/2] sunrpc/cache: remove races with queuing an upcall.

On 05 Mar 2013 16:07:39 +0100 Bodo Stroesser <[email protected]>
wrote:

> Hi,
>
> the patch in my previous mail was a bit too optimized,
> in that it gets an cache_request* pointer from each entry
> on the detail->queue, even if it really is a cache_reader.
>
> As the resulting pointer is used as a cache_request* after
> checking cache_queue->reader only, this isn't a bug, but
> it is unclean.
>
> Thus I changed the patch below once again to keep the sane
> handling as it has been implemented.
>
> Bodo
>
>
> On 04 Mar 2013 07:10:00 +0100 NeilBrown <[email protected]> wrote:
>
> > We currently queue an upcall after setting CACHE_PENDING,
> > and dequeue after clearing CACHE_PENDING.
> > So a request should only be present when CACHE_PENDING is set.
> >
> > However we don't combine the test and the enqueue/dequeue in
> > a protected region, so it is possible (if unlikely) for a race
> > to result in a request being queued without CACHE_PENDING set,
> > or a request to be absent despite CACHE_PENDING.
> >
> > So: include a test for CACHE_PENDING inside the regions of
> > enqueue and dequeue where queue_lock is held, and abort
> > the operation if the value is not as expected.
> >
> > Also remove the 'return' from cache_dequeue() to ensure it always
> > removes all entries: As there is no locking between setting
> > CACHE_PENDING and calling sunrpc_cache_pipe_upcall it is not
> > inconceivable for some other thread to clear CACHE_PENDING and then
> > someone else to set it can call sunrpc_cache_pipe_upcall, both before
> > the original thread completed the call.
> >
> > With this, it perfectly safe and correct to:
> > - call cache_dequeue() if and only if we have just
> > cleared CACHE_PENDING
> > - call sunrpc_cache_pipe_upcall() (via cache_make_upcall)
> > if and only if we have just set CACHE_PENDING.
> >
> > Reported-by: Bodo Stroesser <[email protected]>
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > net/sunrpc/cache.c | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> > index 9afa439..0400a92 100644
> > --- a/net/sunrpc/cache.c
> > +++ b/net/sunrpc/cache.c
> > @@ -1022,6 +1022,9 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch)
> > struct cache_request *cr = container_of(cq, struct cache_request, q);
> > if (cr->item != ch)
> > continue;
> > + if (test_bit(CACHE_PENDING, &ch->flags))
> > + /* Lost a race and it is pending again */
> > + break;
> > if (cr->readers != 0)
> > continue;
> > list_del(&cr->q.list);
> > @@ -1029,7 +1032,6 @@ static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch)
> > cache_put(cr->item, detail);
> > kfree(cr->buf);
> > kfree(cr);
> > - return;
>
> Just removing "return" is not enough. If the loop no longer stops
> after dequeueing the first entry found, some other changes are
> necessary also:
>
> 1) use list_for_each_entry_safe() instead of list_for_each_entry()
>
> 2) don't call spin_unlock() in the loop.
>
> Thus, at the end of this mail I added a revised patch. With this
> patch cache_dequeue() no longer frees the requests in the loop,
> but moves them to a temporary queue.
> After the loop it calls spin_unlock() and does the kfree() and
> cache_put() in a second loop.
>
> The patch is not tested on a mainline kernel. Instead, I
> ported it to SLES11 SP1 2.6.32.59-0.7.1. On SLES 11 the test
> is still running fine.
>
> Best Regards,
> Bodo
>
> > }
> > spin_unlock(&queue_lock);
> > }
> > @@ -1151,6 +1153,7 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
> > struct cache_request *crq;
> > char *bp;
> > int len;
> > + int ret = 0;
> >
> > if (!cache_listeners_exist(detail)) {
> > warn_no_listener(detail);
> > @@ -1182,10 +1185,18 @@ int sunrpc_cache_pipe_upcall(struct cache_detail *detail, struct cache_head *h,
> > crq->len = PAGE_SIZE - len;
> > crq->readers = 0;
> > spin_lock(&queue_lock);
> > - list_add_tail(&crq->q.list, &detail->queue);
> > + if (test_bit(CACHE_PENDING, &h->flags))
> > + list_add_tail(&crq->q.list, &detail->queue);
> > + else
> > + /* Lost a race, no longer PENDING, so don't enqueue */
> > + ret = -EAGAIN;
> > spin_unlock(&queue_lock);
> > wake_up(&queue_wait);
> > - return 0;
> > + if (ret == -EAGAIN) {
> > + kfree(buf);
> > + kfree(crq);
> > + }
> > + return ret;
> > }
> > EXPORT_SYMBOL_GPL(sunrpc_cache_pipe_upcall);
> >
> >
>
>
> .....
>
> Reported-by: Bodo Stroesser <[email protected]>
> Signed-off-by: NeilBrown <[email protected]>
> Signed-off-by: Bodo Stroesser <[email protected]>
> ---
>
> --- a/net/sunrpc/cache.c 2013-02-20 14:05:08.000000000 +0100
> +++ b/net/sunrpc/cache.c 2013-03-05 13:30:13.000000000 +0100
> @@ -1015,23 +1015,33 @@
>
> static void cache_dequeue(struct cache_detail *detail, struct cache_head *ch)
> {
> - struct cache_queue *cq;
> + struct cache_queue *cq, *tmp;
> + struct cache_request *cr;
> + struct list_head dequeued;
> +
> + INIT_LIST_HEAD(&dequeued);
> spin_lock(&queue_lock);
> - list_for_each_entry(cq, &detail->queue, list)
> + list_for_each_entry_safe(cq, tmp, &detail->queue, list)
> if (!cq->reader) {
> - struct cache_request *cr = container_of(cq, struct cache_request, q);
> + cr = container_of(cq, struct cache_request, q);
> if (cr->item != ch)
> continue;
> + if (test_bit(CACHE_PENDING, &ch->flags))
> + /* Lost a race and it is pending again */
> + break;
> if (cr->readers != 0)
> continue;
> - list_del(&cr->q.list);
> - spin_unlock(&queue_lock);
> - cache_put(cr->item, detail);
> - kfree(cr->buf);
> - kfree(cr);
> - return;
> + list_move(&cr->q.list, &dequeued);
> }
> spin_unlock(&queue_lock);
> +
> + while (!list_empty(&dequeued)) {
> + cr = list_entry(dequeued.next, struct cache_request, q.list);
> + list_del(&cr->q.list);
> + cache_put(cr->item, detail);
> + kfree(cr->buf);
> + kfree(cr);
> + }
> }
>
> /*
> @@ -1151,6 +1161,7 @@
> struct cache_request *crq;
> char *bp;
> int len;
> + int ret = 0;
>
> if (!cache_listeners_exist(detail)) {
> warn_no_listener(detail);
> @@ -1182,10 +1193,18 @@
> crq->len = PAGE_SIZE - len;
> crq->readers = 0;
> spin_lock(&queue_lock);
> - list_add_tail(&crq->q.list, &detail->queue);
> + if (test_bit(CACHE_PENDING, &h->flags))
> + list_add_tail(&crq->q.list, &detail->queue);
> + else
> + /* Lost a race, no longer PENDING, so don't enqueue */
> + ret = -EAGAIN;
> spin_unlock(&queue_lock);
> wake_up(&queue_wait);
> - return 0;
> + if (ret == -EAGAIN) {
> + kfree(buf);
> + kfree(crq);
> + }
> + return ret;
> }
> EXPORT_SYMBOL_GPL(sunrpc_cache_pipe_upcall);
>


Thanks for this - that was careless of me. sorry.

Your patch looks good. I'll forward the two to Bruce again shortly, then
look at submitting a backport for SLES.

Thanks,
NeilBrown


Attachments:
signature.asc (828.00 B)

2013-03-05 13:57:22

by Bodo Stroesser

[permalink] [raw]
Subject: Re: [PATCH 1/2] sunrpc/cache: remove races with queuing an upcall.

T24gMDQgTWFyIDIwMTMgMDc6MTA6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRl
PiB3cm90ZToKCj4gV2UgY3VycmVudGx5IHF1ZXVlIGFuIHVwY2FsbCBhZnRlciBzZXR0aW5n
IENBQ0hFX1BFTkRJTkcsCj4gYW5kIGRlcXVldWUgYWZ0ZXIgY2xlYXJpbmcgQ0FDSEVfUEVO
RElORy4KPiBTbyBhIHJlcXVlc3Qgc2hvdWxkIG9ubHkgYmUgcHJlc2VudCB3aGVuIENBQ0hF
X1BFTkRJTkcgaXMgc2V0Lgo+IAo+IEhvd2V2ZXIgd2UgZG9uJ3QgY29tYmluZSB0aGUgdGVz
dCBhbmQgdGhlIGVucXVldWUvZGVxdWV1ZSBpbgo+IGEgcHJvdGVjdGVkIHJlZ2lvbiwgc28g
aXQgaXMgcG9zc2libGUgKGlmIHVubGlrZWx5KSBmb3IgYSByYWNlCj4gdG8gcmVzdWx0IGlu
IGEgcmVxdWVzdCBiZWluZyBxdWV1ZWQgd2l0aG91dCBDQUNIRV9QRU5ESU5HIHNldCwKPiBv
ciBhIHJlcXVlc3QgdG8gYmUgYWJzZW50IGRlc3BpdGUgQ0FDSEVfUEVORElORy4KPiAKPiBT
bzogaW5jbHVkZSBhIHRlc3QgZm9yIENBQ0hFX1BFTkRJTkcgaW5zaWRlIHRoZSByZWdpb25z
IG9mCj4gZW5xdWV1ZSBhbmQgZGVxdWV1ZSB3aGVyZSBxdWV1ZV9sb2NrIGlzIGhlbGQsIGFu
ZCBhYm9ydAo+IHRoZSBvcGVyYXRpb24gaWYgdGhlIHZhbHVlIGlzIG5vdCBhcyBleHBlY3Rl
ZC4KPiAKPiBBbHNvIHJlbW92ZSB0aGUgJ3JldHVybicgZnJvbSBjYWNoZV9kZXF1ZXVlKCkg
dG8gZW5zdXJlIGl0IGFsd2F5cwo+IHJlbW92ZXMgYWxsIGVudHJpZXM6ICBBcyB0aGVyZSBp
cyBubyBsb2NraW5nIGJldHdlZW4gc2V0dGluZwo+IENBQ0hFX1BFTkRJTkcgYW5kIGNhbGxp
bmcgc3VucnBjX2NhY2hlX3BpcGVfdXBjYWxsIGl0IGlzIG5vdAo+IGluY29uY2VpdmFibGUg
Zm9yIHNvbWUgb3RoZXIgdGhyZWFkIHRvIGNsZWFyIENBQ0hFX1BFTkRJTkcgYW5kIHRoZW4K
PiBzb21lb25lIGVsc2UgdG8gc2V0IGl0IGNhbiBjYWxsIHN1bnJwY19jYWNoZV9waXBlX3Vw
Y2FsbCwgYm90aCBiZWZvcmUKPiB0aGUgb3JpZ2luYWwgdGhyZWFkIGNvbXBsZXRlZCB0aGUg
Y2FsbC4KPiAKPiBXaXRoIHRoaXMsIGl0IHBlcmZlY3RseSBzYWZlIGFuZCBjb3JyZWN0IHRv
Ogo+ICAtIGNhbGwgY2FjaGVfZGVxdWV1ZSgpIGlmIGFuZCBvbmx5IGlmIHdlIGhhdmUganVz
dAo+ICAgIGNsZWFyZWQgQ0FDSEVfUEVORElORwo+ICAtIGNhbGwgc3VucnBjX2NhY2hlX3Bp
cGVfdXBjYWxsKCkgKHZpYSBjYWNoZV9tYWtlX3VwY2FsbCkKPiAgICBpZiBhbmQgb25seSBp
ZiB3ZSBoYXZlIGp1c3Qgc2V0IENBQ0hFX1BFTkRJTkcuCj4gCj4gUmVwb3J0ZWQtYnk6IEJv
ZG8gU3Ryb2Vzc2VyIDxic3Ryb2Vzc2VyQHRzLmZ1aml0c3UuY29tPgo+IFNpZ25lZC1vZmYt
Ynk6IE5laWxCcm93biA8bmVpbGJAc3VzZS5kZT4KPiAtLS0KPiAgbmV0L3N1bnJwYy9jYWNo
ZS5jIHwgICAxNyArKysrKysrKysrKysrKy0tLQo+ICAxIGZpbGUgY2hhbmdlZCwgMTQgaW5z
ZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvbmV0L3N1bnJw
Yy9jYWNoZS5jIGIvbmV0L3N1bnJwYy9jYWNoZS5jCj4gaW5kZXggOWFmYTQzOS4uMDQwMGE5
MiAxMDA2NDQKPiAtLS0gYS9uZXQvc3VucnBjL2NhY2hlLmMKPiArKysgYi9uZXQvc3VucnBj
L2NhY2hlLmMKPiBAQCAtMTAyMiw2ICsxMDIyLDkgQEAgc3RhdGljIHZvaWQgY2FjaGVfZGVx
dWV1ZShzdHJ1Y3QgY2FjaGVfZGV0YWlsICpkZXRhaWwsIHN0cnVjdCBjYWNoZV9oZWFkICpj
aCkKPiAgCQkJc3RydWN0IGNhY2hlX3JlcXVlc3QgKmNyID0gY29udGFpbmVyX29mKGNxLCBz
dHJ1Y3QgY2FjaGVfcmVxdWVzdCwgcSk7Cj4gIAkJCWlmIChjci0+aXRlbSAhPSBjaCkKPiAg
CQkJCWNvbnRpbnVlOwo+ICsJCQlpZiAodGVzdF9iaXQoQ0FDSEVfUEVORElORywgJmNoLT5m
bGFncykpCj4gKwkJCQkvKiBMb3N0IGEgcmFjZSBhbmQgaXQgaXMgcGVuZGluZyBhZ2FpbiAq
Lwo+ICsJCQkJYnJlYWs7Cj4gIAkJCWlmIChjci0+cmVhZGVycyAhPSAwKQo+ICAJCQkJY29u
dGludWU7Cj4gIAkJCWxpc3RfZGVsKCZjci0+cS5saXN0KTsKPiBAQCAtMTAyOSw3ICsxMDMy
LDYgQEAgc3RhdGljIHZvaWQgY2FjaGVfZGVxdWV1ZShzdHJ1Y3QgY2FjaGVfZGV0YWlsICpk
ZXRhaWwsIHN0cnVjdCBjYWNoZV9oZWFkICpjaCkKPiAgCQkJY2FjaGVfcHV0KGNyLT5pdGVt
LCBkZXRhaWwpOwo+ICAJCQlrZnJlZShjci0+YnVmKTsKPiAgCQkJa2ZyZWUoY3IpOwo+IC0J
CQlyZXR1cm47CgpKdXN0IHJlbW92aW5nICJyZXR1cm4iIGlzIG5vdCBlbm91Z2guIElmIHRo
ZSBsb29wIG5vIGxvbmdlciBzdG9wcwphZnRlciBkZXF1ZXVlaW5nIHRoZSBmaXJzdCBlbnRy
eSBmb3VuZCwgc29tZSBvdGhlciBjaGFuZ2VzIGFyZQpuZWNlc3NhcnkgYWxzbzoKCjEpIHVz
ZSBsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUoKSBpbnN0ZWFkIG9mIGxpc3RfZm9yX2VhY2hf
ZW50cnkoKQoKMikgZG9uJ3QgY2FsbCBzcGluX3VubG9jaygpIGluIHRoZSBsb29wLgoKVGh1
cywgYXQgdGhlIGVuZCBvZiB0aGlzIG1haWwgSSBhZGRlZCBhIHJldmlzZWQgcGF0Y2guIFdp
dGggdGhpcwpwYXRjaCBjYWNoZV9kZXF1ZXVlKCkgbm8gbG9uZ2VyIGZyZWVzIHRoZSByZXF1
ZXN0cyBpbiB0aGUgbG9vcCwKYnV0IG1vdmVzIHRoZW0gdG8gYSB0ZW1wb3JhcnkgcXVldWUu
CkFmdGVyIHRoZSBsb29wIGl0IGNhbGxzIHNwaW5fdW5sb2NrKCkgYW5kIGRvZXMgdGhlIGtm
cmVlKCkgYW5kCmNhY2hlX3B1dCgpIGluIGEgc2Vjb25kIGxvb3AuCgpUaGUgcGF0Y2ggaXMg
bm90IHRlc3RlZCBvbiBhIG1haW5saW5lIGtlcm5lbC4gSW5zdGVhZCwgSQpwb3J0ZWQgaXQg
dG8gU0xFUzExIFNQMSAyLjYuMzIuNTktMC43LjEuIE9uIFNMRVMgMTEgdGhlIHRlc3QKaXMg
c3RpbGwgcnVubmluZyBmaW5lLgoKQmVzdCBSZWdhcmRzLApCb2RvCgo+ICAJCX0KPiAgCXNw
aW5fdW5sb2NrKCZxdWV1ZV9sb2NrKTsKPiAgfQo+IEBAIC0xMTUxLDYgKzExNTMsNyBAQCBp
bnQgc3VucnBjX2NhY2hlX3BpcGVfdXBjYWxsKHN0cnVjdCBjYWNoZV9kZXRhaWwgKmRldGFp
bCwgc3RydWN0IGNhY2hlX2hlYWQgKmgsCj4gIAlzdHJ1Y3QgY2FjaGVfcmVxdWVzdCAqY3Jx
Owo+ICAJY2hhciAqYnA7Cj4gIAlpbnQgbGVuOwo+ICsJaW50IHJldCA9IDA7Cj4gIAo+ICAJ
aWYgKCFjYWNoZV9saXN0ZW5lcnNfZXhpc3QoZGV0YWlsKSkgewo+ICAJCXdhcm5fbm9fbGlz
dGVuZXIoZGV0YWlsKTsKPiBAQCAtMTE4MiwxMCArMTE4NSwxOCBAQCBpbnQgc3VucnBjX2Nh
Y2hlX3BpcGVfdXBjYWxsKHN0cnVjdCBjYWNoZV9kZXRhaWwgKmRldGFpbCwgc3RydWN0IGNh
Y2hlX2hlYWQgKmgsCj4gIAljcnEtPmxlbiA9IFBBR0VfU0laRSAtIGxlbjsKPiAgCWNycS0+
cmVhZGVycyA9IDA7Cj4gIAlzcGluX2xvY2soJnF1ZXVlX2xvY2spOwo+IC0JbGlzdF9hZGRf
dGFpbCgmY3JxLT5xLmxpc3QsICZkZXRhaWwtPnF1ZXVlKTsKPiArCWlmICh0ZXN0X2JpdChD
QUNIRV9QRU5ESU5HLCAmaC0+ZmxhZ3MpKQo+ICsJCWxpc3RfYWRkX3RhaWwoJmNycS0+cS5s
aXN0LCAmZGV0YWlsLT5xdWV1ZSk7Cj4gKwllbHNlCj4gKwkJLyogTG9zdCBhIHJhY2UsIG5v
IGxvbmdlciBQRU5ESU5HLCBzbyBkb24ndCBlbnF1ZXVlICovCj4gKwkJcmV0ID0gLUVBR0FJ
TjsKPiAgCXNwaW5fdW5sb2NrKCZxdWV1ZV9sb2NrKTsKPiAgCXdha2VfdXAoJnF1ZXVlX3dh
aXQpOwo+IC0JcmV0dXJuIDA7Cj4gKwlpZiAocmV0ID09IC1FQUdBSU4pIHsKPiArCQlrZnJl
ZShidWYpOwo+ICsJCWtmcmVlKGNycSk7Cj4gKwl9Cj4gKwlyZXR1cm4gcmV0Owo+ICB9Cj4g
IEVYUE9SVF9TWU1CT0xfR1BMKHN1bnJwY19jYWNoZV9waXBlX3VwY2FsbCk7Cj4gIAo+IAoK
Ci4uLi4uCgpSZXBvcnRlZC1ieTogQm9kbyBTdHJvZXNzZXIgPGJzdHJvZXNzZXJAdHMuZnVq
aXRzdS5jb20+ClNpZ25lZC1vZmYtYnk6IE5laWxCcm93biA8bmVpbGJAc3VzZS5kZT4KU2ln
bmVkLW9mZi1ieTogQm9kbyBTdHJvZXNzZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+
Ci0tLQoKLS0tIGEvbmV0L3N1bnJwYy9jYWNoZS5jCTIwMTMtMDItMjAgMTQ6MDU6MDguMDAw
MDAwMDAwICswMTAwCisrKyBiL25ldC9zdW5ycGMvY2FjaGUuYwkyMDEzLTAzLTA1IDEzOjMw
OjEzLjAwMDAwMDAwMCArMDEwMApAQCAtMTAxNSwyMyArMTAxNSwzMSBAQAogCiBzdGF0aWMg
dm9pZCBjYWNoZV9kZXF1ZXVlKHN0cnVjdCBjYWNoZV9kZXRhaWwgKmRldGFpbCwgc3RydWN0
IGNhY2hlX2hlYWQgKmNoKQogewotCXN0cnVjdCBjYWNoZV9xdWV1ZSAqY3E7CisJc3RydWN0
IGNhY2hlX3JlcXVlc3QgKmNyLCAqdG1wOworCXN0cnVjdCBsaXN0X2hlYWQgZGVxdWV1ZWQ7
CisKKwlJTklUX0xJU1RfSEVBRCgmZGVxdWV1ZWQpOwogCXNwaW5fbG9jaygmcXVldWVfbG9j
ayk7Ci0JbGlzdF9mb3JfZWFjaF9lbnRyeShjcSwgJmRldGFpbC0+cXVldWUsIGxpc3QpCi0J
CWlmICghY3EtPnJlYWRlcikgewotCQkJc3RydWN0IGNhY2hlX3JlcXVlc3QgKmNyID0gY29u
dGFpbmVyX29mKGNxLCBzdHJ1Y3QgY2FjaGVfcmVxdWVzdCwgcSk7CisJbGlzdF9mb3JfZWFj
aF9lbnRyeV9zYWZlKGNyLCB0bXAsICZkZXRhaWwtPnF1ZXVlLCBxLmxpc3QpCisJCWlmICgh
Y3ItPnEucmVhZGVyKSB7CiAJCQlpZiAoY3ItPml0ZW0gIT0gY2gpCiAJCQkJY29udGludWU7
CisJCQlpZiAodGVzdF9iaXQoQ0FDSEVfUEVORElORywgJmNoLT5mbGFncykpCisJCQkJLyog
TG9zdCBhIHJhY2UgYW5kIGl0IGlzIHBlbmRpbmcgYWdhaW4gKi8KKwkJCQlicmVhazsKIAkJ
CWlmIChjci0+cmVhZGVycyAhPSAwKQogCQkJCWNvbnRpbnVlOwotCQkJbGlzdF9kZWwoJmNy
LT5xLmxpc3QpOwotCQkJc3Bpbl91bmxvY2soJnF1ZXVlX2xvY2spOwotCQkJY2FjaGVfcHV0
KGNyLT5pdGVtLCBkZXRhaWwpOwotCQkJa2ZyZWUoY3ItPmJ1Zik7Ci0JCQlrZnJlZShjcik7
Ci0JCQlyZXR1cm47CisJCQlsaXN0X21vdmUoJmNyLT5xLmxpc3QsICZkZXF1ZXVlZCk7CiAJ
CX0KIAlzcGluX3VubG9jaygmcXVldWVfbG9jayk7CisKKwl3aGlsZSAoIWxpc3RfZW1wdHko
JmRlcXVldWVkKSkgeworCQljciA9IGxpc3RfZW50cnkoZGVxdWV1ZWQubmV4dCwgc3RydWN0
IGNhY2hlX3JlcXVlc3QsIHEubGlzdCk7CisJCWxpc3RfZGVsKCZjci0+cS5saXN0KTsKKwkJ
Y2FjaGVfcHV0KGNyLT5pdGVtLCBkZXRhaWwpOworCQlrZnJlZShjci0+YnVmKTsKKwkJa2Zy
ZWUoY3IpOworCX0KIH0KIAogLyoKQEAgLTExNTEsNiArMTE1OSw3IEBACiAJc3RydWN0IGNh
Y2hlX3JlcXVlc3QgKmNycTsKIAljaGFyICpicDsKIAlpbnQgbGVuOworCWludCByZXQgPSAw
OwogCiAJaWYgKCFjYWNoZV9saXN0ZW5lcnNfZXhpc3QoZGV0YWlsKSkgewogCQl3YXJuX25v
X2xpc3RlbmVyKGRldGFpbCk7CkBAIC0xMTgyLDEwICsxMTkxLDE4IEBACiAJY3JxLT5sZW4g
PSBQQUdFX1NJWkUgLSBsZW47CiAJY3JxLT5yZWFkZXJzID0gMDsKIAlzcGluX2xvY2soJnF1
ZXVlX2xvY2spOwotCWxpc3RfYWRkX3RhaWwoJmNycS0+cS5saXN0LCAmZGV0YWlsLT5xdWV1
ZSk7CisJaWYgKHRlc3RfYml0KENBQ0hFX1BFTkRJTkcsICZoLT5mbGFncykpCisJCWxpc3Rf
YWRkX3RhaWwoJmNycS0+cS5saXN0LCAmZGV0YWlsLT5xdWV1ZSk7CisJZWxzZQorCQkvKiBM
b3N0IGEgcmFjZSwgbm8gbG9uZ2VyIFBFTkRJTkcsIHNvIGRvbid0IGVucXVldWUgKi8KKwkJ
cmV0ID0gLUVBR0FJTjsKIAlzcGluX3VubG9jaygmcXVldWVfbG9jayk7CiAJd2FrZV91cCgm
cXVldWVfd2FpdCk7Ci0JcmV0dXJuIDA7CisJaWYgKHJldCA9PSAtRUFHQUlOKSB7CisJCWtm
cmVlKGJ1Zik7CisJCWtmcmVlKGNycSk7CisJfQorCXJldHVybiByZXQ7CiB9CiBFWFBPUlRf
U1lNQk9MX0dQTChzdW5ycGNfY2FjaGVfcGlwZV91cGNhbGwpOwogCg==



2013-03-05 15:07:44

by Bodo Stroesser

[permalink] [raw]
Subject: Re: [PATCH 1/2] sunrpc/cache: remove races with queuing an upcall.

SGksCgp0aGUgcGF0Y2ggaW4gbXkgcHJldmlvdXMgbWFpbCB3YXMgYSBiaXQgdG9vIG9wdGlt
aXplZCwKaW4gdGhhdCBpdCBnZXRzIGFuIGNhY2hlX3JlcXVlc3QqIHBvaW50ZXIgZnJvbSBl
YWNoIGVudHJ5Cm9uIHRoZSBkZXRhaWwtPnF1ZXVlLCBldmVuIGlmIGl0IHJlYWxseSBpcyBh
IGNhY2hlX3JlYWRlci4KCkFzIHRoZSByZXN1bHRpbmcgcG9pbnRlciBpcyB1c2VkIGFzIGEg
Y2FjaGVfcmVxdWVzdCogYWZ0ZXIKY2hlY2tpbmcgY2FjaGVfcXVldWUtPnJlYWRlciBvbmx5
LCB0aGlzIGlzbid0IGEgYnVnLCBidXQKaXQgaXMgdW5jbGVhbi4KClRodXMgSSBjaGFuZ2Vk
IHRoZSBwYXRjaCBiZWxvdyBvbmNlIGFnYWluIHRvIGtlZXAgdGhlIHNhbmUKaGFuZGxpbmcg
YXMgaXQgaGFzIGJlZW4gaW1wbGVtZW50ZWQuCgpCb2RvCgoKT24gMDQgTWFyIDIwMTMgMDc6
MTA6MDAgKzAxMDAgTmVpbEJyb3duIDxuZWlsYkBzdXNlLmRlPiB3cm90ZToKCj4gV2UgY3Vy
cmVudGx5IHF1ZXVlIGFuIHVwY2FsbCBhZnRlciBzZXR0aW5nIENBQ0hFX1BFTkRJTkcsCj4g
YW5kIGRlcXVldWUgYWZ0ZXIgY2xlYXJpbmcgQ0FDSEVfUEVORElORy4KPiBTbyBhIHJlcXVl
c3Qgc2hvdWxkIG9ubHkgYmUgcHJlc2VudCB3aGVuIENBQ0hFX1BFTkRJTkcgaXMgc2V0Lgo+
IAo+IEhvd2V2ZXIgd2UgZG9uJ3QgY29tYmluZSB0aGUgdGVzdCBhbmQgdGhlIGVucXVldWUv
ZGVxdWV1ZSBpbgo+IGEgcHJvdGVjdGVkIHJlZ2lvbiwgc28gaXQgaXMgcG9zc2libGUgKGlm
IHVubGlrZWx5KSBmb3IgYSByYWNlCj4gdG8gcmVzdWx0IGluIGEgcmVxdWVzdCBiZWluZyBx
dWV1ZWQgd2l0aG91dCBDQUNIRV9QRU5ESU5HIHNldCwKPiBvciBhIHJlcXVlc3QgdG8gYmUg
YWJzZW50IGRlc3BpdGUgQ0FDSEVfUEVORElORy4KPiAKPiBTbzogaW5jbHVkZSBhIHRlc3Qg
Zm9yIENBQ0hFX1BFTkRJTkcgaW5zaWRlIHRoZSByZWdpb25zIG9mCj4gZW5xdWV1ZSBhbmQg
ZGVxdWV1ZSB3aGVyZSBxdWV1ZV9sb2NrIGlzIGhlbGQsIGFuZCBhYm9ydAo+IHRoZSBvcGVy
YXRpb24gaWYgdGhlIHZhbHVlIGlzIG5vdCBhcyBleHBlY3RlZC4KPiAKPiBBbHNvIHJlbW92
ZSB0aGUgJ3JldHVybicgZnJvbSBjYWNoZV9kZXF1ZXVlKCkgdG8gZW5zdXJlIGl0IGFsd2F5
cwo+IHJlbW92ZXMgYWxsIGVudHJpZXM6ICBBcyB0aGVyZSBpcyBubyBsb2NraW5nIGJldHdl
ZW4gc2V0dGluZwo+IENBQ0hFX1BFTkRJTkcgYW5kIGNhbGxpbmcgc3VucnBjX2NhY2hlX3Bp
cGVfdXBjYWxsIGl0IGlzIG5vdAo+IGluY29uY2VpdmFibGUgZm9yIHNvbWUgb3RoZXIgdGhy
ZWFkIHRvIGNsZWFyIENBQ0hFX1BFTkRJTkcgYW5kIHRoZW4KPiBzb21lb25lIGVsc2UgdG8g
c2V0IGl0IGNhbiBjYWxsIHN1bnJwY19jYWNoZV9waXBlX3VwY2FsbCwgYm90aCBiZWZvcmUK
PiB0aGUgb3JpZ2luYWwgdGhyZWFkIGNvbXBsZXRlZCB0aGUgY2FsbC4KPiAKPiBXaXRoIHRo
aXMsIGl0IHBlcmZlY3RseSBzYWZlIGFuZCBjb3JyZWN0IHRvOgo+ICAtIGNhbGwgY2FjaGVf
ZGVxdWV1ZSgpIGlmIGFuZCBvbmx5IGlmIHdlIGhhdmUganVzdAo+ICAgIGNsZWFyZWQgQ0FD
SEVfUEVORElORwo+ICAtIGNhbGwgc3VucnBjX2NhY2hlX3BpcGVfdXBjYWxsKCkgKHZpYSBj
YWNoZV9tYWtlX3VwY2FsbCkKPiAgICBpZiBhbmQgb25seSBpZiB3ZSBoYXZlIGp1c3Qgc2V0
IENBQ0hFX1BFTkRJTkcuCj4gCj4gUmVwb3J0ZWQtYnk6IEJvZG8gU3Ryb2Vzc2VyIDxic3Ry
b2Vzc2VyQHRzLmZ1aml0c3UuY29tPgo+IFNpZ25lZC1vZmYtYnk6IE5laWxCcm93biA8bmVp
bGJAc3VzZS5kZT4KPiAtLS0KPiAgbmV0L3N1bnJwYy9jYWNoZS5jIHwgICAxNyArKysrKysr
KysrKysrKy0tLQo+ICAxIGZpbGUgY2hhbmdlZCwgMTQgaW5zZXJ0aW9ucygrKSwgMyBkZWxl
dGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvbmV0L3N1bnJwYy9jYWNoZS5jIGIvbmV0L3N1
bnJwYy9jYWNoZS5jCj4gaW5kZXggOWFmYTQzOS4uMDQwMGE5MiAxMDA2NDQKPiAtLS0gYS9u
ZXQvc3VucnBjL2NhY2hlLmMKPiArKysgYi9uZXQvc3VucnBjL2NhY2hlLmMKPiBAQCAtMTAy
Miw2ICsxMDIyLDkgQEAgc3RhdGljIHZvaWQgY2FjaGVfZGVxdWV1ZShzdHJ1Y3QgY2FjaGVf
ZGV0YWlsICpkZXRhaWwsIHN0cnVjdCBjYWNoZV9oZWFkICpjaCkKPiAgCQkJc3RydWN0IGNh
Y2hlX3JlcXVlc3QgKmNyID0gY29udGFpbmVyX29mKGNxLCBzdHJ1Y3QgY2FjaGVfcmVxdWVz
dCwgcSk7Cj4gIAkJCWlmIChjci0+aXRlbSAhPSBjaCkKPiAgCQkJCWNvbnRpbnVlOwo+ICsJ
CQlpZiAodGVzdF9iaXQoQ0FDSEVfUEVORElORywgJmNoLT5mbGFncykpCj4gKwkJCQkvKiBM
b3N0IGEgcmFjZSBhbmQgaXQgaXMgcGVuZGluZyBhZ2FpbiAqLwo+ICsJCQkJYnJlYWs7Cj4g
IAkJCWlmIChjci0+cmVhZGVycyAhPSAwKQo+ICAJCQkJY29udGludWU7Cj4gIAkJCWxpc3Rf
ZGVsKCZjci0+cS5saXN0KTsKPiBAQCAtMTAyOSw3ICsxMDMyLDYgQEAgc3RhdGljIHZvaWQg
Y2FjaGVfZGVxdWV1ZShzdHJ1Y3QgY2FjaGVfZGV0YWlsICpkZXRhaWwsIHN0cnVjdCBjYWNo
ZV9oZWFkICpjaCkKPiAgCQkJY2FjaGVfcHV0KGNyLT5pdGVtLCBkZXRhaWwpOwo+ICAJCQlr
ZnJlZShjci0+YnVmKTsKPiAgCQkJa2ZyZWUoY3IpOwo+IC0JCQlyZXR1cm47CgpKdXN0IHJl
bW92aW5nICJyZXR1cm4iIGlzIG5vdCBlbm91Z2guIElmIHRoZSBsb29wIG5vIGxvbmdlciBz
dG9wcwphZnRlciBkZXF1ZXVlaW5nIHRoZSBmaXJzdCBlbnRyeSBmb3VuZCwgc29tZSBvdGhl
ciBjaGFuZ2VzIGFyZQpuZWNlc3NhcnkgYWxzbzoKCjEpIHVzZSBsaXN0X2Zvcl9lYWNoX2Vu
dHJ5X3NhZmUoKSBpbnN0ZWFkIG9mIGxpc3RfZm9yX2VhY2hfZW50cnkoKQoKMikgZG9uJ3Qg
Y2FsbCBzcGluX3VubG9jaygpIGluIHRoZSBsb29wLgoKVGh1cywgYXQgdGhlIGVuZCBvZiB0
aGlzIG1haWwgSSBhZGRlZCBhIHJldmlzZWQgcGF0Y2guIFdpdGggdGhpcwpwYXRjaCBjYWNo
ZV9kZXF1ZXVlKCkgbm8gbG9uZ2VyIGZyZWVzIHRoZSByZXF1ZXN0cyBpbiB0aGUgbG9vcCwK
YnV0IG1vdmVzIHRoZW0gdG8gYSB0ZW1wb3JhcnkgcXVldWUuCkFmdGVyIHRoZSBsb29wIGl0
IGNhbGxzIHNwaW5fdW5sb2NrKCkgYW5kIGRvZXMgdGhlIGtmcmVlKCkgYW5kCmNhY2hlX3B1
dCgpIGluIGEgc2Vjb25kIGxvb3AuCgpUaGUgcGF0Y2ggaXMgbm90IHRlc3RlZCBvbiBhIG1h
aW5saW5lIGtlcm5lbC4gSW5zdGVhZCwgSQpwb3J0ZWQgaXQgdG8gU0xFUzExIFNQMSAyLjYu
MzIuNTktMC43LjEuIE9uIFNMRVMgMTEgdGhlIHRlc3QKaXMgc3RpbGwgcnVubmluZyBmaW5l
LgoKQmVzdCBSZWdhcmRzLApCb2RvCgo+ICAJCX0KPiAgCXNwaW5fdW5sb2NrKCZxdWV1ZV9s
b2NrKTsKPiAgfQo+IEBAIC0xMTUxLDYgKzExNTMsNyBAQCBpbnQgc3VucnBjX2NhY2hlX3Bp
cGVfdXBjYWxsKHN0cnVjdCBjYWNoZV9kZXRhaWwgKmRldGFpbCwgc3RydWN0IGNhY2hlX2hl
YWQgKmgsCj4gIAlzdHJ1Y3QgY2FjaGVfcmVxdWVzdCAqY3JxOwo+ICAJY2hhciAqYnA7Cj4g
IAlpbnQgbGVuOwo+ICsJaW50IHJldCA9IDA7Cj4gIAo+ICAJaWYgKCFjYWNoZV9saXN0ZW5l
cnNfZXhpc3QoZGV0YWlsKSkgewo+ICAJCXdhcm5fbm9fbGlzdGVuZXIoZGV0YWlsKTsKPiBA
QCAtMTE4MiwxMCArMTE4NSwxOCBAQCBpbnQgc3VucnBjX2NhY2hlX3BpcGVfdXBjYWxsKHN0
cnVjdCBjYWNoZV9kZXRhaWwgKmRldGFpbCwgc3RydWN0IGNhY2hlX2hlYWQgKmgsCj4gIAlj
cnEtPmxlbiA9IFBBR0VfU0laRSAtIGxlbjsKPiAgCWNycS0+cmVhZGVycyA9IDA7Cj4gIAlz
cGluX2xvY2soJnF1ZXVlX2xvY2spOwo+IC0JbGlzdF9hZGRfdGFpbCgmY3JxLT5xLmxpc3Qs
ICZkZXRhaWwtPnF1ZXVlKTsKPiArCWlmICh0ZXN0X2JpdChDQUNIRV9QRU5ESU5HLCAmaC0+
ZmxhZ3MpKQo+ICsJCWxpc3RfYWRkX3RhaWwoJmNycS0+cS5saXN0LCAmZGV0YWlsLT5xdWV1
ZSk7Cj4gKwllbHNlCj4gKwkJLyogTG9zdCBhIHJhY2UsIG5vIGxvbmdlciBQRU5ESU5HLCBz
byBkb24ndCBlbnF1ZXVlICovCj4gKwkJcmV0ID0gLUVBR0FJTjsKPiAgCXNwaW5fdW5sb2Nr
KCZxdWV1ZV9sb2NrKTsKPiAgCXdha2VfdXAoJnF1ZXVlX3dhaXQpOwo+IC0JcmV0dXJuIDA7
Cj4gKwlpZiAocmV0ID09IC1FQUdBSU4pIHsKPiArCQlrZnJlZShidWYpOwo+ICsJCWtmcmVl
KGNycSk7Cj4gKwl9Cj4gKwlyZXR1cm4gcmV0Owo+ICB9Cj4gIEVYUE9SVF9TWU1CT0xfR1BM
KHN1bnJwY19jYWNoZV9waXBlX3VwY2FsbCk7Cj4gIAo+IAoKCi4uLi4uCgpSZXBvcnRlZC1i
eTogQm9kbyBTdHJvZXNzZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+ClNpZ25lZC1v
ZmYtYnk6IE5laWxCcm93biA8bmVpbGJAc3VzZS5kZT4KU2lnbmVkLW9mZi1ieTogQm9kbyBT
dHJvZXNzZXIgPGJzdHJvZXNzZXJAdHMuZnVqaXRzdS5jb20+Ci0tLQoKLS0tIGEvbmV0L3N1
bnJwYy9jYWNoZS5jCTIwMTMtMDItMjAgMTQ6MDU6MDguMDAwMDAwMDAwICswMTAwCisrKyBi
L25ldC9zdW5ycGMvY2FjaGUuYwkyMDEzLTAzLTA1IDEzOjMwOjEzLjAwMDAwMDAwMCArMDEw
MApAQCAtMTAxNSwyMyArMTAxNSwzMyBAQAogCiBzdGF0aWMgdm9pZCBjYWNoZV9kZXF1ZXVl
KHN0cnVjdCBjYWNoZV9kZXRhaWwgKmRldGFpbCwgc3RydWN0IGNhY2hlX2hlYWQgKmNoKQog
ewotCXN0cnVjdCBjYWNoZV9xdWV1ZSAqY3E7CisJc3RydWN0IGNhY2hlX3F1ZXVlICpjcSwg
KnRtcDsKKwlzdHJ1Y3QgY2FjaGVfcmVxdWVzdCAqY3I7CisJc3RydWN0IGxpc3RfaGVhZCBk
ZXF1ZXVlZDsKKworCUlOSVRfTElTVF9IRUFEKCZkZXF1ZXVlZCk7CiAJc3Bpbl9sb2NrKCZx
dWV1ZV9sb2NrKTsKLQlsaXN0X2Zvcl9lYWNoX2VudHJ5KGNxLCAmZGV0YWlsLT5xdWV1ZSwg
bGlzdCkKKwlsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUoY3EsIHRtcCwgJmRldGFpbC0+cXVl
dWUsIGxpc3QpCiAJCWlmICghY3EtPnJlYWRlcikgewotCQkJc3RydWN0IGNhY2hlX3JlcXVl
c3QgKmNyID0gY29udGFpbmVyX29mKGNxLCBzdHJ1Y3QgY2FjaGVfcmVxdWVzdCwgcSk7CisJ
CQljciA9IGNvbnRhaW5lcl9vZihjcSwgc3RydWN0IGNhY2hlX3JlcXVlc3QsIHEpOwogCQkJ
aWYgKGNyLT5pdGVtICE9IGNoKQogCQkJCWNvbnRpbnVlOworCQkJaWYgKHRlc3RfYml0KENB
Q0hFX1BFTkRJTkcsICZjaC0+ZmxhZ3MpKQorCQkJCS8qIExvc3QgYSByYWNlIGFuZCBpdCBp
cyBwZW5kaW5nIGFnYWluICovCisJCQkJYnJlYWs7CiAJCQlpZiAoY3ItPnJlYWRlcnMgIT0g
MCkKIAkJCQljb250aW51ZTsKLQkJCWxpc3RfZGVsKCZjci0+cS5saXN0KTsKLQkJCXNwaW5f
dW5sb2NrKCZxdWV1ZV9sb2NrKTsKLQkJCWNhY2hlX3B1dChjci0+aXRlbSwgZGV0YWlsKTsK
LQkJCWtmcmVlKGNyLT5idWYpOwotCQkJa2ZyZWUoY3IpOwotCQkJcmV0dXJuOworCQkJbGlz
dF9tb3ZlKCZjci0+cS5saXN0LCAmZGVxdWV1ZWQpOwogCQl9CiAJc3Bpbl91bmxvY2soJnF1
ZXVlX2xvY2spOworCisJd2hpbGUgKCFsaXN0X2VtcHR5KCZkZXF1ZXVlZCkpIHsKKwkJY3Ig
PSBsaXN0X2VudHJ5KGRlcXVldWVkLm5leHQsIHN0cnVjdCBjYWNoZV9yZXF1ZXN0LCBxLmxp
c3QpOworCQlsaXN0X2RlbCgmY3ItPnEubGlzdCk7CisJCWNhY2hlX3B1dChjci0+aXRlbSwg
ZGV0YWlsKTsKKwkJa2ZyZWUoY3ItPmJ1Zik7CisJCWtmcmVlKGNyKTsKKwl9CiB9CiAKIC8q
CkBAIC0xMTUxLDYgKzExNjEsNyBAQAogCXN0cnVjdCBjYWNoZV9yZXF1ZXN0ICpjcnE7CiAJ
Y2hhciAqYnA7CiAJaW50IGxlbjsKKwlpbnQgcmV0ID0gMDsKIAogCWlmICghY2FjaGVfbGlz
dGVuZXJzX2V4aXN0KGRldGFpbCkpIHsKIAkJd2Fybl9ub19saXN0ZW5lcihkZXRhaWwpOwpA
QCAtMTE4MiwxMCArMTE5MywxOCBAQAogCWNycS0+bGVuID0gUEFHRV9TSVpFIC0gbGVuOwog
CWNycS0+cmVhZGVycyA9IDA7CiAJc3Bpbl9sb2NrKCZxdWV1ZV9sb2NrKTsKLQlsaXN0X2Fk
ZF90YWlsKCZjcnEtPnEubGlzdCwgJmRldGFpbC0+cXVldWUpOworCWlmICh0ZXN0X2JpdChD
QUNIRV9QRU5ESU5HLCAmaC0+ZmxhZ3MpKQorCQlsaXN0X2FkZF90YWlsKCZjcnEtPnEubGlz
dCwgJmRldGFpbC0+cXVldWUpOworCWVsc2UKKwkJLyogTG9zdCBhIHJhY2UsIG5vIGxvbmdl
ciBQRU5ESU5HLCBzbyBkb24ndCBlbnF1ZXVlICovCisJCXJldCA9IC1FQUdBSU47CiAJc3Bp
bl91bmxvY2soJnF1ZXVlX2xvY2spOwogCXdha2VfdXAoJnF1ZXVlX3dhaXQpOwotCXJldHVy
biAwOworCWlmIChyZXQgPT0gLUVBR0FJTikgeworCQlrZnJlZShidWYpOworCQlrZnJlZShj
cnEpOworCX0KKwlyZXR1cm4gcmV0OwogfQogRVhQT1JUX1NZTUJPTF9HUEwoc3VucnBjX2Nh
Y2hlX3BpcGVfdXBjYWxsKTsKIAo=



2013-03-04 06:10:55

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/2] sunrpc/cache: use cache_fresh_unlocked consistently and correctly.

cache_fresh_unlocked() is called when a cache entry
has been updated and ensures that if there were any
pending upcalls, they are cleared.

So every time we update a cache entry, we should call this,
and this should be the only way that we try to clear
pending calls (that sort of uniformity makes code sooo much
easier to read).

try_to_negate_entry() will (possibly) mark an entry as
negative. If it doesn't, it is because the entry already
is VALID.
So the entry will be valid on exit, so it is appropriate to
call cache_fresh_unlocked().
So tidy up try_to_negate_entry() to do that, and remove
partial open-coded cache_fresh_unlocked() from the one
call-site of try_to_negate_entry().

In the other branch of the 'switch(cache_make_upcall())',
we again have a partial open-coded version of cache_fresh_unlocked().
Replace that with a real call.

And again in cache_clean(), use a real call to cache_fresh_unlocked().

These call sites might previously have called
cache_revisit_request() if CACHE_PENDING wasn't set.
This is never necessary because cache_revisit_request() can
only do anything if the item is in the cache_defer_hash,
However any time that an item is added to the cache_defer_hash
(setup_deferral), the code immediately tests CACHE_PENDING,
and removes the entry again if it is clear. So all other
places we only need to 'cache_revisit_request' if we've
just cleared CACHE_PENDING.

Reported-by: Bodo Stroesser <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
net/sunrpc/cache.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 0400a92..2e07ba1 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -228,15 +228,14 @@ static int try_to_negate_entry(struct cache_detail *detail, struct cache_head *h

write_lock(&detail->hash_lock);
rv = cache_is_valid(detail, h);
- if (rv != -EAGAIN) {
- write_unlock(&detail->hash_lock);
- return rv;
+ if (rv == -EAGAIN) {
+ set_bit(CACHE_NEGATIVE, &h->flags);
+ cache_fresh_locked(h, seconds_since_boot()+CACHE_NEW_EXPIRY);
+ rv = -ENOENT;
}
- set_bit(CACHE_NEGATIVE, &h->flags);
- cache_fresh_locked(h, seconds_since_boot()+CACHE_NEW_EXPIRY);
write_unlock(&detail->hash_lock);
cache_fresh_unlocked(h, detail);
- return -ENOENT;
+ return rv;
}

/*
@@ -275,13 +274,10 @@ int cache_check(struct cache_detail *detail,
if (!test_and_set_bit(CACHE_PENDING, &h->flags)) {
switch (cache_make_upcall(detail, h)) {
case -EINVAL:
- clear_bit(CACHE_PENDING, &h->flags);
- cache_revisit_request(h);
rv = try_to_negate_entry(detail, h);
break;
case -EAGAIN:
- clear_bit(CACHE_PENDING, &h->flags);
- cache_revisit_request(h);
+ cache_fresh_unlocked(h, detail);
break;
}
}
@@ -457,9 +453,7 @@ static int cache_clean(void)
current_index ++;
spin_unlock(&cache_list_lock);
if (ch) {
- if (test_and_clear_bit(CACHE_PENDING, &ch->flags))
- cache_dequeue(current_detail, ch);
- cache_revisit_request(ch);
+ cache_fresh_unlocked(ch, d);
cache_put(ch, d);
}
} else