lkml.org 
[lkml]   [2017]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net 0/3] Fix for BPF devmap percpu allocation splat
On 10/18/2017 04:03 PM, Daniel Borkmann wrote:
> On 10/18/2017 03:25 PM, Tejun Heo wrote:
>> Hello, Daniel.
>>
>> (cc'ing Dennis)
>>
>> On Tue, Oct 17, 2017 at 04:55:51PM +0200, Daniel Borkmann wrote:
>>> The set fixes a splat in devmap percpu allocation when we alloc
>>> the flush bitmap. Patch 1 is a prerequisite for the fix in patch 2,
>>> patch 1 is rather small, so if this could be routed via -net, for
>>> example, with Tejun's Ack that would be good. Patch 3 gets rid of
>>> remaining PCPU_MIN_UNIT_SIZE checks, which are percpu allocator
>>> internals and should not be used.
>>>
>>> Thanks!
>>>
>>> Daniel Borkmann (3):
>>> mm, percpu: add support for __GFP_NOWARN flag
>>
>> This looks fine.
>
> Great, thanks!
>
>>> bpf: fix splat for illegal devmap percpu allocation
>>> bpf: do not test for PCPU_MIN_UNIT_SIZE before percpu allocations
>>
>> These look okay too but if it helps percpu allocator can expose the
>> maximum size / alignment supported to take out the guessing game too.
>
> At least from BPF side there's right now no infra for exposing
> max possible alloc sizes for maps to e.g. user space as indication.
> There are few users left in the tree, where it would make sense for
> having some helpers though:
>
> arch/tile/kernel/setup.c:729: if (size < PCPU_MIN_UNIT_SIZE)
> arch/tile/kernel/setup.c:730: size = PCPU_MIN_UNIT_SIZE;
> drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:346: unsigned int max = (PCPU_MIN_UNIT_SIZE - sizeof(*pools)) << 3;
> drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:352: /* make sure per cpu pool fits into PCPU_MIN_UNIT_SIZE */
> drivers/scsi/libfc/fc_exch.c:2488: /* reduce range so per cpu pool fits into PCPU_MIN_UNIT_SIZE pool */
> drivers/scsi/libfc/fc_exch.c:2489: pool_exch_range = (PCPU_MIN_UNIT_SIZE - sizeof(*pool)) /
>
>> Also, the reason why PCPU_MIN_UNIT_SIZE is what it is is because
>> nobody needed anything bigger. Increasing the size doesn't really
>> cost much at least on 64bit archs. Is that something we want to be
>> considering?
>
> For devmap (and cpumap) itself it wouldn't make sense. For per-cpu
> hashtable we could indeed consider it in the future.

Higher prio imo would be to make the allocation itself faster
though, I remember we talked about this back in May wrt hashtable,
but I kind of lost track whether there was an update on this in
the mean time. ;-)

Cheers,
Daniel

\
 
 \ /
  Last update: 2017-10-22 17:14    [W:0.089 / U:0.244 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site