Re: [Intel-wired-lan] [PATCH iwl-next v2 7/8] ixgbe: use GFP_KERNEL in ixgbe_fcoe_ddp_setup()

Simon Horman
2026-04-14T13:29:48Z

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ixgbe: use GFP_KERNEL in ixgbe_fcoe_ddp_setup()

This commit restructures ixgbe_fcoe_ddp_setup() to use GFP_KERNEL
instead of GFP_ATOMIC by splitting get_cpu()/put_cpu() usage into two
narrow critical sections, moving DMA mapping and pool allocation between
them where preemption is enabled.

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 011fda9c6193..e338ff0e6522 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c

[ ... ]

> @@ -179,29 +180,43 @@ static int ixgbe_fcoe_ddp_setup(struct net_device *netdev, u16 xid,
> return 0;
> }
>
> + /* Pin to current CPU only to read the per-CPU pool pointer; drop
> + * the pin before any allocations that may sleep under direct reclaim.
> + */
> ddp_pool = per_cpu_ptr(fcoe->ddp_pool, get_cpu());
> if (!ddp_pool->pool) {
> e_warn(drv, "xid=0x%x no ddp pool for fcoe\n", xid);
> - goto out_noddp;
> + put_cpu();
> + return 0;
> }
> + pool = ddp_pool->pool;
> + put_cpu();
>
> /* setup dma from scsi command sgl */
> dmacount = dma_map_sg(&adapter->pdev->dev, sgl, sgc, DMA_FROM_DEVICE);
> if (dmacount == 0) {
> e_err(drv, "xid 0x%x DMA map error\n", xid);
> - goto out_noddp;
> + return 0;
> }
>
> - /* alloc the udl from per cpu ddp pool */
> - ddp->udl = dma_pool_alloc(ddp_pool->pool, GFP_ATOMIC, &ddp->udp);
> + /* Allocate from per-CPU pool; GFP_KERNEL is safe: preemption is
> + * re-enabled after the put_cpu() above. Per-CPU DMA pools are only
> + * destroyed under RTNL during interface teardown, so the saved pool
> + * pointer remains valid.
> + */
> + ddp->udl = dma_pool_alloc(pool, GFP_KERNEL, &ddp->udp);

Is this safe given the calling context?

Looking at the call path from the FCoE stack:

fc_exch_seq_send()
fc_exch_alloc()
fc_exch_em_alloc()
spin_lock_bh(&pool->lock)
# allocate exchange
# set ep->ex_lock = held
return with ep->ex_lock still held
fc_fcp_ddp_setup()
lport->tt.ddp_setup()
.ndo_fcoe_ddp_setup() == ixgbe_fcoe_ddp_get()
ixgbe_fcoe_ddp_setup() # <-- we are here
# ... continues with ep->ex_lock held
spin_unlock_bh(&ep->ex_lock)

fc_exch_em_alloc() acquires ep->ex_lock (a spinlock) and returns with
it held. The lock remains held through fc_fcp_ddp_setup() and into
ixgbe_fcoe_ddp_setup(). Additionally, spin_lock_bh() disables bottom
halves.

Can GFP_KERNEL be used while holding ep->ex_lock with BH disabled, or
does this require GFP_ATOMIC?