From patchwork Mon Jul 6 01:26:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 6719051 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id EA8559F38C for ; Mon, 6 Jul 2015 01:27:08 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0A46820643 for ; Mon, 6 Jul 2015 01:27:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 075472063F for ; Mon, 6 Jul 2015 01:27:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753225AbbGFB06 (ORCPT ); Sun, 5 Jul 2015 21:26:58 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39213 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753124AbbGFB05 (ORCPT ); Sun, 5 Jul 2015 21:26:57 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id B8E3CAC5F; Mon, 6 Jul 2015 01:26:55 +0000 (UTC) Date: Mon, 6 Jul 2015 11:26:47 +1000 From: NeilBrown To: Trond Myklebust Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH] SUNRPC: handle -EAGAIN from socket writes better. Message-ID: <20150706112647.2d4c3780@noble> In-Reply-To: <1435931379-6654-1-git-send-email-trond.myklebust@primarydata.com> References: <20150629142623.5afc0e6d@noble> <1435931379-6654-1-git-send-email-trond.myklebust@primarydata.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.28; x86_64-suse-linux-gnu) MIME-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 3 Jul 2015 09:49:37 -0400 Trond Myklebust wrote: > Hi Neil, > > There should already be a handler for ENOBUFS in call_status, but > I can see that it has a couple of flaws. What say we try to fix that > instead? > > Cheers > Trond > Hi Trond, your patches make sense I think, but they are only part of a solution. In the problem case the error comes from sk_stream_wait_memory, and that returns EAGAIN, never ENOBUFS. So fixing the handling of ENOBUFS won't be enough. The call path is xs_tcp_send_request -> xs_sendpages -> xs_send_pagedata -> (sock->ops->sendpage == tcp_sendpage) -> do_tcp_sendpage -> sk_stream_wait_memory and EAGAIN travels all the way from bottom to top unmolested. We could possibly change sk_stream_wait_memory to return ENOBUFS if vm_wait is != 0, but: - that could have other consequences so needs to go through netdev and probably isn't a quick fix - there could be other paths that don't return ENOBUFS - it really don't seem that ENOBUFS appears all that much in 'net/' in places where it would need to... Maybe we could check and translate the error in xs_sendpages: Though that is a bit of a hack. If/when net-dev gets the correct error returns, we can then remove that. Though I'm beginning to wonder if ENOBUFS is the correct error code anyway. "man 2 send" suggests ENOMEM, with ENOBUFS meaning: The output queue for a network interface was full. This generally indicates that the interface has stopped send- ing, but may be caused by transient congestion. (Nor- mally, this does not occur in Linux. Packets are just silently dropped when a device queue overflows.) So I'm not sure I feel to comfortable about relying on the exact error code. What do you think? Thanks, NeilBrown --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index 66891e32c5e3..8474d79ec2b2 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -431,6 +431,14 @@ out: if (err > 0) { *sent_p += err; err = 0; + } else if (err == -EAGAIN) { + /* Might be wrong error code. */ + if (sock->sk->sk_write_space == xs_tcp_write_space && + sk_stream_is_writeable(sock->sk)) + err = -ENOBUFS; + if (sock->sk->sk_write_space == xs_udp_write_space && + sock_writeable(sock->sk)) + err = -ENOBUFS; } return err; }