* [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init
@ 2019-04-18 18:27 Ilias Apalodimas
2019-04-18 19:50 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Ilias Apalodimas @ 2019-04-18 18:27 UTC (permalink / raw)
To: netdev, jaswinder.singh, davem
Cc: ard.biesheuvel, masahisa.kojima, Ilias Apalodimas
Use netdev_alloc_frag during the Rx ring setup instead napi_alloc_frag
Fixes: 4acb20b46214 ("net: socionext: different approach on DMA")
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
drivers/net/ethernet/socionext/netsec.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index a18149720aa2..cba5881b2746 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -673,7 +673,8 @@ static void netsec_process_tx(struct netsec_priv *priv)
}
static void *netsec_alloc_rx_data(struct netsec_priv *priv,
- dma_addr_t *dma_handle, u16 *desc_len)
+ dma_addr_t *dma_handle, u16 *desc_len,
+ bool napi)
{
size_t total_len = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
size_t payload_len = NETSEC_RX_BUF_SZ;
@@ -682,7 +683,7 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
total_len += SKB_DATA_ALIGN(payload_len + NETSEC_SKB_PAD);
- buf = napi_alloc_frag(total_len);
+ buf = napi ? napi_alloc_frag(total_len) : netdev_alloc_frag(total_len);
if (!buf)
return NULL;
@@ -765,7 +766,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
/* allocate a fresh buffer and map it to the hardware.
* This will eventually replace the old buffer in the hardware
*/
- buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len);
+ buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len,
+ true);
if (unlikely(!buf_addr))
break;
@@ -1069,7 +1071,8 @@ static int netsec_setup_rx_dring(struct netsec_priv *priv)
void *buf;
u16 len;
- buf = netsec_alloc_rx_data(priv, &dma_handle, &len);
+ buf = netsec_alloc_rx_data(priv, &dma_handle, &len,
+ false);
if (!buf) {
netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
goto err_out;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init
2019-04-18 18:27 [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init Ilias Apalodimas
@ 2019-04-18 19:50 ` Ard Biesheuvel
2019-04-18 20:24 ` Ilias Apalodimas
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2019-04-18 19:50 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: <netdev@vger.kernel.org>,
Jaswinder Singh, David S. Miller, Masahisa Kojima
On Thu, 18 Apr 2019 at 11:27, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Use netdev_alloc_frag during the Rx ring setup instead napi_alloc_frag
>
Why?
> Fixes: 4acb20b46214 ("net: socionext: different approach on DMA")
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> drivers/net/ethernet/socionext/netsec.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index a18149720aa2..cba5881b2746 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -673,7 +673,8 @@ static void netsec_process_tx(struct netsec_priv *priv)
> }
>
> static void *netsec_alloc_rx_data(struct netsec_priv *priv,
> - dma_addr_t *dma_handle, u16 *desc_len)
> + dma_addr_t *dma_handle, u16 *desc_len,
> + bool napi)
> {
> size_t total_len = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> size_t payload_len = NETSEC_RX_BUF_SZ;
> @@ -682,7 +683,7 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
>
> total_len += SKB_DATA_ALIGN(payload_len + NETSEC_SKB_PAD);
>
> - buf = napi_alloc_frag(total_len);
> + buf = napi ? napi_alloc_frag(total_len) : netdev_alloc_frag(total_len);
> if (!buf)
> return NULL;
>
> @@ -765,7 +766,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
> /* allocate a fresh buffer and map it to the hardware.
> * This will eventually replace the old buffer in the hardware
> */
> - buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len);
> + buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len,
> + true);
> if (unlikely(!buf_addr))
> break;
>
> @@ -1069,7 +1071,8 @@ static int netsec_setup_rx_dring(struct netsec_priv *priv)
> void *buf;
> u16 len;
>
> - buf = netsec_alloc_rx_data(priv, &dma_handle, &len);
> + buf = netsec_alloc_rx_data(priv, &dma_handle, &len,
> + false);
> if (!buf) {
> netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
> goto err_out;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init
2019-04-18 19:50 ` Ard Biesheuvel
@ 2019-04-18 20:24 ` Ilias Apalodimas
2019-04-19 6:46 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Ilias Apalodimas @ 2019-04-18 20:24 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: <netdev@vger.kernel.org>,
Jaswinder Singh, David S. Miller, Masahisa Kojima
Hi Ard,
> > Use netdev_alloc_frag during the Rx ring setup instead napi_alloc_frag
> >
>
> Why?
>
The netdev variant is usable on any context since it disables interrupts.
The napi variant of the call is supposed to be used under softirq context
only
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> > drivers/net/ethernet/socionext/netsec.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> > index a18149720aa2..cba5881b2746 100644
> > --- a/drivers/net/ethernet/socionext/netsec.c
> > +++ b/drivers/net/ethernet/socionext/netsec.c
> > @@ -673,7 +673,8 @@ static void netsec_process_tx(struct netsec_priv *priv)
> > }
> >
> > static void *netsec_alloc_rx_data(struct netsec_priv *priv,
> > - dma_addr_t *dma_handle, u16 *desc_len)
> > + dma_addr_t *dma_handle, u16 *desc_len,
> > + bool napi)
> > {
> > size_t total_len = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> > size_t payload_len = NETSEC_RX_BUF_SZ;
> > @@ -682,7 +683,7 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
> >
> > total_len += SKB_DATA_ALIGN(payload_len + NETSEC_SKB_PAD);
> >
> > - buf = napi_alloc_frag(total_len);
> > + buf = napi ? napi_alloc_frag(total_len) : netdev_alloc_frag(total_len);
> > if (!buf)
> > return NULL;
> >
> > @@ -765,7 +766,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
> > /* allocate a fresh buffer and map it to the hardware.
> > * This will eventually replace the old buffer in the hardware
> > */
> > - buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len);
> > + buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len,
> > + true);
> > if (unlikely(!buf_addr))
> > break;
> >
> > @@ -1069,7 +1071,8 @@ static int netsec_setup_rx_dring(struct netsec_priv *priv)
> > void *buf;
> > u16 len;
> >
> > - buf = netsec_alloc_rx_data(priv, &dma_handle, &len);
> > + buf = netsec_alloc_rx_data(priv, &dma_handle, &len,
> > + false);
> > if (!buf) {
> > netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
> > goto err_out;
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init
2019-04-18 20:24 ` Ilias Apalodimas
@ 2019-04-19 6:46 ` Ard Biesheuvel
2019-04-19 23:48 ` Jassi Brar
2019-04-23 4:40 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2019-04-19 6:46 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: <netdev@vger.kernel.org>,
Jaswinder Singh, David S. Miller, Masahisa Kojima
On Thu, 18 Apr 2019 at 22:24, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Ard,
> > > Use netdev_alloc_frag during the Rx ring setup instead napi_alloc_frag
> > >
> >
> > Why?
> >
> The netdev variant is usable on any context since it disables interrupts.
> The napi variant of the call is supposed to be used under softirq context
> only
>
Helpful, thanks. In general, commit logs need to describe *why* you do
something, since *what* you do is usually obvious from the patch
itself, but why you do it isn't.
With the above clarification added:
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > ---
> > > drivers/net/ethernet/socionext/netsec.c | 11 +++++++----
> > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> > > index a18149720aa2..cba5881b2746 100644
> > > --- a/drivers/net/ethernet/socionext/netsec.c
> > > +++ b/drivers/net/ethernet/socionext/netsec.c
> > > @@ -673,7 +673,8 @@ static void netsec_process_tx(struct netsec_priv *priv)
> > > }
> > >
> > > static void *netsec_alloc_rx_data(struct netsec_priv *priv,
> > > - dma_addr_t *dma_handle, u16 *desc_len)
> > > + dma_addr_t *dma_handle, u16 *desc_len,
> > > + bool napi)
> > > {
> > > size_t total_len = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> > > size_t payload_len = NETSEC_RX_BUF_SZ;
> > > @@ -682,7 +683,7 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
> > >
> > > total_len += SKB_DATA_ALIGN(payload_len + NETSEC_SKB_PAD);
> > >
> > > - buf = napi_alloc_frag(total_len);
> > > + buf = napi ? napi_alloc_frag(total_len) : netdev_alloc_frag(total_len);
> > > if (!buf)
> > > return NULL;
> > >
> > > @@ -765,7 +766,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
> > > /* allocate a fresh buffer and map it to the hardware.
> > > * This will eventually replace the old buffer in the hardware
> > > */
> > > - buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len);
> > > + buf_addr = netsec_alloc_rx_data(priv, &dma_handle, &desc_len,
> > > + true);
> > > if (unlikely(!buf_addr))
> > > break;
> > >
> > > @@ -1069,7 +1071,8 @@ static int netsec_setup_rx_dring(struct netsec_priv *priv)
> > > void *buf;
> > > u16 len;
> > >
> > > - buf = netsec_alloc_rx_data(priv, &dma_handle, &len);
> > > + buf = netsec_alloc_rx_data(priv, &dma_handle, &len,
> > > + false);
> > > if (!buf) {
> > > netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
> > > goto err_out;
> > > --
> > > 2.7.4
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init
2019-04-19 6:46 ` Ard Biesheuvel
@ 2019-04-19 23:48 ` Jassi Brar
2019-04-23 4:40 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Jassi Brar @ 2019-04-19 23:48 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Ilias Apalodimas, <netdev@vger.kernel.org>,
David S. Miller, Masahisa Kojima
On Fri, 19 Apr 2019 at 01:46, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Thu, 18 Apr 2019 at 22:24, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Ard,
> > > > Use netdev_alloc_frag during the Rx ring setup instead napi_alloc_frag
> > > >
> > >
> > > Why?
> > >
> > The netdev variant is usable on any context since it disables interrupts.
> > The napi variant of the call is supposed to be used under softirq context
> > only
> >
>
> Helpful, thanks. In general, commit logs need to describe *why* you do
> something, since *what* you do is usually obvious from the patch
> itself, but why you do it isn't.
>
> With the above clarification added:
>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
Fine by me too.
Acked-by: Jassi Brar <jaswinder.singh@linaro.org>
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init
2019-04-19 6:46 ` Ard Biesheuvel
2019-04-19 23:48 ` Jassi Brar
@ 2019-04-23 4:40 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2019-04-23 4:40 UTC (permalink / raw)
To: ard.biesheuvel; +Cc: ilias.apalodimas, netdev, jaswinder.singh, masahisa.kojima
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 19 Apr 2019 08:46:40 +0200
> On Thu, 18 Apr 2019 at 22:24, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
>>
>> Hi Ard,
>> > > Use netdev_alloc_frag during the Rx ring setup instead napi_alloc_frag
>> > >
>> >
>> > Why?
>> >
>> The netdev variant is usable on any context since it disables interrupts.
>> The napi variant of the call is supposed to be used under softirq context
>> only
>>
>
> Helpful, thanks. In general, commit logs need to describe *why* you do
> something, since *what* you do is usually obvious from the patch
> itself, but why you do it isn't.
>
> With the above clarification added:
>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Just being clear, this patch needs to be reposted with the commit log extended
as requested.
Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-23 4:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-18 18:27 [PATCH] net: socionext: replace napi_alloc_frag with the netdev variant on init Ilias Apalodimas
2019-04-18 19:50 ` Ard Biesheuvel
2019-04-18 20:24 ` Ilias Apalodimas
2019-04-19 6:46 ` Ard Biesheuvel
2019-04-19 23:48 ` Jassi Brar
2019-04-23 4:40 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).