Re: [PATCH 06/12] sunrpc/cache: avoid variable over-loading in cache_defer_req

J. Bruce Fields
2009-08-04T20:47:28Z

On Tue, Aug 04, 2009 at 03:22:39PM +1000, NeilBrown wrote:
> In cache_defer_req, 'dreq' is used for two significantly different
> values that happen to be of the same type.
>
> This is both confusing, and make it hard to extend the range of one of
> the values as we will in the next patch.
> So introduce 'discard' to take one of the values.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>
> net/sunrpc/cache.c | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 2f19463..4892c5c 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -561,7 +561,7 @@ static int cache_defer_cnt;
>
> static int cache_defer_req(struct cache_req *req, struct cache_head *item)
> {
> - struct cache_deferred_req *dreq;
> + struct cache_deferred_req *dreq, *discard;
> int hash = DFR_HASH(item);
>
> if (cache_defer_cnt >= DFR_MAX) {
> @@ -586,20 +586,20 @@ static int cache_defer_req(struct cache_req *req, struct cache_head *item)
> list_add(&dreq->hash, &cache_defer_hash[hash]);
>
> /* it is in, now maybe clean up */
> - dreq = NULL;
> + discard = NULL;
> if (++cache_defer_cnt > DFR_MAX) {
> - dreq = list_entry(cache_defer_list.prev,
> - struct cache_deferred_req, recent);
> + discard = list_entry(cache_defer_list.prev,
> + struct cache_deferred_req, recent);
> list_del_init(&dreq->recent);
> list_del_init(&dreq->hash);

Shouldn't these two "dreq"'s be "discard"'s as well?

--b.

> cache_defer_cnt--;
> }
> spin_unlock(&cache_defer_lock);
>
> - if (dreq) {
> + if (discard)
> /* there was one too many */
> - dreq->revisit(dreq, 1);
> - }
> + discard->revisit(discard, 1);
> +
> if (!test_bit(CACHE_PENDING, &item->flags)) {
> /* must have just been validated... */
> cache_revisit_request(item);
>
>