linux-kernel - Re: [PATCH 2/2] genirq: Resend nested irq's ancestor irq

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] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.02.1206151159340.3086@ionos>
Date:	Fri, 15 Jun 2012 14:18:00 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Ning Jiang <ning.n.jiang@...il.com>
cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] genirq: Resend nested irq's ancestor irq

On Fri, 15 Jun 2012, Ning Jiang wrote:
> 2012/6/14 Thomas Gleixner <tglx@...utronix.de>:
> > The correct solution for this is to replace the tasklet with a kernel
> > thread and check whether the interrupt is marked nested or not and
> > then invoke the correct function.
> >
> 
> Do you want something like this?

Definitely not.

> This just gives a rough idea. Please help to review.

It's a very bad idea.

>  #ifdef CONFIG_HARDIRQS_SW_RESEND
> -			/* Set it pending and activate the softirq: */
> -			set_bit(irq, irqs_resend);
> -			tasklet_schedule(&resend_tasklet);
> +			struct task_struct *t;
> +			if (irq_settings_is_nested_thread(desc)) {
> +				raw_spin_unlock(&desc->lock);
> +				t = kthread_create(irq_thread, desc->action,
> +					"irq/%d-%s", irq, desc->action->name);

Did you even try to run that code?

You CANNOT call kthread_create() with interrupts disabled and you
CANNOT expect that the state of the irq desc is unchanged when you
drop the lock.

And even if this would work it would be fricking insane to create a
kernel thread every time we resend an interrupt. Not to talk about the
exit madness you decided to stick into the existing thread function.

Either we have a kthread set up explicitely for that and just have to
wake it up or if we can deal with the possible latency just hand it
off to workqueue context.

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/