On Apr 16, 2014, at 11:23 AM, Sagi Grimberg <sagig@xxxxxxxxxxxxxxxxxx> wrote:
> On 4/16/2014 6:08 PM, Chuck Lever wrote:
>> Hi Sagi-
>>
>> Thanks for the review! In-line replies below.
> <SNIP>
>>
>>>> 2. If I understood correctly, I see that the CQs are loop-polled in an interrupt context.
>>>> This may become problematic in stress workload where the CQ simply never drains (imagine
>>>> some studios task keeps posting WRs as soon as IOs complete). This will cause CQ poll loop
>>>> to go on forever. This situation may cause starvation of other CQs that share the same EQ (CPU
>>>> is hogged by the endless poller).
>>>> This may be solved in 2 ways:
>>>> - Use blk-iopoll to maintain fairness - the downside will be moving from interrupt context to soft-irq (slower).
>>>> - Set some configurable budget to CQ poller - after finishing the budget, notify the CQ and bail.
>>>> If there is another CQ waiting to get in - it's only fair that it will be given with a chance, else another interrupt
>>>> on that CQ will immediately come.
>> I think it would be a reasonable change to pass an array of WC’s to
>> ib_poll_cq() just once in rpcrdma_{send,recv}cq_poll(), instead of
>> looping. Would that be enough?
>>
>> To be clear, my patch merely cleans up the completion handler logic,
>> which already did loop-polling. Should we consider this improvement
>> for another patch?
>
> Well, I wasn't suggesting passing an array, carrying it around (or on the stack for that manner) might be annoying...
> I was suggesting a budget (poll loops or a time bound - jiffy is usually well behaved).
Passing a small array to ip_poll_cq() is actually easy to do, and is
exactly equivalent to a poll budget. The struct ib_wc should be taken
off the stack anyway, IMO.
The only other example I see in 3.15 right now is IPoIB, which seems
to do exactly this.
I’m testing a patch now. I’d like to start simple and make it more
complex only if we need to.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
