| Message ID | 20190529221523.22399-2-bigeasy@linutronix.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Avoid local_irq_save() and use napi_alloc_frag() where possible | expand |
On 5/29/19 3:15 PM, Sebastian Andrzej Siewior wrote: > netdev_alloc_frag() can be used from any context and is used by NAPI > and non-NAPI drivers. Non-NAPI drivers use it in interrupt context > and NAPI drivers use it during initial allocation (->ndo_open() or > ->ndo_change_mtu()). Some NAPI drivers share the same function for the > initial allocation and the allocation in their NAPI callback. ... > + > + fragsz = SKB_DATA_ALIGN(fragsz); > + if (irqs_disabled()) { What is the difference between this prior test, and the following ? if (in_irq() || irqs_disabled()) I am asking because I see the latter being used in __dev_kfree_skb_any() > + nc = this_cpu_ptr(&netdev_alloc_cache); > + data = page_frag_alloc(nc, fragsz, GFP_ATOMIC); > + } else { > + local_bh_disable(); > + data = __napi_alloc_frag(fragsz, GFP_ATOMIC); > + local_bh_enable(); > + } > + return data; > +} > +EXPORT_SYMBOL(netdev_alloc_frag); > + > /** > * __netdev_alloc_skb - allocate an skbuff for rx on a specific device > * @dev: network device to receive on >
On 2019-05-29 15:48:51 [-0700], Eric Dumazet wrote: > > + > > + fragsz = SKB_DATA_ALIGN(fragsz); > > + if (irqs_disabled()) { > > > What is the difference between this prior test, and the following ? > > if (in_irq() || irqs_disabled()) > > I am asking because I see the latter being used in __dev_kfree_skb_any() in_irq() is always true in hardirq context which is true for non-NAPI drivers. If in_irq() is true, irqs_disabled() will also be true. So I *think* I could replace the irqs_disabled() check with in_irq() which should be cheaper because it just checks the preempt counter. Sebastian
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index e89be62826937..8a5ff67e14d90 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -369,34 +369,6 @@ struct napi_alloc_cache { static DEFINE_PER_CPU(struct page_frag_cache, netdev_alloc_cache); static DEFINE_PER_CPU(struct napi_alloc_cache, napi_alloc_cache); -static void *__netdev_alloc_frag(unsigned int fragsz, gfp_t gfp_mask) -{ - struct page_frag_cache *nc; - unsigned long flags; - void *data; - - local_irq_save(flags); - nc = this_cpu_ptr(&netdev_alloc_cache); - data = page_frag_alloc(nc, fragsz, gfp_mask); - local_irq_restore(flags); - return data; -} - -/** - * netdev_alloc_frag - allocate a page fragment - * @fragsz: fragment size - * - * Allocates a frag from a page for receive buffer. - * Uses GFP_ATOMIC allocations. - */ -void *netdev_alloc_frag(unsigned int fragsz) -{ - fragsz = SKB_DATA_ALIGN(fragsz); - - return __netdev_alloc_frag(fragsz, GFP_ATOMIC); -} -EXPORT_SYMBOL(netdev_alloc_frag); - static void *__napi_alloc_frag(unsigned int fragsz, gfp_t gfp_mask) { struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache); @@ -412,6 +384,31 @@ void *napi_alloc_frag(unsigned int fragsz) } EXPORT_SYMBOL(napi_alloc_frag); +/** + * netdev_alloc_frag - allocate a page fragment + * @fragsz: fragment size + * + * Allocates a frag from a page for receive buffer. + * Uses GFP_ATOMIC allocations. + */ +void *netdev_alloc_frag(unsigned int fragsz) +{ + struct page_frag_cache *nc; + void *data; + + fragsz = SKB_DATA_ALIGN(fragsz); + if (irqs_disabled()) { + nc = this_cpu_ptr(&netdev_alloc_cache); + data = page_frag_alloc(nc, fragsz, GFP_ATOMIC); + } else { + local_bh_disable(); + data = __napi_alloc_frag(fragsz, GFP_ATOMIC); + local_bh_enable(); + } + return data; +} +EXPORT_SYMBOL(netdev_alloc_frag); + /** * __netdev_alloc_skb - allocate an skbuff for rx on a specific device * @dev: network device to receive on
netdev_alloc_frag() can be used from any context and is used by NAPI and non-NAPI drivers. Non-NAPI drivers use it in interrupt context and NAPI drivers use it during initial allocation (->ndo_open() or ->ndo_change_mtu()). Some NAPI drivers share the same function for the initial allocation and the allocation in their NAPI callback. The interrupts are disabled in order to ensure locked access from every context to `netdev_alloc_cache'. Let netdev_alloc_frag() check if interrupts are disabled. If they are, use `netdev_alloc_cache' otherwise disable BH and invoke __napi_alloc_frag() for the allocation. The IRQ check is cheaper compared to disabling & enabling interrupts and memory allocation with disabled interrupts does not work on -RT. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- net/core/skbuff.c | 53 ++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 28 deletions(-)