Re: [PATCH net-next 2/6] octeontx2-pf: Handle VF function level reset

From: Jakub Kicinski <kuba@kernel.org>
To: sunil.kovvuri@gmail.com
Cc: netdev@vger.kernel.org, davem@davemloft.net,
	Geetha sowjanya <gakula@marvell.com>,
	Sunil Goutham <sgoutham@marvell.com>
Subject: Re: [PATCH net-next 2/6] octeontx2-pf: Handle VF function level reset
Date: Tue, 10 Mar 2020 14:43:28 -0700	[thread overview]
Message-ID: <20200310144328.0db53c68@kicinski-fedora-PC1C0HJN> (raw)
In-Reply-To: <1583866045-7129-3-git-send-email-sunil.kovvuri@gmail.com>

On Wed, 11 Mar 2020 00:17:21 +0530 sunil.kovvuri@gmail.com wrote:
> +static int otx2_pf_flr_init(struct otx2_nic *pf, int num_vfs)
> +{
> +	int vf;
> +
> +	pf->flr_wq = alloc_workqueue("otx2_pf_flr_wq", WQ_UNBOUND | WQ_HIGHPRI
> +				     | WQ_MEM_RECLAIM, 1);

Are you sure WQ_MEM_RECLAIM is necessary? AFAIU sending FLRs is not part
of memory reclaim, and therefore this flag isn't needed?

> +	if (!pf->flr_wq)
> +		return -ENOMEM;
> +
> +	pf->flr_wrk = devm_kcalloc(pf->dev, num_vfs,
> +				   sizeof(struct flr_work), GFP_KERNEL);

Since SR-IOV can be enabled and disabled multiple times you should free
this memory explicitly when it's disabled. Otherwise the driver will
hold onto it until remove.

> +	if (!pf->flr_wrk) {
> +		destroy_workqueue(pf->flr_wq);
> +		return -ENOMEM;
> +	}
> +
> +	for (vf = 0; vf < num_vfs; vf++) {
> +		pf->flr_wrk[vf].pf = pf;
> +		INIT_WORK(&pf->flr_wrk[vf].work, otx2_flr_handler);
> +	}
> +
> +	return 0;
> +}