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

[Thomas Gleixner]

On Fri, 15 Jun 2012, Ning Jiang wrote:
> 2012/6/14 Thomas Gleixner <tglx@xxxxxxxxxxxxx>:
> > 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/