Re: [PATCH RFC] nvme-rdma: support devices with queue size < 32

[Marta Rybczynska <mrybczyn-FNhOzJFKnXGHXe+LvDLADg@public.gmane.org>]

----- Mail original -----
>>> Thanks, this looks useful.  But wouldn't it be better to do something
>>> like queue_size divided by 2 or 4 to get a better refill latency?
>>
>> That's an interesting question. The max number of requests is already at 3 or 4
>> times
>> of the queue size because of different message types (see Sam's original
>> message in 'NVMe RDMA driver: CX4 send queue fills up when nvme queue depth is
>> low').
>> I guess it would have inflence on configs with bigger latency.
>>
>> I would like to have Sagi's view on this as he's the one who has changed that
>> part in the iSER initiator in 6df5a128f0fde6315a44e80b30412997147f5efd
> 
> Hi Marta,
> 
> This looks good indeed. IIRC both for IB and iWARP we need to signal
> at least once every send-queue depth (in practice I remember that some
> devices need more than once) so maybe it'll be good to have division by
> 2.
> 
> Maybe it'll be better if we do:
> 
> static inline bool queue_sig_limit(struct nvme_rdma_queue *queue)
> {
>	return (++queue->sig_count % (queue->queue_size / 2)) == 0;
> }
> 
> And lose the hard-coded 32 entirely. Care to test that?

Hello Sigi,
I agree with you, we've found a setup where the signalling every queue
depth is not enough and we're testing the division by two that seems
to work fine till now.

In your version in case of queue length > 32 the notifications would
be sent less often that they are now. I'm wondering if it will have
impact on performance and internal card buffering (it seems that
Mellanox buffers are ~100 elements). Wouldn't it create issues?

I'd like see the magic constant removed. From what I can see we 
need to have something not exceeding send buffer of the card but 
also not lower than the queue depth. What do you think?

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