Re: [PATCH rdma-next 1/1] RDMA/mana_ib: device memory support

[Leon Romanovsky <leon@kernel.org>]

On Fri, Jan 16, 2026 at 01:04:50AM -0800, Konstantin Taranov wrote:
> From: Konstantin Taranov <kotaranov@microsoft.com>
> 
> Basic implementation of DM allowing to create and register
> DM memory and use its memory keys for networking.
> 
> Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
> ---
>  drivers/infiniband/hw/mana/device.c  |   7 ++
>  drivers/infiniband/hw/mana/mana_ib.h |  12 +++
>  drivers/infiniband/hw/mana/mr.c      | 138 +++++++++++++++++++++++++++
>  include/net/mana/gdma.h              |  47 ++++++++-
>  4 files changed, 201 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
> index bdeddb642..ccc2279ca 100644
> --- a/drivers/infiniband/hw/mana/device.c
> +++ b/drivers/infiniband/hw/mana/device.c
> @@ -69,6 +69,12 @@ static const struct ib_device_ops mana_ib_device_stats_ops = {
>  	.alloc_hw_device_stats = mana_ib_alloc_hw_device_stats,
>  };

<...>

> +static int mana_ib_gd_alloc_dm(struct mana_ib_dev *mdev, struct mana_ib_dm *dm,
> +			       struct ib_dm_alloc_attr *attr)
> +{
> +	struct gdma_context *gc = mdev_to_gc(mdev);
> +	struct gdma_alloc_dm_resp resp = {};
> +	struct gdma_alloc_dm_req req = {};
> +	int err;
> +
> +	mana_gd_init_req_hdr(&req.hdr, GDMA_ALLOC_DM, sizeof(req), sizeof(resp));
> +	req.length = attr->length;
> +	req.alignment = attr->alignment;
> +	req.flags =  attr->flags;
> +
> +	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> +	if (err || resp.hdr.status) {
> +		ibdev_dbg(&mdev->ib_dev, "Failed to alloc dm err %d, %u", err,
> +			  resp.hdr.status);
> +		if (!err)
> +			err = -EPROTO;
> +
> +		return err;
> +	}
> +
> +	dm->dm_handle = resp.dm_handle;
> +
> +	return 0;
> +}
> +
> +struct ib_dm *mana_ib_alloc_dm(struct ib_device *ibdev,
> +			       struct ib_ucontext *context,
> +			       struct ib_dm_alloc_attr *attr,
> +			       struct uverbs_attr_bundle *attrs)
> +{
> +	struct mana_ib_dev *dev = container_of(ibdev, struct mana_ib_dev, ib_dev);
> +	struct mana_ib_dm *dm;
> +	int err;
> +
> +	dm = kzalloc(sizeof(*dm), GFP_KERNEL);
> +	if (!dm)
> +		return ERR_PTR(-ENOMEM);
> +
> +	err = mana_ib_gd_alloc_dm(dev, dm, attr);
> +	if (err) {
> +		ibdev_dbg(ibdev, "Failed allocate dm memory, %d\n", err);

I intended to apply this patch, but encountered too many minor issues that
needed local fixes.

You already printed debug message when mana_ib_gd_alloc_dm() failed.

> +		goto err_free;
> +	}
> +
> +	return &dm->ibdm;
> +
> +err_free:
> +	kfree(dm);
> +	return ERR_PTR(err);
> +}
> +
> +static int mana_ib_gd_destroy_dm(struct mana_ib_dev *mdev, struct mana_ib_dm *dm)
> +{
> +	struct gdma_context *gc = mdev_to_gc(mdev);
> +	struct gdma_destroy_dm_resp resp = {};
> +	struct gdma_destroy_dm_req req = {};
> +	int err;
> +
> +	mana_gd_init_req_hdr(&req.hdr, GDMA_DESTROY_DM, sizeof(req), sizeof(resp));
> +	req.dm_handle = dm->dm_handle;
> +
> +	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> +	if (err || resp.hdr.status) {
> +		ibdev_dbg(&mdev->ib_dev, "Failed to destroy dm err %d, %u", err,
> +			  resp.hdr.status);
> +		if (!err)
> +			err = -EPROTO;
> +
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +int mana_ib_dealloc_dm(struct ib_dm *ibdm, struct uverbs_attr_bundle *attrs)
> +{
> +	struct mana_ib_dev *dev = container_of(ibdm->device, struct mana_ib_dev, ib_dev);
> +	struct mana_ib_dm *dm = container_of(ibdm, struct mana_ib_dm, ibdm);
> +	int err;
> +
> +	err = mana_ib_gd_destroy_dm(dev, dm);
> +	if (err)
> +		ibdev_dbg(ibdm->device, "Failed destroy dm memory, %d\n", err);

Same error print as for alloc.

In order to simplify things for you: unless you can clearly justify why this
print is required and why you cannot proceed without it, I must ask you to stop
adding any new debug or error messages to the mana_ib driver. There is a wide
range of existing tools and well‑established practices for debugging the kernel,
and none of them require spamming dmesg.

> +	/* Free dm even on HW errors as we do not use the DM anymore.
> +	 * Since DM is a purely HW resource, faulty HW cannot harm the kernel.
> +	 */
> +	kfree(dm);
> +	return err;

You have already freed the DM, so you should return 0 here and make
mana_ib_gd_destroy_dm() return void, as no one uses its return value.

Thanks