Re: [PATCH] NVMe: Avoid interrupt disable during queue init.

[Parav Pandit]

On Fri, May 22, 2015 at 11:17 PM, Keith Busch <keith.busch@xxxxxxxxx> wrote:
> On Fri, 22 May 2015, Parav Pandit wrote:
>>
>> I agree to it that nvmeq won't be null after mb(); That alone is not
>> sufficient.
>>
>> What I have proposed in previous email is,
>>
>> Converting,
>>
>> struct nvme_queue *nvmeq = dev->queues[i];
>> if (!nvmeq)
>>    continue;
>> spin_lock_irq(nvmeq->q_lock);
>>
>> to replace with,
>>
>> struct nvme_queue *nvmeq = rcu_dereference(dev->queues[i]);
>> if (!nvmeq)
>>    continue;
>> spin_lock_irq(nvmeq->q_lock);
>>
>> This will prevent fetching content of q_lock before checking for NULL
>> condition. Classic usage or RCU.
>
>
> What the heck are you talking about? The value of dev->queue_count won't
> even let the thread iterate an nvmeq before q_lock is initialized.
>
Oh, right. My bad. I was getting confused with the possible reordering
in reset path within,
nvme_free_queues()
1320                 dev->queue_count--;
1321                 dev->queues[i] = NULL;

while nvme_kthread is working on it.
But thats unlikely case because device is removed from the list before.

Yes, so mb() is good enough.
I will send you the patch.
Thanks.


> We used to rcu protect queue access, but that was to make nvme's
> make_request_fn safe to surprise removal, not for the polling thread.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/