From: NeilBrown <neilb@suse.de>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-nfs@vger.kernel.org, NeilBrown <neilb@suse.de>
Subject: [PATCH 1/2] sunrpc: remove xpt_pool
Date: Mon, 15 Nov 2010 11:27:01 +1100 [thread overview]
Message-ID: <20101115002701.19121.46414.stgit@notabene.brown> (raw)
In-Reply-To: <20101115002634.19121.7027.stgit@notabene.brown>
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>
---
include/linux/sunrpc/svc_xprt.h | 1 -
net/sunrpc/svc_xprt.c | 6 ------
2 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index bbdb680..79baf68 100644
--- 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 c82fe73..ec9d8ef 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -342,8 +342,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;
/* Handle pending connection */
if (test_bit(XPT_CONN, &xprt->xpt_flags))
@@ -358,7 +356,6 @@ void svc_xprt_enqueue(struct svc_xprt *xprt)
/* Don't enqueue while not enough space for reply */
dprintk("svc: no write space, transport %p not enqueued\n",
xprt);
- xprt->xpt_pool = NULL;
clear_bit(XPT_BUSY, &xprt->xpt_flags);
goto out_unlock;
}
@@ -380,13 +377,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:
@@ -425,7 +420,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;
clear_bit(XPT_BUSY, &xprt->xpt_flags);
svc_xprt_enqueue(xprt);
}
next prev parent reply other threads:[~2010-11-15 0:27 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-15 0:27 [PATCH 0/2] sunrpc: fix two server-side race problems NeilBrown
2010-11-15 0:27 ` [PATCH 2/2] sunrpc: svc_sock_names should hold ref to socket being closed NeilBrown
2010-11-15 0:27 ` NeilBrown [this message]
2010-11-16 6:07 ` [PATCH 0/2] sunrpc: fix two server-side race problems Neil Brown
2010-11-16 15:04 ` J. Bruce Fields
2010-11-16 22:24 ` Neil Brown
2010-11-16 15:33 ` J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101115002701.19121.46414.stgit@notabene.brown \
--to=neilb@suse.de \
--cc=bfields@fieldses.org \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).