Re: [RFC 1/5] nvme-rdma: use ib_client API to wrap ib_device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




   -     if (!(ndev->dev->attrs.device_cap_flags &
-             IB_DEVICE_MEM_MGT_EXTENSIONS)) {
-               dev_err(&ndev->dev->dev,
-                       "Memory registrations not supported.\n");
-               goto out_free_pd;
-       }


Why was this removed from here? I think it should live in
nvme_rdma_alloc_device.

Hi Sagi,

The idea was to move such capability check exactly inside event
handler nvme_rdma_add_one(), i.e. event comes, we firstly check
is this device capable or not and if not we do not take any other
branches but return with pr_err().

But device creation is called when we are completely sure that
we can create the device, i.e. if error happens inside
nvme_rdma_alloc_device() that will be something not nice, i.e.
-ENOMEM, but not more generic -ENOTSUPP.

Not sure I understand why this is not nice...

Of course that is only my sense of beauty :)  I do not insist
and for sure can calmly move that check back.

I think we can keep it where it was.

+       ib_set_client_data(ndev->dev, &nvme_rdma_ib_client, ndev);


This however, should be done in the call-site after this completes
successfully.

Well, then nvme_rdma_alloc_device() and its brother nvme_rdma_free_dev()
(hm, brother's name should also end with *ice, will fix that) won't be
so similar, please take a look:

nvme_rdma_free_dev(...)
{
      ...
      ib_set_client_data(ndev->dev, &nvme_rdma_ib_client, NULL);
      ...
}

nvme_rdma_alloc_device(...)
{
      ...
      ib_set_client_data(ndev->dev, &nvme_rdma_ib_client, ndev);
      ...
}

So if _free_dev() does ib_set_client_data() then for the sake
of symmetry *_alloc_device() also does the same.

Makes sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux