Re: [PATCH rdma-next 4/4] net: mana: Add support for auxiliary device servicing events

Leon Romanovsky
2025-04-20T10:53:14Z

On Mon, Apr 14, 2025 at 11:28:49AM -0700, Konstantin Taranov wrote:
> From: Shiraz Saleem <shirazsaleem@microsoft.com>
>
> Handle soc servcing events which require the rdma auxiliary device resources to
> be cleaned up during a suspend, and re-initialized during a resume.
>
> Signed-off-by: Shiraz Saleem <shirazsaleem@microsoft.com>
> Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> ---
> .../net/ethernet/microsoft/mana/gdma_main.c | 11 +++-
> .../net/ethernet/microsoft/mana/hw_channel.c | 19 ++++++
> drivers/net/ethernet/microsoft/mana/mana_en.c | 60 +++++++++++++++++++
> include/net/mana/gdma.h | 18 ++++++
> include/net/mana/hw_channel.h | 9 +++
> 5 files changed, 116 insertions(+), 1 deletion(-)

<...>

> @@ -1474,6 +1481,8 @@ static void mana_gd_cleanup(struct pci_dev *pdev)
> mana_hwc_destroy_channel(gc);
>
> mana_gd_remove_irqs(pdev);
> +
> + destroy_workqueue(gc->service_wq);
> }

<...>

> +static void mana_handle_rdma_servicing(struct work_struct *work)
> +{
> + struct mana_service_work *serv_work =
> + container_of(work, struct mana_service_work, work);
> + struct gdma_dev *gd = serv_work->gdma_dev;
> + struct device *dev = gd->gdma_context->dev;
> + int ret;
> +
> + switch (serv_work->event) {
> + case GDMA_SERVICE_TYPE_RDMA_SUSPEND:
> + if (!gd->adev || gd->is_suspended)
> + break;
> +
> + remove_adev(gd);
> + gd->is_suspended = true;
> + break;
> +
> + case GDMA_SERVICE_TYPE_RDMA_RESUME:
> + if (!gd->is_suspended)
> + break;
> +
> + ret = add_adev(gd, "rdma");
> + if (ret)
> + dev_err(dev, "Failed to add adev on resume: %d\n", ret);
> + else
> + gd->is_suspended = false;
> + break;
> +
> + default:
> + dev_warn(dev, "unknown adev service event %u\n",
> + serv_work->event);
> + break;
> + }
> +
> + kfree(serv_work);

The series looks ok to me, except one question. Are you sure that it is
safe to have not-connected and not-locked general work while add_adev/remove_adev
can be called in parallel from different thread? For example getting event
GDMA_SERVICE_TYPE_RDMA_SUSPEND while mana_gd_probe() fails or some other
intervention with PCI (GDMA_SERVICE_TYPE_RDMA_SUSPEND and PCI shutdown).

What type of protection do you have here?

Thanks