netdev - Re: [PATCH net] ionic: Fix allocation of q/cq info structures from device local node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 9 Apr 2023 13:52:42 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Brett Creeley <brett.creeley@....com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, kuba@...nel.org,
        drivers@...sando.io, shannon.nelson@....com, neel.patel@....com
Subject: Re: [PATCH net] ionic: Fix allocation of q/cq info structures from
 device local node

On Fri, Apr 07, 2023 at 04:36:45PM -0700, Brett Creeley wrote:
> Commit 116dce0ff047 ("ionic: Use vzalloc for large per-queue related
> buffers") made a change to relieve memory pressure by making use of
> vzalloc() due to the structures not requiring DMA mapping. However,
> it overlooked that these structures are used in the fast path of the
> driver and allocations on the non-local node could cause performance
> degredation. Fix this by first attempting to use vzalloc_node()
> using the device's local node and if that fails try again with
> vzalloc().
> 
> Fixes: 116dce0ff047 ("ionic: Use vzalloc for large per-queue related buffers")
> Signed-off-by: Neel Patel <neel.patel@....com>
> Signed-off-by: Brett Creeley <brett.creeley@....com>
> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
> ---
>  .../net/ethernet/pensando/ionic/ionic_lif.c   | 24 ++++++++++++-------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 957027e546b3..2c4e226b8cf1 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -560,11 +560,15 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
>  	new->q.dev = dev;
>  	new->flags = flags;
>  
> -	new->q.info = vzalloc(num_descs * sizeof(*new->q.info));
> +	new->q.info = vzalloc_node(num_descs * sizeof(*new->q.info),
> +				   dev_to_node(dev));
>  	if (!new->q.info) {
> -		netdev_err(lif->netdev, "Cannot allocate queue info\n");
> -		err = -ENOMEM;
> -		goto err_out_free_qcq;
> +		new->q.info = vzalloc(num_descs * sizeof(*new->q.info));
> +		if (!new->q.info) {
> +			netdev_err(lif->netdev, "Cannot allocate queue info\n");

Kernel memory allocator will try local node first and if memory is
depleted it will go to remote nodes. So basically, you open-coded that
behaviour but with OOM splash when first call to vzalloc_node fails and
with custom error message about memory allocation failure.

Thanks