linux-kernel - Re: [PATCH 7/8 v3] i2c: i801: enable irq for i801 smbus transactions

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: <20120706135500.44b17c42@endymion.delvare>
Date:	Fri, 6 Jul 2012 13:55:00 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	Daniel Kurtz <djkurtz@...omium.org>
Cc:	Ben Dooks <ben-linux@...ff.org>,
	Wolfram Sang <w.sang@...gutronix.de>,
	Seth Heasley <seth.heasley@...el.com>,
	Olof Johansson <olof@...om.net>,
	Benson Leung <bleung@...omium.org>, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/8 v3] i2c: i801: enable irq for i801 smbus
 transactions

Hi Daniel,

On Fri, 6 Jul 2012 18:28:28 +0800, Daniel Kurtz wrote:
> On Thu, Jul 5, 2012 at 4:10 PM, Jean Delvare <khali@...ux-fr.org> wrote:
> > On Thu, 5 Jul 2012 12:31:11 +0800, Daniel Kurtz wrote:
> >> The logic for clearing the STATUS_FLAGS was simply that they all
> >> indicate the end of a transaction, so there won't be any further
> >> interrupts.  Thus, it is safe to clear it in the irq, since the irq
> >> will be followed by exactly one wait_event, which can read and process
> >> saved status.
> >
> > It is safe if and only if someone is actually listening to the event.
> > This was not the case for me yesterday. Even if we don't mix interrupts
> > and polling, i801_isr() might still get called when we aren't
> > listening. Not only because the IRQ may be shared, but also for events
> > we don't yet handle, such as an SMBus Alert. Not sure about slave
> > mode.
> 
> The real reason for clearing the flag in the hard irq is that
> otherwise, we end up with an infinite irq loop.  The interrupt is
> apparently level triggered, and must be cleared before exiting the
> ISR.

Oh, OK. If we have to do it that way, then we do.

> Unfortunately, I only had time today to discover this, but not time to fix it.
> Next week I will be away, so I won't have a chance to provide more
> patches for the next 10 days.
> 
> I think the 6 patches you have already make a complete set, however,
> and shouldn't be waiting for the interrupt patch(es) which can follow
> at some future date.

Agreed, that's why they are already in linux-next waiting for the next
merge window to open.

That being said, I would hate to miss that merge window for interrupt
support patches, as this is really a great performance improvement. So,
if you have no objection, I propose to fix your patches myself today of
tomorrow, test them, and if I find no problem, post them and push them
to linux-next. You can review and test them when you return.

-- 
Jean Delvare
--
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/