Re: About a shortcoming of the verbs API

Bart Van Assche
2010-08-09T07:57:56Z

On Mon, Aug 9, 2010 at 1:51 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>
> [ ... ]
>
> Further, the approach you outline in your follow on message for
> blkio, has problems.. Look at how IPOIB does NAPI to see how
> this must look.
>
> 1) ib_req_notify_cq must only be called if you are processing less
>   the budget
> 2) blk_iopoll_complete must be called prior to ib_req_notify_cq, since
>   call ib_req_notify_cq can immediately generate an interrupt, and
>   that interrupt must see the sched bit as cleared. If
>   ib_req_notify_cq races then you have to blkiopoll_reschedual.
>   (and maybe continue looping depending on your strategy for
>    call blkio_poll_disable elsewhere)
> 3) The idea you can hand off to normal processing if
>   blk_iopoll_sched_prep fails in the ISR does not work for anything
>   relying on the non-rentrancy of the blkio_poll call back for
>   locking. This seems to describe the SRP driver.

Good catches.

> There is no easy way you can switch from processing in a non-ISR
> context to processing in an interrupt on the fly.. Each relies on
> different implicit locking and switching between those two domains is
> ugly. Something like this pseudo-code:
>
> srp_supress_ib_req_notify_cq = 1;
> blkio_poll_disable();
>
> // now there will be no more blkio calls, and no more interrupts!
>
> // Neuter the ISR while we are piddling:
> set_bit(IOPOLL_F_DISABLE, &iop->state);
>
> // Drain the CQ
> poll_again:
> while (srp_recv_poll_once())
>   ;
>
> // Try to swith back to interrupts!
> disable_interrupts();
> ret = ib_req_notify_cq(priv->recv_cq,
>                       IB_CQ_NEXT_COMP |
>                       IB_CQ_REPORT_MISSED_EVENTS));
> if (ret) {
>   enable_interrupts();
>   goto poll_agian;
> }
>
> // OK! We will *definately* get an interrupt now!
> srp_do_not_use_blkio_poll = 1;
> enable_interrupts();

Regarding the above pseudo-code: I do not know of any Linux kernel
version that defines the functions disable_interrupts() and
enable_interrupts(). There exist functions however that allow to
disable and re-enable interrupts on the local CPU. But for the above
pseudo-code, disabling interrupts on the local CPU only would make the
above code behave incorrectly on multiprocessor systems. And as far as
I know there are no functions available to disable all interrupts on
all CPUs - which would be a very expensive operation anyway. Invoking
disable_irq() and enable_irq() could help, but this requires knowledge
of the interrupt number, which is not available in the context of a
completion handler. So I'm not sure whether it is possible to
translate the above approach from pseudo-code to real code.

Bart.