[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171102085313.GD655@jagdpanzerIV>
Date: Thu, 2 Nov 2017 17:53:13 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
akpm@...ux-foundation.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Cong Wang <xiyou.wangcong@...il.com>,
Dave Hansen <dave.hansen@...el.com>,
Johannes Weiner <hannes@...xchg.org>,
Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...nel.org>,
Petr Mladek <pmladek@...e.com>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Vlastimil Babka <vbabka@...e.cz>,
"yuwang.yuwang" <yuwang.yuwang@...baba-inc.com>
Subject: Re: [PATCH] mm: don't warn about allocations which stall for too long
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()?
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 somehow reminds me of "static unsigned int logbuf_cpu", which
we used to have in vprintk_emit() and were happy to remove it...
the whole "console_unlock() is non-preemptible" can bite, I'm
afraid. it's not always printk()->console_unlock(), sometimes
it's console_lock()->console_unlock() that has to flush the
logbuf.
CPU0 CPU1 ~ CPU99
console_lock();
printk(); ... printk();
console_unlock()
preempt_disable();
for (;;)
call_console_drivers();
<<lockup>>
this pattern is not so unusual. _especially_ in the existing scheme
of things.
not to mention the problem of "the last printk()", which will take
over and do the flush.
CPU0 CPU1 ~ CPU99
console_lock();
printk(); ... printk();
console_unlock();
IRQ on CPU2
printk()
// take over console_sem
console_unlock()
and so on.
seems that there will be lots of if-s.
-ss
Powered by blists - more mailing lists