All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jim Rees <rees@umich.edu>
To: NeilBrown <neilb@suse.de>
Cc: "J.Bruce Fields" <bfields@citi.umich.edu>,
	Olga Kornievskaia <aglo@citi.umich.edu>,
	NFS <linux-nfs@vger.kernel.org>
Subject: Re: Is tcp autotuning really what NFS wants?
Date: Mon, 15 Jul 2013 07:57:11 -0400	[thread overview]
Message-ID: <20130715115711.GA9053@umich.edu> (raw)
In-Reply-To: <20130715150229.06ff8464@notabene.brown>

NeilBrown wrote:

  On Sun, 14 Jul 2013 21:26:20 -0400 Jim Rees <rees@umich.edu> wrote:
  
  > J.Bruce Fields wrote:
  > 
  >   On Wed, Jul 10, 2013 at 09:22:55AM +1000, NeilBrown wrote:
  >   > 
  >   > Hi,
  >   >  I just noticed this commit:
  >   > 
  >   > commit 9660439861aa8dbd5e2b8087f33e20760c2c9afc
  >   > Author: Olga Kornievskaia <aglo@citi.umich.edu>
  >   > Date:   Tue Oct 21 14:13:47 2008 -0400
  >   > 
  >   >     svcrpc: take advantage of tcp autotuning
  >   > 
  >   > 
  >   > which I must confess surprised me.  I wonder if the full implications of
  >   > removing that functionality were understood.
  >   > 
  >   > Previously nfsd would set the transmit buffer space for a connection to
  >   > ensure there is plenty to hold all replies.  Now it doesn't.
  >   > 
  >   > nfsd refuses to accept a request if there isn't enough space in the transmit
  >   > buffer to send a reply.  This is important to ensure that each reply gets
  >   > sent atomically without blocking and there is no risk of replies getting
  >   > interleaved.
  >   > 
  >   > The server starts out with a large estimate of the reply space (1M) and for
  >   > NFSv3 and v2 it quickly adjusts this down to something realistic.  For NFSv4
  >   > it is much harder to estimate the space needed so it just assumes every
  >   > reply will require 1M of space.
  >   > 
  >   > This means that with NFSv4, as soon as you have enough concurrent requests
  >   > such that 1M each reserves all of whatever window size was auto-tuned, new
  >   > requests on that connection will be ignored.
  >   >
  >   > This could significantly limit the amount of parallelism that can be achieved
  >   > for a single TCP connection (and given that the Linux client strongly prefers
  >   > a single connection now, this could become more of an issue).
  >   
  >   Worse, I believe it can deadlock completely if the transmit buffer
  >   shrinks too far, and people really have run into this:
  > 
  > It's been a few years since I looked at this, but are you sure autotuning
  > reduces the buffer space available on the sending socket? That doesn't sound
  > like correct behavior to me. I know we thought about this at the time.
  
  Autotuning is enabled when SOCK_SNDBUF_LOCK is not set in sk_userlocks.
  
  One of the main effects of this flag is to disable:
  
  static inline void sk_stream_moderate_sndbuf(struct sock *sk)
  {
  	if (!(sk->sk_userlocks & SOCK_SNDBUF_LOCK)) {
  		sk->sk_sndbuf = min(sk->sk_sndbuf, sk->sk_wmem_queued >> 1);
  		sk->sk_sndbuf = max(sk->sk_sndbuf, SOCK_MIN_SNDBUF);
  	}
  }
  
  
  which will reduce sk_sndbuf to half the queued writes. As sk_wmem_queued
  cannot grow above sk_sndbuf, the definitely reduces sk_sndbuf (though never
  below SOCK_MIN_SNDBUF which is 2K.
  
  This seems to happen under memory pressure (sk_stream_alloc_skb).
  
  So yes: memory pressure can reduce the sndbuf size when autotuning is enabled, and it
  can get as low as 2K.
  (An API to set this minimum to e.g. 2M for nfsd connections would be an alternate
  fix for the deadlock, as Bruce has already mentioned).
  
  > 
  > It does seem like a bug that we don't multiply the needed send buffer space
  > by the number of threads. I think that's because we don't know how many
  > threads there are going to be in svc_setup_socket()?
  
  We used to, but it turned out to be too small in practice!  As it auto-grows,
  the "4 * serv->sv_max_mesg" setting is big enough ... if only it wouldn't
  shrink below that.

This sounds familiar. In fact I think we asked on the network mailing list
about providing an api to set a minimum on the socket buffer. I'll go
through my old email and see if I can find it.

  reply	other threads:[~2013-07-15 11:57 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20130710092255.0240a36d@notabene.brown>
2013-07-10  2:27 ` Is tcp autotuning really what NFS wants? J.Bruce Fields
2013-07-10  4:32   ` NeilBrown
2013-07-10 19:07     ` J.Bruce Fields
2013-07-15  4:32       ` NeilBrown
2013-07-16  1:58         ` J.Bruce Fields
2013-07-16  4:00           ` NeilBrown
2013-07-16 14:24             ` J.Bruce Fields
2013-07-18  0:03               ` Ben Myers
2013-07-24 21:07                 ` J.Bruce Fields
2013-07-25  1:30                   ` [PATCH] NFSD/sunrpc: avoid deadlock on TCP connection due to memory pressure NeilBrown
2013-07-25 12:35                     ` Jim Rees
2013-07-25 20:18                     ` J.Bruce Fields
2013-07-25 20:33                       ` NeilBrown
2013-07-26 14:19                         ` J.Bruce Fields
2013-07-30  2:48                           ` NeilBrown
2013-08-01  2:49                             ` J.Bruce Fields
2013-07-10 17:33   ` Is tcp autotuning really what NFS wants? Dean
2013-07-10 17:39     ` Ben Greear
2013-07-15  4:35       ` NeilBrown
2013-07-15 23:32         ` Ben Greear
2013-07-16  4:46           ` NeilBrown
2013-07-10 19:59     ` Michael Richardson
2013-07-15  1:26   ` Jim Rees
2013-07-15  5:02     ` NeilBrown
2013-07-15 11:57       ` Jim Rees [this message]
2013-07-15 13:42   ` Jim Rees
2013-07-16  1:10     ` NeilBrown

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=20130715115711.GA9053@umich.edu \
    --to=rees@umich.edu \
    --cc=aglo@citi.umich.edu \
    --cc=bfields@citi.umich.edu \
    --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.