* [PATCH net] ps3/gelic: Fix SKB allocation
@ 2024-01-26 9:25 Geoff Levand
2024-01-30 12:37 ` Paolo Abeni
0 siblings, 1 reply; 3+ messages in thread
From: Geoff Levand @ 2024-01-26 9:25 UTC (permalink / raw)
To: sambat goson, Christophe Leroy, David S. Miller, netdev
Commit 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures")
of 6.8-rc1 did not set up the ps3 gelic network SKB's correctly,
resulting in a kernel panic.
This fix changes the way the napi buffer and corresponding SKB are
allocated and managed.
Reported-by: sambat goson <sombat3960@gmail.com>
Fixes: 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures")
Signed-off-by: Geoff Levand <geoff@infradead.org>
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index d5b75af163d3..1870f173e850 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -375,20 +375,16 @@ static int gelic_card_init_chain(struct gelic_card *card,
static int gelic_descr_prepare_rx(struct gelic_card *card,
struct gelic_descr *descr)
{
- static const unsigned int rx_skb_size =
- ALIGN(GELIC_NET_MAX_FRAME, GELIC_NET_RXBUF_ALIGN) +
- GELIC_NET_RXBUF_ALIGN - 1;
+ static const unsigned int napi_buff_size =
+ round_up(GELIC_NET_MAX_FRAME, GELIC_NET_RXBUF_ALIGN);
+
+ struct device *dev = ctodev(card);
dma_addr_t cpu_addr;
- int offset;
+ void *napi_buff;
if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE)
- dev_info(ctodev(card), "%s: ERROR status\n", __func__);
+ dev_info(dev, "%s: ERROR status\n", __func__);
- descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size);
- if (!descr->skb) {
- descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
- return -ENOMEM;
- }
descr->hw_regs.dmac_cmd_status = 0;
descr->hw_regs.result_size = 0;
descr->hw_regs.valid_size = 0;
@@ -397,24 +393,33 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
descr->hw_regs.payload.size = 0;
descr->skb = NULL;
- offset = ((unsigned long)descr->skb->data) &
- (GELIC_NET_RXBUF_ALIGN - 1);
- if (offset)
- skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset);
- /* io-mmu-map the skb */
- cpu_addr = dma_map_single(ctodev(card), descr->skb->data,
- GELIC_NET_MAX_FRAME, DMA_FROM_DEVICE);
- descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
- if (dma_mapping_error(ctodev(card), cpu_addr)) {
- dev_kfree_skb_any(descr->skb);
+ napi_buff = napi_alloc_frag_align(napi_buff_size,
+ GELIC_NET_RXBUF_ALIGN);
+
+ if (unlikely(!napi_buff)) {
+ return -ENOMEM;
+ }
+
+ descr->skb = napi_build_skb(napi_buff, napi_buff_size);
+
+ if (unlikely(!descr->skb)) {
+ skb_free_frag(napi_buff);
+ return -ENOMEM;
+ }
+
+ cpu_addr = dma_map_single(dev, napi_buff, napi_buff_size,
+ DMA_FROM_DEVICE);
+
+ if (dma_mapping_error(dev, cpu_addr)) {
+ skb_free_frag(napi_buff);
descr->skb = NULL;
- dev_info(ctodev(card),
- "%s:Could not iommu-map rx buffer\n", __func__);
+ dev_err_once(dev, "%s:Could not iommu-map rx buffer\n",
+ __func__);
gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
return -ENOMEM;
}
- descr->hw_regs.payload.size = cpu_to_be32(GELIC_NET_MAX_FRAME);
+ descr->hw_regs.payload.size = cpu_to_be32(napi_buff_size);
descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] ps3/gelic: Fix SKB allocation
2024-01-26 9:25 [PATCH net] ps3/gelic: Fix SKB allocation Geoff Levand
@ 2024-01-30 12:37 ` Paolo Abeni
2024-02-05 0:08 ` Geoff Levand
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2024-01-30 12:37 UTC (permalink / raw)
To: Geoff Levand, sambat goson, Christophe Leroy, David S. Miller, netdev
Hi,
On Fri, 2024-01-26 at 18:25 +0900, Geoff Levand wrote:
> Commit 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures")
> of 6.8-rc1 did not set up the ps3 gelic network SKB's correctly,
> resulting in a kernel panic.
>
> This fix changes the way the napi buffer and corresponding SKB are
> allocated and managed.
>
> Reported-by: sambat goson <sombat3960@gmail.com>
> Fixes: 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures")
> Signed-off-by: Geoff Levand <geoff@infradead.org>
The patch overall looks correct to me, but there are a few formal
issues worthy to be addressed, see below.
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index d5b75af163d3..1870f173e850 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -375,20 +375,16 @@ static int gelic_card_init_chain(struct gelic_card *card,
> static int gelic_descr_prepare_rx(struct gelic_card *card,
> struct gelic_descr *descr)
> {
> - static const unsigned int rx_skb_size =
> - ALIGN(GELIC_NET_MAX_FRAME, GELIC_NET_RXBUF_ALIGN) +
> - GELIC_NET_RXBUF_ALIGN - 1;
> + static const unsigned int napi_buff_size =
> + round_up(GELIC_NET_MAX_FRAME, GELIC_NET_RXBUF_ALIGN);
> +
No empty line in the declaration area.
> + struct device *dev = ctodev(card);
> dma_addr_t cpu_addr;
> - int offset;
> + void *napi_buff;
>
> if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE)
> - dev_info(ctodev(card), "%s: ERROR status\n", __func__);
> + dev_info(dev, "%s: ERROR status\n", __func__);
>
> - descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size);
> - if (!descr->skb) {
> - descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
> - return -ENOMEM;
> - }
> descr->hw_regs.dmac_cmd_status = 0;
> descr->hw_regs.result_size = 0;
> descr->hw_regs.valid_size = 0;
> @@ -397,24 +393,33 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
> descr->hw_regs.payload.size = 0;
> descr->skb = NULL;
>
> - offset = ((unsigned long)descr->skb->data) &
> - (GELIC_NET_RXBUF_ALIGN - 1);
> - if (offset)
> - skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset);
> - /* io-mmu-map the skb */
> - cpu_addr = dma_map_single(ctodev(card), descr->skb->data,
> - GELIC_NET_MAX_FRAME, DMA_FROM_DEVICE);
> - descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
> - if (dma_mapping_error(ctodev(card), cpu_addr)) {
> - dev_kfree_skb_any(descr->skb);
> + napi_buff = napi_alloc_frag_align(napi_buff_size,
> + GELIC_NET_RXBUF_ALIGN);
Please align with the above bracket:
napi_buff = napi_alloc_frag_align(napi_buff_size,
GELIC_NET_RXBUF_ALIGN);
> +
> + if (unlikely(!napi_buff)) {
> + return -ENOMEM;
> + }
The brackets are not needed here.
> +
> + descr->skb = napi_build_skb(napi_buff, napi_buff_size);
> +
> + if (unlikely(!descr->skb)) {
> + skb_free_frag(napi_buff);
> + return -ENOMEM;
> + }
> +
> + cpu_addr = dma_map_single(dev, napi_buff, napi_buff_size,
> + DMA_FROM_DEVICE);
Please align with the above bracket
> +
> + if (dma_mapping_error(dev, cpu_addr)) {
> + skb_free_frag(napi_buff);
> descr->skb = NULL;
> - dev_info(ctodev(card),
> - "%s:Could not iommu-map rx buffer\n", __func__);
> + dev_err_once(dev, "%s:Could not iommu-map rx buffer\n",
> + __func__);
Same here.
Side note: as a nice net-next follow-up you could move the skb
allocation into gelic_net_pass_skb_up() - so that the skbuff is hot in
the cache at rx time.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] ps3/gelic: Fix SKB allocation
2024-01-30 12:37 ` Paolo Abeni
@ 2024-02-05 0:08 ` Geoff Levand
0 siblings, 0 replies; 3+ messages in thread
From: Geoff Levand @ 2024-02-05 0:08 UTC (permalink / raw)
To: Paolo Abeni, sambat goson, Christophe Leroy, David S. Miller, netdev
Hi Paolo,
On 1/30/24 21:37, Paolo Abeni wrote:
> Hi,
>
> On Fri, 2024-01-26 at 18:25 +0900, Geoff Levand wrote:
>> Commit 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures")
>> of 6.8-rc1 did not set up the ps3 gelic network SKB's correctly,
>> resulting in a kernel panic.
>>
>> This fix changes the way the napi buffer and corresponding SKB are
>> allocated and managed.
>>
>> Reported-by: sambat goson <sombat3960@gmail.com>
>> Fixes: 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures")
>> Signed-off-by: Geoff Levand <geoff@infradead.org>
>
> The patch overall looks correct to me, but there are a few formal
> issues worthy to be addressed, see below.
Thanks for the review. I'll send out an updated patch sometime soon.
-Geoff
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-05 0:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-26 9:25 [PATCH net] ps3/gelic: Fix SKB allocation Geoff Levand
2024-01-30 12:37 ` Paolo Abeni
2024-02-05 0:08 ` Geoff Levand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.