All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: dwmw2@infradead.org
Cc: netdev@vger.kernel.org, paulus@samba.org, chas@cmf.nrl.navy.mil,
	eric.dumazet@gmail.com
Subject: Re: [PATCH] pppoatm: Fix excessive queue bloat
Date: Fri, 13 Apr 2012 13:05:57 -0400 (EDT)	[thread overview]
Message-ID: <20120413.130557.2049216012537456628.davem@davemloft.net> (raw)
In-Reply-To: <1333914837.31812.57.camel@shinybook.infradead.org>

From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 08 Apr 2012 21:53:57 +0200

> We discovered that PPPoATM has an excessively deep transmit queue. A
> queue the size of the default socket send buffer (wmem_default) is
> maintained between the PPP generic core and the ATM device.
> 
> Fix it to queue a maximum of *two* packets. The one the ATM device is
> currently working on, and one more for the ATM driver to process
> immediately in its TX done interrupt handler. The PPP core is designed
> to feed packets to the channel with minimal latency, so that really
> ought to be enough to keep the ATM device busy.
> 
> While we're at it, fix the fact that we were triggering the wakeup
> tasklet on *every* pppoatm_pop() call. The comment saying "this is
> inefficient, but doing it right is too hard" turns out to be overly
> pessimistic... I think :)
> 
> On machines like the Traverse Geos, with a slow Geode CPU and two
> high-speed ADSL2+ interfaces, there were reports of extremely high CPU
> usage which could partly be attributed to the extra wakeups.
> 
> (The wakeup handling could actually be made a whole lot easier if we
>  stop checking sk->sk_sndbuf altogether. Given that we now only queue
>  *two* packets ever, one wonders what the point is. As it is, you could
>  already deadlock the thing by setting the sk_sndbuf to a value lower
>  than the MTU of the device, and it'd just block for ever.)
> 
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

Applied to net-next, thanks David.

I always considered the use of socket buffers to handle packets
moving around the ATM layers as a fundamental design error.

Although it's major surgery, fixing that up would be a much
appreciated contribution for someone suitably motivated :-)

  parent reply	other threads:[~2012-04-13 17:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-22 21:03 [STRAW MAN PATCH] sch_teql doesn't load-balance ppp(oatm) slaves David Woodhouse
2012-03-23  3:03 ` David Miller
2012-03-25 10:43   ` David Woodhouse
2012-03-25 21:36     ` David Miller
2012-03-26  8:32       ` David Woodhouse
2012-03-26  9:45         ` David Woodhouse
2012-03-26 10:03       ` [PATCH] ppp: Don't stop and restart queue on every TX packet David Woodhouse
2012-04-03 21:29         ` David Miller
2012-04-08 19:58           ` David Woodhouse
2012-04-08 20:01             ` ppp: Fix race condition with queue start/stop David Woodhouse
2012-04-13 17:07               ` David Miller
2012-03-27 19:10       ` [STRAW MAN PATCH] sch_teql doesn't load-balance ppp(oatm) slaves David Woodhouse
2012-03-27 19:55         ` Eric Dumazet
2012-03-27 20:35           ` David Woodhouse
2012-04-08 19:53             ` [PATCH] pppoatm: Fix excessive queue bloat David Woodhouse
2012-04-10 14:26               ` chas williams - CONTRACTOR
2012-04-10 20:28                 ` David Woodhouse
2012-04-13 17:04                 ` David Miller
2012-04-13 17:27                   ` David Miller
2012-04-13 17:05               ` David Miller [this message]
2012-04-08 19:55             ` David Woodhouse

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=20120413.130557.2049216012537456628.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=chas@cmf.nrl.navy.mil \
    --cc=dwmw2@infradead.org \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=paulus@samba.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.