From: Petr Mladek <pmladek@suse.com> To: Steven Rostedt <rostedt@goodmis.org> Cc: Vlastimil Babka <vbabka@suse.cz>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Cong Wang <xiyou.wangcong@gmail.com>, Dave Hansen <dave.hansen@intel.com>, Johannes Weiner <hannes@cmpxchg.org>, Mel Gorman <mgorman@suse.de>, Michal Hocko <mhocko@kernel.org>, Sergey Senozhatsky <sergey.senozhatsky@gmail.com>, "yuwang.yuwang" <yuwang.yuwang@alibaba-inc.com> Subject: Re: [PATCH] mm: don't warn about allocations which stall for too long Date: Thu, 2 Nov 2017 12:46:50 +0100 [thread overview] Message-ID: <20171102114650.GB31148@pathway.suse.cz> (raw) In-Reply-To: <20171101113647.243eecf8@gandalf.local.home> On Wed 2017-11-01 11:36:47, Steven Rostedt wrote: > On Wed, 1 Nov 2017 14:38:45 +0100 > Petr Mladek <pmladek@suse.com> wrote: > > My current main worry with Steven's approach is a risk of deadlocks > > that Jan Kara saw when he played with similar solution. > > And if there exists such a deadlock, then the deadlock exists today. The patch is going to effectively change console_trylock() to console_lock() and this might add problems. The most simple example is: console_lock() printk() console_trylock() was SAFE. console_lock() printk() console_lock() cause DEADLOCK! Sure, we could detect this and avoid waiting when console_owner == current. But does this cover all situations? What about? CPU0 CPU1 console_lock() func() console->write() take_lockA() func() printk() busy wait for console_lock() take_lockA() By other words, it used to be safe to call printk() from console->write() functions because printk() used console_trylock(). Your patch is going to change this. It is even worse because you probably will not use console_lock() directly and therefore this might be hidden for lockdep. BTW: I am still not sure how to make the busy waiter preferred over console_lock() callers. I mean that the busy waiter has to get console_sem even if there are some tasks in the workqueue. > > But let's wait for the patch. It might look and work nicely > > in the end. > > Oh, I need to write a patch? Bah, I guess I should. Where's all those > developers dying to do kernel programing where I can pass this off to? Yes, where are these days when my primary task was to learn kernel hacking? This would have been a great training material. I still have to invest time into fixing printk. But I personally think that the lazy offloading to kthreads is more promising way to go. It is pretty straightforward. The only problem is the guaranty of the takeover. But there must be a reasonable way how to detect that the system heart is still beating and we are not the only working CPU. Best Regards, Petr
WARNING: multiple messages have this Message-ID (diff)
From: Petr Mladek <pmladek@suse.com> To: Steven Rostedt <rostedt@goodmis.org> Cc: Vlastimil Babka <vbabka@suse.cz>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Cong Wang <xiyou.wangcong@gmail.com>, Dave Hansen <dave.hansen@intel.com>, Johannes Weiner <hannes@cmpxchg.org>, Mel Gorman <mgorman@suse.de>, Michal Hocko <mhocko@kernel.org>, Sergey Senozhatsky <sergey.senozhatsky@gmail.com>, "yuwang.yuwang" <yuwang.yuwang@alibaba-inc.com> Subject: Re: [PATCH] mm: don't warn about allocations which stall for too long Date: Thu, 2 Nov 2017 12:46:50 +0100 [thread overview] Message-ID: <20171102114650.GB31148@pathway.suse.cz> (raw) In-Reply-To: <20171101113647.243eecf8@gandalf.local.home> On Wed 2017-11-01 11:36:47, Steven Rostedt wrote: > On Wed, 1 Nov 2017 14:38:45 +0100 > Petr Mladek <pmladek@suse.com> wrote: > > My current main worry with Steven's approach is a risk of deadlocks > > that Jan Kara saw when he played with similar solution. > > And if there exists such a deadlock, then the deadlock exists today. The patch is going to effectively change console_trylock() to console_lock() and this might add problems. The most simple example is: console_lock() printk() console_trylock() was SAFE. console_lock() printk() console_lock() cause DEADLOCK! Sure, we could detect this and avoid waiting when console_owner == current. But does this cover all situations? What about? CPU0 CPU1 console_lock() func() console->write() take_lockA() func() printk() busy wait for console_lock() take_lockA() By other words, it used to be safe to call printk() from console->write() functions because printk() used console_trylock(). Your patch is going to change this. It is even worse because you probably will not use console_lock() directly and therefore this might be hidden for lockdep. BTW: I am still not sure how to make the busy waiter preferred over console_lock() callers. I mean that the busy waiter has to get console_sem even if there are some tasks in the workqueue. > > But let's wait for the patch. It might look and work nicely > > in the end. > > Oh, I need to write a patch? Bah, I guess I should. Where's all those > developers dying to do kernel programing where I can pass this off to? Yes, where are these days when my primary task was to learn kernel hacking? This would have been a great training material. I still have to invest time into fixing printk. But I personally think that the lazy offloading to kthreads is more promising way to go. It is pretty straightforward. The only problem is the guaranty of the takeover. But there must be a reasonable way how to detect that the system heart is still beating and we are not the only working CPU. Best Regards, Petr -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-11-02 11:46 UTC|newest] Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-26 11:28 [PATCH] mm: don't warn about allocations which stall for too long Tetsuo Handa 2017-10-26 11:28 ` Tetsuo Handa 2017-10-26 11:41 ` Michal Hocko 2017-10-26 11:41 ` Michal Hocko 2017-11-08 10:30 ` peter enderborg 2017-11-09 8:52 ` Michal Hocko 2017-11-09 8:52 ` Michal Hocko 2017-11-09 9:34 ` peter enderborg 2017-11-09 10:09 ` Michal Hocko 2017-11-09 10:09 ` Michal Hocko 2017-11-09 10:19 ` Tetsuo Handa 2017-11-09 10:19 ` Tetsuo Handa 2017-10-26 14:37 ` Johannes Weiner 2017-10-26 14:37 ` Johannes Weiner 2017-10-31 19:32 ` Steven Rostedt 2017-11-01 8:30 ` Vlastimil Babka 2017-11-01 8:30 ` Vlastimil Babka 2017-11-01 13:38 ` Petr Mladek 2017-11-01 13:38 ` Petr Mladek 2017-11-01 15:36 ` Steven Rostedt 2017-11-01 15:36 ` Steven Rostedt 2017-11-02 11:46 ` Petr Mladek [this message] 2017-11-02 11:46 ` Petr Mladek 2017-11-02 14:49 ` Steven Rostedt 2017-11-02 14:49 ` Steven Rostedt 2017-11-01 15:33 ` Steven Rostedt 2017-11-01 15:33 ` Steven Rostedt 2017-11-01 17:42 ` Vlastimil Babka 2017-11-01 17:42 ` Vlastimil Babka 2017-11-01 17:54 ` Steven Rostedt 2017-11-01 17:54 ` Steven Rostedt 2017-11-02 8:53 ` Sergey Senozhatsky 2017-11-02 8:53 ` Sergey Senozhatsky 2017-11-02 9:14 ` Sergey Senozhatsky 2017-11-02 9:14 ` Sergey Senozhatsky 2017-11-02 14:55 ` Steven Rostedt 2017-11-02 14:55 ` Steven Rostedt 2017-11-02 12:55 ` Michal Hocko 2017-11-02 12:55 ` Michal Hocko 2017-11-02 15:56 ` Steven Rostedt 2017-11-02 15:56 ` Steven Rostedt 2017-11-02 17:06 ` [PATCH v2] printk: Add console owner and waiter logic to load balance console writes Steven Rostedt 2017-11-02 17:06 ` Steven Rostedt 2017-11-02 17:10 ` Steven Rostedt 2017-11-02 17:10 ` Steven Rostedt 2017-11-02 17:38 ` Steven Rostedt 2017-11-02 17:38 ` Steven Rostedt 2017-11-03 10:19 ` Jan Kara 2017-11-03 10:19 ` Jan Kara 2017-11-03 11:18 ` Steven Rostedt 2017-11-03 11:18 ` Steven Rostedt
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171102114650.GB31148@pathway.suse.cz \ --to=pmladek@suse.com \ --cc=akpm@linux-foundation.org \ --cc=dave.hansen@intel.com \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@suse.de \ --cc=mhocko@kernel.org \ --cc=penguin-kernel@I-love.SAKURA.ne.jp \ --cc=rostedt@goodmis.org \ --cc=sergey.senozhatsky@gmail.com \ --cc=vbabka@suse.cz \ --cc=xiyou.wangcong@gmail.com \ --cc=yuwang.yuwang@alibaba-inc.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.