netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>, Jason Wang <jasowang@redhat.com>
Subject: Re: [PATCH net] ptr_ring: use kmalloc_array()
Date: Fri, 25 Aug 2017 22:25:05 +0300	[thread overview]
Message-ID: <20170825222015-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <1503687439.11498.5.camel@edumazet-glaptop3.roam.corp.google.com>

On Fri, Aug 25, 2017 at 11:57:19AM -0700, Eric Dumazet wrote:
> On Fri, 2017-08-25 at 21:03 +0300, Michael S. Tsirkin wrote:
> > On Wed, Aug 16, 2017 at 10:36:47AM -0700, Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > > 
> > > As found by syzkaller, malicious users can set whatever tx_queue_len
> > > on a tun device and eventually crash the kernel.
> > > 
> > > Lets remove the ALIGN(XXX, SMP_CACHE_BYTES) thing since a small
> > > ring buffer is not fast anyway.
> > 
> > I'm not sure it's worth changing for small rings.
> > 
> > Does kmalloc_array guarantee cache line alignment for big buffers
> > then? If the ring is misaligned it will likely cause false sharing
> > as it's designed to be accessed from two CPUs.
> 
> I specifically said that in the changelog :
> 
> "since a small ring buffer is not fast anyway."
> 
> If one user sets up a pathological small ring buffer, kernel should not
> try to fix it.

Yes, I got that point. My question is about big buffers.
Does kmalloc_array give us an aligned array in that case?

E.g. imagine a 100 slot array. Will 800 bytes be allocated?
In that case it uses up 12.5 cache lines. It looks like the
last cache line can become false shared with something else,
causing cache line bounces on each wrap around.


> In this case, you would have to setup a ring of 2 or 4 slots to
> eventually hit false sharing.
> 

I don't think many people set up such tiny rings so I do not really
think we care what happens in that case. But you need 8 slots to avoid
false sharing I think.

-- 
MST

  reply	other threads:[~2017-08-25 19:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-16 17:36 [PATCH net] ptr_ring: use kmalloc_array() Eric Dumazet
2017-08-16 23:29 ` David Miller
2017-08-25 18:03 ` Michael S. Tsirkin
2017-08-25 18:57   ` Eric Dumazet
2017-08-25 19:25     ` Michael S. Tsirkin [this message]
2017-08-25 20:13       ` Eric Dumazet

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=20170825222015-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=netdev@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).