Re: [PATCH] mm: don't warn about allocations which stall for too long

[Sergey Senozhatsky]

On (11/02/17 17:53), Sergey Senozhatsky wrote:
> On (10/31/17 15:32), Steven Rostedt wrote:
> [..]
> > (new globals)
> > static DEFINE_SPIN_LOCK(console_owner_lock);
> > static struct task_struct console_owner;
> > static bool waiter;
> > 
> > console_unlock() {
> > 
> > [ Assumes this part can not preempt ]
> >
> > 	spin_lock(console_owner_lock);
> > 	console_owner = current;
> > 	spin_unlock(console_owner_lock);
> 
>  + disables IRQs?
> 
> > 	for each message
> > 		write message out to console
> > 
> > 		if (READ_ONCE(waiter))
> > 			break;
> > 
> > 	spin_lock(console_owner_lock);
> > 	console_owner = NULL;
> > 	spin_unlock(console_owner_lock);
> > 
> > [ preemption possible ]
> 
> otherwise
> 
>      printk()
>       if (console_trylock())
>         console_unlock()
>          preempt_disable()
>           spin_lock(console_owner_lock);
>           console_owner = current;
>           spin_unlock(console_owner_lock);
>           .......
>           spin_lock(console_owner_lock);
> IRQ
>     printk()
>      console_trylock() // fails so we go to busy-loop part
>       spin_lock(console_owner_lock);       << deadlock
> 
> 
> even if we would replace spin_lock(console_owner_lock) with IRQ
> spin_lock, we still would need to protect against IRQs on the very
> same CPU. right? IOW, we need to store smp_processor_id() of a CPU
> currently doing console_unlock() and check it in vprintk_emit()?


a major self-correction:

> and we need to protect the entire console_unlock() function. not
> just the printing loop, otherwise the IRQ CPU will spin forever
> waiting for itself to up() the console_sem.

this part is wrong. should have been
	"we need to protect the entire printing loop"


so now console_unlock()'s printing loop is going to run

a) under preempt_disable()
b) under local_irq_save()

which is risky.

	-ss