On 12/09/2014 08:16 PM, Alexei Starovoitov wrote:

On Tue, Dec 9, 2014 at 7:40 PM, Alexander Duyck
[off-list ref] wrote:

quoted

This patch splits the netdev_alloc_frag function up so that it can be used
on one of two page frag pools instead of being fixed on the
netdev_alloc_cache.  By doing this we can add a NAPI specific function
__napi_alloc_frag that accesses a pool that is only used from softirq
context.  The advantage to this is that we do not need to call
local_irq_save/restore which can be a significant savings.

I also took the opportunity to refactor the core bits that were placed in
__alloc_page_frag.  First I updated the allocation to do either a 32K
allocation or an order 0 page.  This is based on the changes in commmit
d9b2938aa where it was found that latencies could be reduced in case of

thanks for explaining that piece of it.

quoted

+       struct page *page = NULL;
+       gfp_t gfp = gfp_mask;
+
+       if (order) {
+               gfp_mask |= __GFP_COMP | __GFP_NOWARN | __GFP_NORETRY;
+               page = alloc_pages_node(NUMA_NO_NODE, gfp_mask, order);
+               nc->frag.size = PAGE_SIZE << (page ? order : 0);
+       }

-       local_irq_save(flags);
-       nc = this_cpu_ptr(&netdev_alloc_cache);
-       if (unlikely(!nc->frag.page)) {
+       if (unlikely(!page))
+               page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);

I'm guessing you're not combining this 'if' with above one to
keep gfp untouched, so there is a 'warn' when it actually fails 2nd time.
Tricky :)
Anyway looks good to me and I think I understand it enough to say:
Acked-by: Alexei Starovoitov <redacted>

Thanks.  Yes the compiler is smart enough to combine the frag.size and
the second check into one if order is non-zero.  The other trick here is
if order is 0 then that whole block disappears and I don't have to touch
frag.size or gfp at all and the code gets much simpler as the *page =
NULL falls though and cancels out the 'if' as a compile time check.

- Alex