[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DDA4Y2GRUHD4.1DFHX01NOJYCB@gmail.com>
Date: Sun, 05 Oct 2025 14:22:43 +0900
From: "Yeounsu Moon" <yyyynoom@...il.com>
To: "Simon Horman" <horms@...nel.org>
Cc: "Andrew Lunn" <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, "Eric Dumazet" <edumazet@...gle.com>, "Jakub
Kicinski" <kuba@...nel.org>, "Paolo Abeni" <pabeni@...hat.com>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] net: dlink: handle dma_map_single() failure
properly
Hello Simon.
I'm currenly re-writing the code as you suggested. I think `alloc_list()`
can easily adopt the `goto` pattern, but for others functions, it's not
that straightforward.
My question is whether a style combining `goto`, `continue`, and `break`
would be acceptable in this context:
```c
if (np->cur_rx - np->old_rx >= RX_RING_SIZE) {
printk(KERN_INFO "Try to recover rx ring exhausted...\n");
/* Re-allocate skbuffs to fill the descriptor ring */
for (; np->cur_rx - np->old_rx > 0; np->old_rx++) {
struct sk_buff *skb;
dma_addr_t addr;
entry = np->old_rx % RX_RING_SIZE;
/* Dropped packets don't need to re-allocate */
if (np->rx_skbuff[entry])
goto fill_entry;
skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
if (skb == NULL)
goto out_clear_fraginfo;
addr = dma_map_single(&np->pdev->dev, skb->data,
np->rx_buf_sz,
DMA_FROM_DEVICE);
if (dma_mapping_error(&np->pdev->dev, addr))
goto out_kfree_skb;
np->rx_skbuff[entry] = skb;
np->rx_ring[entry].fraginfo = cpu_to_le64(addr);
fill_entry:
np->rx_ring[entry].fraginfo |=
cpu_to_le64((u64)np->rx_buf_sz << 48);
np->rx_ring[entry].status = 0;
continue;
out_kfree_skb:
dev_kfree_skb_irq(skb);
out_clear_fraginfo:
np->rx_ring[entry].fraginfo = 0;
printk(KERN_INFO
"%s: Still unable to re-allocate Rx skbuff.#%d\n"
, dev->name, entry);
break;
} /* end for */
} /* end if */
spin_unlock_irqrestore (&np->rx_lock, flags);
np->timer.expires = jiffies + next_tick;
add_timer(&np->timer);
}
```
Or is there any better way to handle errors here?
I'd appreciate your guidance.
Powered by blists - more mailing lists