From: Ben Myers <bpm@sgi.com>
To: Aaron Straus <aaron-bYFJunmd+ZV8UrSeD/g0lQ@public.gmane.org>
Cc: bfields@fieldses.org, linux-nfs@vger.kernel.org, neilb@suse.de,
Trond.Myklebust@netapp.com
Subject: [PATCH] sunrpc: xprt is not connected until after sock is set
Date: Fri, 27 Feb 2009 17:54:34 -0600 [thread overview]
Message-ID: <20090227235434.GH15475@sgi.com> (raw)
In-Reply-To: <20090226001744.GB7613-bYFJunmd+ZV8UrSeD/g0lQ@public.gmane.org>
Hi Aaron,
I'm just getting back to this.
On Wed, Feb 25, 2009 at 04:17:45PM -0800, Aaron Straus wrote:
> Quick question, do we need a barrier between setting the transport->sock
> and the xprt_set_connected(xprt)?
Yeah, looks that way. That was some interesting reading. Here is the
patch as it stands now. Hopefully I got that right. This has
apparently fixed the problem on ia64 even with out the barriers. rpciod
racing with a write.
> Also, out of curiosity, do you know what changed to introduce the BUG?
I haven't looked into that.
Bruce, can you take this patch? I understand that you're the
maintainer?
Thanks!
Ben
xs_sendpages() returned -ENOTCONN to xs_udp_send_request() and we tried to
clear a bit in transport->sock->flags when sock hadn't been set. With this
change we will instead return earlier in xprt_prepare_transmit() and the rpc
will be retried.
---
include/linux/sunrpc/xprt.h | 9 ++++++++-
net/sunrpc/xprtsock.c | 3 +--
2 files changed, 9 insertions(+), 3 deletions(-)
Index: linux-2.6.27.15-2/include/linux/sunrpc/xprt.h
===================================================================
--- linux-2.6.27.15-2.orig/include/linux/sunrpc/xprt.h
+++ linux-2.6.27.15-2/include/linux/sunrpc/xprt.h
@@ -266,17 +266,24 @@ int xs_swapper(struct rpc_xprt *xprt,
static inline void xprt_set_connected(struct rpc_xprt *xprt)
{
+ smp_wmb();
set_bit(XPRT_CONNECTED, &xprt->state);
}
static inline void xprt_clear_connected(struct rpc_xprt *xprt)
{
clear_bit(XPRT_CONNECTED, &xprt->state);
+ smp_wmb();
}
static inline int xprt_connected(struct rpc_xprt *xprt)
{
- return test_bit(XPRT_CONNECTED, &xprt->state);
+ int connected;
+
+ connected = test_bit(XPRT_CONNECTED, &xprt->state);
+ smp_rmb();
+
+ return connected;
}
static inline int xprt_test_and_set_connected(struct rpc_xprt *xprt)
Index: linux-2.6.27.15-2/net/sunrpc/xprtsock.c
===================================================================
--- linux-2.6.27.15-2.orig/net/sunrpc/xprtsock.c
+++ linux-2.6.27.15-2/net/sunrpc/xprtsock.c
@@ -1512,14 +1512,13 @@ static void xs_udp_finish_connecting(str
sk->sk_no_check = UDP_CSUM_NORCV;
sk->sk_allocation = GFP_ATOMIC;
- xprt_set_connected(xprt);
-
/* Reset to new socket */
transport->sock = sock;
transport->inet = sk;
xs_set_memalloc(xprt);
+ xprt_set_connected(xprt);
write_unlock_bh(&sk->sk_callback_lock);
}
xs_udp_do_set_buffer_size(xprt);
next prev parent reply other threads:[~2009-02-27 23:52 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-23 20:11 BUG NULL pointer dereference in SUNRPC xs_udp_send_request Aaron Straus
2009-02-23 20:11 ` Aaron Straus
[not found] ` <20090223201108.GB3308-bYFJunmd+ZV8UrSeD/g0lQ@public.gmane.org>
2009-02-25 2:39 ` Ben Myers
2009-02-25 2:39 ` Ben Myers
2009-02-26 0:17 ` Aaron Straus
2009-02-26 0:17 ` Aaron Straus
[not found] ` <20090226001744.GB7613-bYFJunmd+ZV8UrSeD/g0lQ@public.gmane.org>
2009-02-27 23:54 ` Ben Myers [this message]
2009-02-28 0:37 ` [PATCH] sunrpc: xprt is not connected until after sock is set Trond Myklebust
[not found] ` <1235781463.20549.33.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org>
2009-02-28 1:34 ` Aaron Straus
[not found] ` <20090228013457.GF7706-bYFJunmd+ZV8UrSeD/g0lQ@public.gmane.org>
2009-02-28 1:40 ` Trond Myklebust
[not found] ` <1235785237.20549.51.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org>
2009-02-28 4:57 ` Aaron Straus
2009-02-28 5:07 ` Aaron Straus
[not found] ` <20090228050707.GB22330-bYFJunmd+ZV8UrSeD/g0lQ@public.gmane.org>
2009-02-28 18:09 ` Trond Myklebust
[not found] ` <1235844568.7677.9.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org>
2009-03-02 16:36 ` Ben Myers
2009-03-02 16:39 ` Chuck Lever
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=20090227235434.GH15475@sgi.com \
--to=bpm@sgi.com \
--cc=Trond.Myklebust@netapp.com \
--cc=aaron-bYFJunmd+ZV8UrSeD/g0lQ@public.gmane.org \
--cc=bfields@fieldses.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.