Re: [Intel-wired-lan] [PATCH iwl-next] ixgbe: replace GFP_ATOMIC with GFP_KERNEL in ixgbe_fcoe_ddp_setup()

[Simon Horman <horms@kernel.org>]

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: replace GFP_ATOMIC with GFP_KERNEL in ixgbe_fcoe_ddp_setup()

This commit changes the memory allocation flag in ixgbe_fcoe_ddp_setup()
from GFP_ATOMIC to GFP_KERNEL based on the claim that the function is
never called from atomic context.

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 011fda9c6193..7fa0971073e5 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> @@ -193,7 +193,7 @@ static int ixgbe_fcoe_ddp_setup(struct net_device *netdev, u16 xid,
>  	}
>
>  	/* alloc the udl from per cpu ddp pool */
> -	ddp->udl = dma_pool_alloc(ddp_pool->pool, GFP_ATOMIC, &ddp->udp);
> +	ddp->udl = dma_pool_alloc(ddp_pool->pool, GFP_KERNEL, &ddp->udp);
                                                    ^^^^^^^^^^
Doesn't this allocation happen in atomic context? Looking at the function,
get_cpu() is called earlier:

ixgbe_fcoe_ddp_setup() {
    ...
    ddp_pool = per_cpu_ptr(fcoe->ddp_pool, get_cpu());
    ...
    ddp->udl = dma_pool_alloc(ddp_pool->pool, GFP_KERNEL, &ddp->udp);
    ...
    put_cpu();
}

get_cpu() disables preemption, which means GFP_KERNEL allocations (which
can sleep during direct reclaim) would trigger a BUG() when attempted with
preemption disabled.

The commit message claims the function is "never from an atomic context",
but while the callers may be in process context, the function itself creates
atomic context via get_cpu().

>  	if (!ddp->udl) {
>  		e_err(drv, "failed allocated ddp context\n");
>  		goto out_noddp_unmap;