[openib-general][PATCH][kdapl]: FMR and EVD patch

[Guy German]

James Lentini wrote:
> I agree with you on the problems poised by the current interface. I 
> hope we can find a solution that fixes the problem. 
> Note that the same problem must be handled by a ULP using the native verbs.

I don't think we have the same problem in the verbs.
In the currently Mellanox hw (which is AFAIK the only available hw in openib) 
there is no race at all (because of the proprietary, more “considerate”, 
completion notification implementation). 

 - Receive a CQ notification callback
 - Wakeup polling thread
 - Poll for completion (empty the queue) 
 - Request completion notification
[you will get a completion notification even for “old” completions on the queue]
- exit thread

In the case of other, more harsh ib compliant future hw implementation – 
Request completion Notification “extended verb” could encapsulate:
- request CQ notification
- if cq !empty request CQ notification _again_
(note that you are not *polling* the cq – just checking the queue.
This is different then draining the evd "one more time")

And the race is solved.
Indeed, it is not as efficient as sparing the context switch 
(to interrupt and back to thread) altogether. 

>I still think that there may be a race condition with this patch. 
>Here's the scenario I'm concerned about:
> -     Receive an evd upcall
> -     Disable evd upcall policy
> -     Wakeup polling thread
> -     Dequeue all events
> -     Enable evd upcall policy by:
>       1. Call dapl_evd_modify_upcall() to enable the evd upcall
>       2. Obtain the EVD spin lock via spin_lock_irqsave, thus 
>          disabling local interrupts
>       3. Check that the EVD's ring buffer is empty (there are no DAPL 
>          "software" events)
>       4. A DTO completion occurs on the EVD's CQ
>       5. Enable the CQ's upcall via ib_req_notify_cq()
>
>If I understand you correctly, you are asserting that event #4, the 
>CQ's DTO completion, cannot occur because the local interrupts are 
>disabled by spin_lock_irqsave(). Have I understood you correctly?

Not quite. The *consumer’s upcall* would not be called, due to the irq disable. 
The race would not occur, OTOH, because the Mellanox hw will initiate a 
completion notification even if the completions in the cq arrived before 
the notification request.
If you want to be more ib compliant, for future possible implementations, 
you can apply the “extended-notify-routine” (as mentioned above).

> My belief is that the completion will occur on the card 
> regardless of the interrupt state. 

True, but the consumer will be notified only as soon as the irq 
is enabled again

> Can you provide me with a reference that guarantees this 
> will not happen?

I’m not saying that it won’t  ;) but I don't think there will be a race...

Guy