netdev - Re: [PATCH] virtio_net: indicate oom when addbuf returns failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201006071147.22102.rusty@rustcorp.com.au>
Date:	Mon, 7 Jun 2010 11:47:21 +0930
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	stable@...nel.org, Bruce Rogers <brogers@...ell.com>,
	Herbert Xu <herbert@...dor.hengli.com.au>,
	netdev@...r.kernel.org, virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH] virtio_net: indicate oom when addbuf returns failure

On Mon, 7 Jun 2010 05:43:00 am Michael S. Tsirkin wrote:
> On Fri, Jun 04, 2010 at 10:28:56AM +0930, Rusty Russell wrote:
> > This patch is a subset of an already upstream patch, but this portion
> > is useful in earlier releases.
> > 
> > Please consider for the 2.6.32 and 2.6.33 stable trees.
> > 
> > If the add_buf operation fails, indicate failure to the caller.
> > 
> > Signed-off-by: Bruce Rogers <brogers@...ell.com>
> > Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
> 
> Actually this code looks strange:
> Note that add_buf inicates out of memory
> condition with a positive return value, and ring full
> (which is not an error!) with -ENOSPC.
> 
> So it seems that this patch (and upstream code) will fill
> the ring and then end up setting oom = true and rescheduling the work
> forever.  And I suspect I actually saw this at some point
> on one of my systems: observed BW would drop
> with high CPU usage until reboot.
> Can't reproduce it now anymore ..

I thought that at first too, but it's subtler than that.

When the ring is exactly filled, err = 0.  With mergeable bufs and small
bufs that's the.  With big buffers, it's probably not, and the code will
indeed respond as if always out of memory, always trying to refill.

Our current code has the same error.  Probably a combination of noone using
big buffers, and noone noticing one timer every 1/2 second.

Want to fix that properly?  And comment it? :)

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html