Messages in this thread | | | Date | Sun, 20 Feb 2022 09:48:52 +0100 | Subject | Re: [PATCH] ravb: Use GFP_KERNEL instead of GFP_ATOMIC when possible | From | Christophe JAILLET <> |
| |
Le 20/02/2022 à 08:53, Biju Das a écrit : > Hi Christophe, > > Thanks for the patch. > > Just a question, As per [1], former can be allocated from interrupt context. > But nothing mentioned for the allocation using the patch you mentioned[2]. I agree GFP_KERNEL > gives more opportunities of successful allocation.
Hi,
netdev_alloc_skb() uses an implicit GFP_ATOMIC, that is why it can be safely called from an interrupt context. __netdev_alloc_skb() is the same as netdev_alloc_skb(), except that you can choose the GFP flag you want to use. ([1])
Here, the netdev_alloc_skb() is called just after some "kcalloc(GFP_KERNEL);"
So this function can already NOT be called from interrupt context.
So if GFP_KERNEL is fine here for kcalloc(), it is fine also for netdev_alloc_skb(), hence __netdev_alloc_skb(GFP_KERNEL).
> > Q1) Here it allocates 8K instead of 1K on each loop, Is there any limitation for netdev_alloc_skb for allocating 8K size?
Not sure to understand. My patch does NOT change anything on the amount of memory allocated. it only changes a GFP_ATOMIC into a GFP_KERNEL.
I'm not aware of specific limitation for netdev_alloc_skb(). My understanding is that in the worst case, it will behave just like malloc() ([3])
So, if it was an issue before, it is still an issue after my patch.
> Q2) In terms of allocation performance which is better netdev_alloc_skb or __netdev_alloc_skb?
AFAIK, there should be no difference, but __netdev_alloc_skb(GFP_KERNEL) can succeed where netdev_alloc_skb() can fail. In such a case, it would be slower but most importantly, it would succeed.
CJ
[1]: https://elixir.bootlin.com/linux/v5.17-rc4/source/include/linux/skbuff.h#L2945
[2]: https://elixir.bootlin.com/linux/v5.17-rc4/source/drivers/net/ethernet/renesas/ravb_main.c#L470
[3]: https://elixir.bootlin.com/linux/v5.17-rc3/source/net/core/skbuff.c#L488
> > [1] https://www.kernel.org/doc/htmldocs/networking/API-netdev-alloc-skb.html > [2] https://www.kernel.org/doc/htmldocs/networking/API---netdev-alloc-skb.html > > Regards, > Biju > >> Subject: [PATCH] ravb: Use GFP_KERNEL instead of GFP_ATOMIC when possible >> >> 'max_rx_len' can be up to GBETH_RX_BUFF_MAX (i.e. 8192) (see >> 'gbeth_hw_info'). >> The default value of 'num_rx_ring' can be BE_RX_RING_SIZE (i.e. 1024). >> >> So this loop can allocate 8 Mo of memory. >> >> Previous memory allocations in this function already use GFP_KERNEL, so >> use __netdev_alloc_skb() and an explicit GFP_KERNEL instead of a implicit >> GFP_ATOMIC. >> >> This gives more opportunities of successful allocation. >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> drivers/net/ethernet/renesas/ravb_main.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >> b/drivers/net/ethernet/renesas/ravb_main.c >> index 24e2635c4c80..525d66f71f02 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c >> @@ -475,7 +475,7 @@ static int ravb_ring_init(struct net_device *ndev, int >> q) >> goto error; >> >> for (i = 0; i < priv->num_rx_ring[q]; i++) { >> - skb = netdev_alloc_skb(ndev, info->max_rx_len); >> + skb = __netdev_alloc_skb(ndev, info->max_rx_len, GFP_KERNEL); >> if (!skb) >> goto error; >> ravb_set_buffer_align(skb); >> -- >> 2.32.0 > >
| |