Re: Question about ip_defrag

[Florian Westphal <fw@strlen.de>]

Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> > I take 2) back.  Its wrong to do this, for large NR_CPU values it
> > would even overflow.
> 
> Alternatively solution 3:
> Why do we want to maintain a (4MBytes) memory limit, across all CPUs?
> Couldn't we just allow each CPU to have a memory limit?

Consider ipv4, ipv6, nf ipv6 defrag, 6lowpan, and 8k cpus... This will
render any limit useless.

> > > To me it looks like we/I have been using the wrong API for comparing
> > > against percpu_counters.  I guess we should have used __percpu_counter_compare().  
> > 
> > Are you sure?  For liujian use case (64 cores) it looks like we would
> > always fall through to percpu_counter_sum() so we eat spinlock_irqsave
> > cost for all compares.
> > 
> > Before we entertain this we should consider reducing frag_percpu_counter_batch
> > to a smaller value.
> 
> Yes, I agree, we really need to lower/reduce the frag_percpu_counter_batch.
> As you say, else the __percpu_counter_compare() call will be useless
> (around systems with >= 32 CPUs).
> 
> I think the bug is in frag_mem_limit().  It just reads the global
> counter (fbc->count), without considering other CPUs can have upto 130K
> that haven't been subtracted yet (due to 3M low limit, become dangerous
> at >=24 CPUs).  The  __percpu_counter_compare() does the right thing,
> and takes into account the number of (online) CPUs and batch size, to
> account for this.

Right, I think we should at very least use __percpu_counter_compare
before denying a new frag queue allocation request.

I'll create a patch.