From: Trond Myklebust <trondmy@primarydata.com>
To: "anna.schumaker@netapp.com" <anna.schumaker@netapp.com>,
"chuck.lever@oracle.com" <chuck.lever@oracle.com>
Cc: "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v3 09/12] sunrpc: Allow xprt->ops->timer method to sleep
Date: Wed, 8 Feb 2017 23:48:02 +0000 [thread overview]
Message-ID: <1486597679.11028.1.camel@primarydata.com> (raw)
In-Reply-To: <20170208220051.7152.67740.stgit@manet.1015granger.net>
T24gV2VkLCAyMDE3LTAyLTA4IGF0IDE3OjAwIC0wNTAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
VGhlIHRyYW5zcG9ydCBsb2NrIGlzIG5lZWRlZCB0byBwcm90ZWN0IHRoZSB4cHJ0X2FkanVzdF9j
d25kKCkgY2FsbA0KPiBpbiB4c191ZHBfdGltZXIsIGJ1dCBpdCBpcyBub3QgbmVjZXNzYXJ5IGZv
ciBhY2Nlc3NpbmcgdGhlDQo+IHJxX3JlcGx5X2J5dGVzX3JlY3ZkIG9yIHRrX3N0YXR1cyBmaWVs
ZHMuIEl0IGlzIGNvcnJlY3QgdG8gc3VibGltYXRlDQo+IHRoZSBsb2NrIGludG8gVURQJ3MgeHNf
dWRwX3RpbWVyIG1ldGhvZCwgd2hlcmUgaXQgaXMgcmVxdWlyZWQuDQo+IA0KPiBUaGUgLT50aW1l
ciBtZXRob2QgaGFzIHRvIHRha2UgdGhlIHRyYW5zcG9ydCBsb2NrIGlmIG5lZWRlZCwgYnV0IGl0
DQo+IGNhbiBub3cgc2xlZXAgc2FmZWx5LCBvciBldmVuIGNhbGwgYmFjayBpbnRvIHRoZSBSUEMg
c2NoZWR1bGVyLg0KPiANCj4gVGhpcyBpcyBtb3JlIGEgY2xlYW4tdXAgdGhhbiBhIGZpeCwgYnV0
IHRoZSAiaXNzdWUiIHdhcyBpbnRyb2R1Y2VkDQo+IGJ5IG15IHRyYW5zcG9ydCBzd2l0Y2ggcGF0
Y2hlcyBiYWNrIGluIDIwMDUuDQo+IA0KPiBGaXhlczogNDZjMGVlOGJjNGFkICgiUlBDOiBzZXBh
cmF0ZSB4cHJ0X3RpbWVyIGltcGxlbWVudGF0aW9ucyIpDQo+IFNpZ25lZC1vZmYtYnk6IENodWNr
IExldmVyIDxjaHVjay5sZXZlckBvcmFjbGUuY29tPg0KPiAtLS0NCj4gwqBuZXQvc3VucnBjL3hw
cnQuY8KgwqDCoMKgwqB8wqDCoMKgwqAyIC0tDQo+IMKgbmV0L3N1bnJwYy94cHJ0c29jay5jIHzC
oMKgwqDCoDIgKysNCj4gwqAyIGZpbGVzIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgMiBkZWxl
dGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9uZXQvc3VucnBjL3hwcnQuYyBiL25ldC9zdW5y
cGMveHBydC5jDQo+IGluZGV4IDlhNmJlMDMuLmI1MzBhMjggMTAwNjQ0DQo+IC0tLSBhL25ldC9z
dW5ycGMveHBydC5jDQo+ICsrKyBiL25ldC9zdW5ycGMveHBydC5jDQo+IEBAIC04OTcsMTMgKzg5
NywxMSBAQCBzdGF0aWMgdm9pZCB4cHJ0X3RpbWVyKHN0cnVjdCBycGNfdGFzayAqdGFzaykNCj4g
wqAJCXJldHVybjsNCj4gwqAJZHByaW50aygiUlBDOiAlNXUgeHBydF90aW1lclxuIiwgdGFzay0+
dGtfcGlkKTsNCj4gwqANCj4gLQlzcGluX2xvY2tfYmgoJnhwcnQtPnRyYW5zcG9ydF9sb2NrKTsN
Cj4gwqAJaWYgKCFyZXEtPnJxX3JlcGx5X2J5dGVzX3JlY3ZkKSB7DQo+IMKgCQlpZiAoeHBydC0+
b3BzLT50aW1lcikNCj4gwqAJCQl4cHJ0LT5vcHMtPnRpbWVyKHhwcnQsIHRhc2spOw0KPiDCoAl9
IGVsc2UNCj4gwqAJCXRhc2stPnRrX3N0YXR1cyA9IDA7DQo+IC0Jc3Bpbl91bmxvY2tfYmgoJnhw
cnQtPnRyYW5zcG9ydF9sb2NrKTsNCj4gwqB9DQo+IMKgDQo+IMKgLyoqDQo+IGRpZmYgLS1naXQg
YS9uZXQvc3VucnBjL3hwcnRzb2NrLmMgYi9uZXQvc3VucnBjL3hwcnRzb2NrLmMNCj4gaW5kZXgg
YWYzOTJkOS4uZDliYjY0NCAxMDA2NDQNCj4gLS0tIGEvbmV0L3N1bnJwYy94cHJ0c29jay5jDQo+
ICsrKyBiL25ldC9zdW5ycGMveHBydHNvY2suYw0KPiBAQCAtMTczNCw3ICsxNzM0LDkgQEAgc3Rh
dGljIHZvaWQgeHNfdWRwX3NldF9idWZmZXJfc2l6ZShzdHJ1Y3QNCj4gcnBjX3hwcnQgKnhwcnQs
IHNpemVfdCBzbmRzaXplLCBzaXplX3QNCj4gwqAgKi8NCj4gwqBzdGF0aWMgdm9pZCB4c191ZHBf
dGltZXIoc3RydWN0IHJwY194cHJ0ICp4cHJ0LCBzdHJ1Y3QgcnBjX3Rhc2sNCj4gKnRhc2spDQo+
IMKgew0KPiArCXNwaW5fbG9ja19iaCgmeHBydC0+dHJhbnNwb3J0X2xvY2spOw0KPiDCoAl4cHJ0
X2FkanVzdF9jd25kKHhwcnQsIHRhc2ssIC1FVElNRURPVVQpOw0KPiArCXNwaW5fdW5sb2NrX2Jo
KCZ4cHJ0LT50cmFuc3BvcnRfbG9jayk7DQo+IMKgfQ0KPiDCoA0KPiDCoHN0YXRpYyB1bnNpZ25l
ZCBzaG9ydCB4c19nZXRfcmFuZG9tX3BvcnQodm9pZCkNCj4gDQoNClRoYW5rcyEgR29vZCBjbGVh
bnVwLi4uDQoNClRyb25kDQoNCi0tIA0KDQoNCg0KCQ0KCQ0KDQoNClRyb25kIE15a2xlYnVzdA0K
UHJpbmNpcGFsIFN5c3RlbSBBcmNoaXRlY3QNCjQzMDAgRWwgQ2FtaW5vIFJlYWwgfCBTdWl0ZSAx
MDANCkxvcyBBbHRvcywgQ0HCoMKgOTQwMjINClc6IDY1MC00MjItMzgwMA0KQzogODAxLTkyMS00
NTgzwqANCnd3dy5wcmltYXJ5ZGF0YS5jb20NCg0KDQoNCg0K
WARNING: multiple messages have this Message-ID (diff)
From: Trond Myklebust <trondmy-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
To: "anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org"
<anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>,
"chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org"
<chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: "linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH v3 09/12] sunrpc: Allow xprt->ops->timer method to sleep
Date: Wed, 8 Feb 2017 23:48:02 +0000 [thread overview]
Message-ID: <1486597679.11028.1.camel@primarydata.com> (raw)
In-Reply-To: <20170208220051.7152.67740.stgit-FYjufvaPoItvLzlybtyyYzGyq/o6K9yX@public.gmane.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 2335 bytes --]
On Wed, 2017-02-08 at 17:00 -0500, Chuck Lever wrote:
> The transport lock is needed to protect the xprt_adjust_cwnd() call
> in xs_udp_timer, but it is not necessary for accessing the
> rq_reply_bytes_recvd or tk_status fields. It is correct to sublimate
> the lock into UDP's xs_udp_timer method, where it is required.
>
> The ->timer method has to take the transport lock if needed, but it
> can now sleep safely, or even call back into the RPC scheduler.
>
> This is more a clean-up than a fix, but the "issue" was introduced
> by my transport switch patches back in 2005.
>
> Fixes: 46c0ee8bc4ad ("RPC: separate xprt_timer implementations")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/xprt.c     |    2 --
> Â net/sunrpc/xprtsock.c |Â Â Â Â 2 ++
> Â 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index 9a6be03..b530a28 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -897,13 +897,11 @@ static void xprt_timer(struct rpc_task *task)
> Â return;
> Â dprintk("RPC: %5u xprt_timer\n", task->tk_pid);
> Â
> - spin_lock_bh(&xprt->transport_lock);
> Â if (!req->rq_reply_bytes_recvd) {
> Â if (xprt->ops->timer)
> Â xprt->ops->timer(xprt, task);
> Â } else
> Â task->tk_status = 0;
> - spin_unlock_bh(&xprt->transport_lock);
> Â }
> Â
> Â /**
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index af392d9..d9bb644 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1734,7 +1734,9 @@ static void xs_udp_set_buffer_size(struct
> rpc_xprt *xprt, size_t sndsize, size_t
> Â */
> Â static void xs_udp_timer(struct rpc_xprt *xprt, struct rpc_task
> *task)
> Â {
> + spin_lock_bh(&xprt->transport_lock);
> Â xprt_adjust_cwnd(xprt, task, -ETIMEDOUT);
> + spin_unlock_bh(&xprt->transport_lock);
> Â }
> Â
> Â static unsigned short xs_get_random_port(void)
>
Thanks! Good cleanup...
Trond
--
Trond Myklebust
Principal System Architect
4300 El Camino Real | Suite 100
Los Altos, CAÂ Â 94022
W: 650-422-3800
C: 801-921-4583Â
www.primarydata.com
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±ÙšŠ{ayº\x1dʇڙë,j\a¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš
next prev parent reply other threads:[~2017-02-09 0:09 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-08 21:59 [PATCH v3 00/12] NFS/RDMA client-side patches for 4.11 Chuck Lever
2017-02-08 21:59 ` Chuck Lever
2017-02-08 21:59 ` [PATCH v3 01/12] xprtrdma: Fix Read chunk padding Chuck Lever
2017-02-08 21:59 ` Chuck Lever
2017-02-08 21:59 ` [PATCH v3 02/12] xprtrdma: Per-connection pad optimization Chuck Lever
2017-02-08 21:59 ` Chuck Lever
2017-02-08 22:00 ` [PATCH v3 03/12] xprtrdma: Disable pad optimization by default Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 22:00 ` [PATCH v3 04/12] xprtrdma: Reduce required number of send SGEs Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 22:00 ` [PATCH v3 05/12] xprtrdma: Shrink send SGEs array Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 22:00 ` [PATCH v3 06/12] xprtrdma: Properly recover FRWRs with in-flight FASTREG WRs Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 22:00 ` [PATCH v3 07/12] xprtrdma: Handle stale connection rejection Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 22:00 ` [PATCH v3 08/12] xprtrdma: Refactor management of mw_list field Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 22:00 ` [PATCH v3 09/12] sunrpc: Allow xprt->ops->timer method to sleep Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 23:48 ` Trond Myklebust [this message]
2017-02-08 23:48 ` Trond Myklebust
2017-02-08 22:00 ` [PATCH v3 10/12] sunrpc: Enable calls to rpc_call_null_helper() from other modules Chuck Lever
2017-02-08 22:00 ` Chuck Lever
2017-02-08 22:01 ` [PATCH v3 11/12] xprtrdma: Detect unreachable NFS/RDMA servers more reliably Chuck Lever
2017-02-08 22:01 ` Chuck Lever
2017-02-08 22:01 ` [PATCH v3 12/12] sunrpc: Allow keepalive ping on a credit-full transport Chuck Lever
2017-02-08 22:01 ` Chuck Lever
2017-02-09 0:05 ` Trond Myklebust
2017-02-09 0:05 ` Trond Myklebust
2017-02-09 0:19 ` Chuck Lever
2017-02-09 0:19 ` Chuck Lever
2017-02-09 0:48 ` Trond Myklebust
2017-02-09 0:48 ` Trond Myklebust
2017-02-09 15:37 ` Chuck Lever
2017-02-09 15:37 ` Chuck Lever
2017-02-09 19:42 ` Chuck Lever
2017-02-09 19:42 ` Chuck Lever
2017-02-09 20:13 ` Trond Myklebust
2017-02-09 20:13 ` Trond Myklebust
2017-02-09 20:39 ` Chuck Lever
2017-02-09 20: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=1486597679.11028.1.camel@primarydata.com \
--to=trondmy@primarydata.com \
--cc=anna.schumaker@netapp.com \
--cc=chuck.lever@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-rdma@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 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.