On Mon, Jun 4, 2018 at 11:49 PM, Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> On Mon, Jun 04, 2018 at 02:29:57PM +0200, Roman Pen wrote:
>> This is just a preparation patch: put kref_put() and kref_get_unless_zero()
>> inside helpers with more suitable names.
>>
>> Signed-off-by: Roman Pen <roman.penyaev@xxxxxxxxxxxxxxx>
>> Cc: Christoph Hellwig <hch@xxxxxx>
>> Cc: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
>> Cc: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
>> Cc: Sagi Grimberg <sagi@xxxxxxxxxxx>
>> Cc: Doug Ledford <dledford@xxxxxxxxxx>
>> Cc: linux-nvme@xxxxxxxxxxxxxxxxxxx
>> drivers/nvme/target/rdma.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
>> index 52e0c5d579a7..4304b8d8d027 100644
>> +++ b/drivers/nvme/target/rdma.c
>> @@ -775,7 +775,7 @@ static int nvmet_rdma_init_srq(struct nvmet_rdma_device *ndev)
>> return ret;
>> }
>>
>> -static void nvmet_rdma_free_dev(struct kref *ref)
>> +static void nvmet_rdma_free_device(struct kref *ref)
>> {
>> struct nvmet_rdma_device *ndev =
>> container_of(ref, struct nvmet_rdma_device, ref);
>> @@ -790,6 +790,16 @@ static void nvmet_rdma_free_dev(struct kref *ref)
>> kfree(ndev);
>> }
>>
>> +static int nvmet_rdma_dev_put(struct nvmet_rdma_device *dev)
>> +{
>> + return kref_put(&dev->ref, nvmet_rdma_free_device);
>> +}
>> +
>> +static int nvmet_rdma_dev_get(struct nvmet_rdma_device *dev)
>> +{
>> + return kref_get_unless_zero(&dev->ref);
>> +}
>
> Don't wrap get_unless_zero with something called get..
>
> unless_zero should only be used in special situations where something tricky
> is going on.. In this case it looks like it is because a
> nvmet_rdma_device can continue to reside in the device_list() even after
> being fully 'put'..
>
> Which does pose a question to this whole series, what have you done to
> ensure the client data access isn't racing with the removal of the
> device/client?
It races, but nvmet_rdma_remove_one() has guarantees that all queued
release works will finish, so no users of ndev are left at the point
when flush_scheduled_work() returns control:
static void
nvme_rdma_remove_one(struct ib_device *ib_device, void *client_data)
{
...
flush_workqueue(nvme_delete_wq);
/*
* Here at this point no users of ndev are left
*/
}
--
Roman
--
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
