linux-kernel - Re: [PATCH 2/3] libata: add support for blk-iopoll

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090810172235.GL12579@kernel.dk>
Date:	Mon, 10 Aug 2009 19:22:35 +0200
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Jonathan Corbet <corbet@....net>
Cc:	linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
	Eric.Moore@....com, jeff@...zik.org
Subject: Re: [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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/