lkml.org 
[lkml]   [2009]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/3] libata: add support for blk-iopoll
    On Mon, Aug 10 2009, Jonathan Corbet wrote:
    > Hey, Jens,
    >
    > I'm a little slow in looking at this, hopefully it's not completely
    > noise...
    >
    > > @@ -2157,7 +2194,12 @@ static irqreturn_t ahci_interrupt(int irq, void *dev_instance)
    > >
    > > ap = host->ports[i];
    > > if (ap) {
    > > - ahci_port_intr(ap);
    > > + if (!blk_iopoll_enabled)
    > > + ahci_port_intr(ap);
    > > + else if (blk_iopoll_sched_prep(&ap->iopoll)) {
    > > + ap_irq_disable(ap);
    > > + blk_iopoll_sched(&ap->iopoll);
    > > + }
    > > VPRINTK("port %u\n", i);
    > > } else {
    > > VPRINTK("port %u (no irq)\n", i);
    >
    > It seems to me that, if blk_iopoll_sched_prep() fails, the interrupt
    > will be dropped on the floor; would you not need an explicit
    > ahci_port_intr() call in that case too? Unless I've misunderstood as
    > usual...

    If that happens, it is probably a spurious IRQ since it's already
    scheduled to run (and hasn't yet). So it should be fine, in reality it
    should not happen since the IRQ should have been acked and the iopoll
    handler scheduled.

    > Documenting the "zero means failure" nature of blk_iopoll_sched_prep()
    > might also be a good idea; I predict confusion otherwise.

    There's no real failure case, it zero just means "already scheduled".
    But we do usually use 0 as the "normal" case, so good point anyway. I'll
    change it.

    --
    Jens Axboe



    \
     
     \ /
      Last update: 2009-08-10 19:25    [W:0.020 / U:32.972 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site