sunrpc: remove xpt_pool

The xpt_pool field is only used for reporting BUGs.
And it isn't used correctly.

In particular, when it is cleared in svc_xprt_received before
XPT_BUSY is cleared, there is no guarantee that either the
compiler or the CPU might not re-order to two assignments, just
setting xpt_pool to NULL after XPT_BUSY is cleared.

If a different cpu were running svc_xprt_enqueue at this moment,
it might see XPT_BUSY clear and then xpt_pool non-NULL, and
so BUG.

This could be fixed by calling
  smp_mb__before_clear_bit()
before the clear_bit.  However as xpt_pool isn't really used,
it seems safest to simply remove xpt_pool.

Another alternate would be to change the clear_bit to
clear_bit_unlock, and the test_and_set_bit to test_and_set_bit_lock.

Signed-off-by: NeilBrown <neilb@suse.de>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index aea0d438e3c716b3a0f62612f15e43b9abe1e2d8..c8f81da15c7e093238b6a0d3c651dd940b6465b2 100644 (file)
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -63,7 +63,6 @@ struct svc_xprt {
 #define XPT_LISTENER   11              /* listening endpoint */
 #define XPT_CACHE_AUTH 12              /* cache auth info */
 
-       struct svc_pool         *xpt_pool;      /* current pool iff queued */
        struct svc_serv         *xpt_server;    /* service for transport */
        atomic_t                xpt_reserved;   /* space on outq that is rsvd */
        struct mutex            xpt_mutex;      /* to serialize sending data */

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 5a75d23645c82a175731003b0dc36f87984e857c..5eae53b1e306d529c7347d463898b2114083e4e2 100644 (file)
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -351,8 +351,6 @@ void svc_xprt_enqueue(struct svc_xprt *xprt)
                dprintk("svc: transport %p busy, not enqueued\n", xprt);
                goto out_unlock;
        }
-       BUG_ON(xprt->xpt_pool != NULL);
-       xprt->xpt_pool = pool;
 
        if (!list_empty(&pool->sp_threads)) {
                rqstp = list_entry(pool->sp_threads.next,
@@ -370,13 +368,11 @@ void svc_xprt_enqueue(struct svc_xprt *xprt)
                rqstp->rq_reserved = serv->sv_max_mesg;
                atomic_add(rqstp->rq_reserved, &xprt->xpt_reserved);
                pool->sp_stats.threads_woken++;
-               BUG_ON(xprt->xpt_pool != pool);
                wake_up(&rqstp->rq_wait);
        } else {
                dprintk("svc: transport %p put into queue\n", xprt);
                list_add_tail(&xprt->xpt_ready, &pool->sp_sockets);
                pool->sp_stats.sockets_queued++;
-               BUG_ON(xprt->xpt_pool != pool);
        }
 
 out_unlock:
@@ -415,7 +411,6 @@ static struct svc_xprt *svc_xprt_dequeue(struct svc_pool *pool)
 void svc_xprt_received(struct svc_xprt *xprt)
 {
        BUG_ON(!test_bit(XPT_BUSY, &xprt->xpt_flags));
-       xprt->xpt_pool = NULL;
        /* As soon as we clear busy, the xprt could be closed and
         * 'put', so we need a reference to call svc_xprt_enqueue with:
         */