linux-kernel - RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically

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]

Date:	Sat, 19 Feb 2011 16:12:27 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	KY Srinivasan <kys@...rosoft.com>
cc:	"gregkh@...e.de" <gregkh@...e.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"virtualization@...ts.osdl.org" <virtualization@...ts.osdl.org>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	Hank Janssen <hjanssen@...rosoft.com>
Subject: RE: [PATCH]: Staging: hv: Allocate the vmbus irq dynamically

On Sat, 19 Feb 2011, KY Srinivasan wrote:
> > From: Thomas Gleixner [mailto:tglx@...utronix.de]
> > Please do not use probe_irq_on for dynamic irq allocation. Highjacking
> > the lower PIC irqs is really not a good idea. Depending on when this
> > runs, you might grab an irq required by a driver which gets loaded
> > later.
> > 
> > Could you please explain what you're trying to do here ?
> 
> The IRQ being allocated is for the VMBUS driver for Linux guests running on
> a Windows virtualization platform (Hyper-V hypervisor).
> The hypervisor is capable of notifying events on the VMBUS via
> a guest specified interrupt line. Prior to this patch,
> the code was statically selecting an interrupt line for
> use by VMBUS. One of the long standing review comments
> on  that code was to make this irq allocation dynamic and that
> is what this patch does. For the Linux guest running as a VM
> on Hyper-V, the concern you raise is not an issue.

That patch does a whole lot of useless crap. 

When grabbing some random irq from the PIC is not an issue, then
what's the point of this probing, retry loop and the comments about
racing ? What races here? That does not make sense at all.

I don't know why the previous reviewer wanted to have that
dynamic. That just does not make sense to me.

Btw, that whole interrupt handler with two tasklets, one of them
scheduling work is just screaming threaded interrupt handler.

Thanks,

	tglx
--
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/