[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1fe20e5e-1c90-4029-9d40-625ec3bb3248@wanadoo.fr>
Date: Thu, 22 Aug 2024 07:34:16 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Haiyang Zhang <haiyangz@...rosoft.com>
Cc: ast@...nel.org, bpf@...r.kernel.org, daniel@...earbox.net,
davem@...emloft.net, decui@...rosoft.com, edumazet@...gle.com,
hawk@...nel.org, jesse.brandeburg@...el.com, john.fastabend@...il.com,
kuba@...nel.org, kys@...rosoft.com, leon@...nel.org,
linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-rdma@...r.kernel.org, longli@...rosoft.com, netdev@...r.kernel.org,
olaf@...fle.de, pabeni@...hat.com, paulros@...rosoft.com,
shradhagupta@...ux.microsoft.com, ssengar@...ux.microsoft.com,
stable@...r.kernel.org, stephen@...workplumber.org, tglx@...utronix.de,
vkuznets@...hat.com, wei.liu@...nel.org
Subject: Re: [PATCH net] net: mana: Fix race of mana_hwc_post_rx_wqe and new
hwc response
Le 21/08/2024 à 22:42, Haiyang Zhang a écrit :
> The mana_hwc_rx_event_handler() / mana_hwc_handle_resp() calls
> complete(&ctx->comp_event) before posting the wqe back. It's
> possible that other callers, like mana_create_txq(), start the
> next round of mana_hwc_send_request() before the posting of wqe.
> And if the HW is fast enough to respond, it can hit no_wqe error
> on the HW channel, then the response message is lost. The mana
> driver may fail to create queues and open, because of waiting for
> the HW response and timed out.
> Sample dmesg:
> [ 528.610840] mana 39d4:00:02.0: HWC: Request timed out!
> [ 528.614452] mana 39d4:00:02.0: Failed to send mana message: -110, 0x0
> [ 528.618326] mana 39d4:00:02.0 enP14804s2: Failed to create WQ object: -110
>
> To fix it, move posting of rx wqe before complete(&ctx->comp_event).
>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@...lic.gmane.org
> Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
> Signed-off-by: Haiyang Zhang <haiyangz-0li6OtcxBFHby3iVrkZq2A@...lic.gmane.org>
> ---
> .../net/ethernet/microsoft/mana/hw_channel.c | 62 ++++++++++---------
> 1 file changed, 34 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index cafded2f9382..a00f915c5188 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -52,9 +52,33 @@ static int mana_hwc_verify_resp_msg(const struct hwc_caller_ctx *caller_ctx,
> return 0;
> }
>
> +static int mana_hwc_post_rx_wqe(const struct hwc_wq *hwc_rxq,
> + struct hwc_work_request *req)
> +{
> + struct device *dev = hwc_rxq->hwc->dev;
> + struct gdma_sge *sge;
> + int err;
> +
> + sge = &req->sge;
> + sge->address = (u64)req->buf_sge_addr;
> + sge->mem_key = hwc_rxq->msg_buf->gpa_mkey;
> + sge->size = req->buf_len;
> +
> + memset(&req->wqe_req, 0, sizeof(struct gdma_wqe_request));
> + req->wqe_req.sgl = sge;
> + req->wqe_req.num_sge = 1;
> + req->wqe_req.client_data_unit = 0;
Hi,
unrelated to your patch, but this initialization is useless, it is
already memset(0)'ed a few lines above.
So why client_data_unit and not some other fields?
> +
> + err = mana_gd_post_and_ring(hwc_rxq->gdma_wq, &req->wqe_req, NULL);
> + if (err)
> + dev_err(dev, "Failed to post WQE on HWC RQ: %d\n", err);
> + return err;
> +}
...
Just my 2c.
CJ
Powered by blists - more mailing lists