All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Tejun Heo <tj@kernel.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	linux-mm@kvack.org, xiyou.wangcong@gmail.com,
	dave.hansen@intel.com, hannes@cmpxchg.org, mgorman@suse.de,
	mhocko@kernel.org, pmladek@suse.com,
	sergey.senozhatsky@gmail.com, vbabka@suse.cz
Subject: Re: [PATCH v3] printk: Add console owner and waiter logic to load balance console writes
Date: Thu, 9 Nov 2017 14:33:58 +0900	[thread overview]
Message-ID: <20171109053358.GD775@jagdpanzerIV> (raw)
In-Reply-To: <20171109000658.7df5a791@vmware.local.home>

On (11/09/17 00:06), Steven Rostedt wrote:
> What does safe context mean?

"safe" means that we don't cause lockups, stalls, sched throttlings, etc.
by doing console_unlock() from that context [task].


> Do we really want to allow the printk thread to sleep when there's more
> to print? What happens if there's a crash at that moment? How do we safely
> flush out all the data when the printk thread is sleeping?

printk-kthread does not schedule with the console_sem locked. one
of the changes to console_unlock() introduced with printk-kthread,
which we can't have without offloading.


> Now we could have something that uses both nicely. When the
> printk_thread wakes up (we need to figure out when to do that), then it
> could constantly take over.

certainly we can have a better hand-off scheme in printk-kthread patch set.


> 
> 	CPU1				CPU2
> 	----				----
>    console_unlock()
>      start printing a lot
>      (more than one, wake up printk_thread)
> 
> 					printk thread wakes up
> 
> 					becomes the waiter
> 
>    sees waiter hands off
> 
> 					starts printing
> 
>    printk()
>      becomes waiter
> 
> 					sees waiter hands off
> 					then becomes new waiter! <-- key
> 
>     starts printing
>     sees waiter hands off
> 					continues printing

there are corners cases here. learned the hard way. real reproducers
do exist.

wake_up_process() may enqueue printk_thread on the same rq that
current printk task is running on. so if your printk(), for instance,
is from IRQ then offloading won't happen.


> That is, we keep the waiter logic, and if anyone starts printing too
> much, it wakes up the printk thread (hopefully on another CPU, or the
> printk thread should migrate)  when the printk thread starts running it

it must migrate, yes. currently I'm playing games with the affinity
mask of printk-kthread when I do offloading.


> becomes the new waiter if the console lock is still held (just like in
> printk). Then it gets handed off the printk. We could just have the
> printk thread keep going, though I'm not sure I would want to let it
> schedule while printing. 

yes, scheduling under console_sem is not right. we don't want this.
not anymore, at least.


> But it could also hand off printks (like
> above), but then take it back immediately. This would mean that a
> printk caller from a "critical" path will only get to do one message,
> before the printk thread asks for it again.
> 
> Perhaps we could have more than one printk thread that migrates around,
> and they each hand off the printing. This makes sure the printing
> always happens and that it never stops due to the console_lock holder
> sleeping and we never lock up one CPU that does printing. This would
> work with just two printk threads. When one starts a printk loop,
> another one wakes up on another CPU and becomes the waiter to get the
> handoff of the console_lock. Then the first could schedule out (migrate
> if the current CPU is busy), and take over. In  fact, this would
> basically have two CPUs bouncing back and forth to do the printing.

can be. I pushed it much further, once. [probably too far].
and had per-CPU printk-kthreads :)


> This gives us our cake and we get to eat it too.
> 
> One, printing never stops (no scheduling out), as there's two threads
> to share the load (obiously only on SMP machines).
> 
> There's no lock up. There's two threads that print a little, pass off
> the console lock, do a cond_resched(), then takes over again.
> 
> Bascially, what I'm saying is that this is not two different solutions.
> There is two algorithms that can work together to give us reliable
> output and not lock up the system in doing so.

sure, I understand.

	-ss

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Tejun Heo <tj@kernel.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	linux-mm@kvack.org, xiyou.wangcong@gmail.com,
	dave.hansen@intel.com, hannes@cmpxchg.org, mgorman@suse.de,
	mhocko@kernel.org, pmladek@suse.com,
	sergey.senozhatsky@gmail.com, vbabka@suse.cz
Subject: Re: [PATCH v3] printk: Add console owner and waiter logic to load balance console writes
Date: Thu, 9 Nov 2017 14:33:58 +0900	[thread overview]
Message-ID: <20171109053358.GD775@jagdpanzerIV> (raw)
In-Reply-To: <20171109000658.7df5a791@vmware.local.home>

On (11/09/17 00:06), Steven Rostedt wrote:
> What does safe context mean?

"safe" means that we don't cause lockups, stalls, sched throttlings, etc.
by doing console_unlock() from that context [task].


> Do we really want to allow the printk thread to sleep when there's more
> to print? What happens if there's a crash at that moment? How do we safely
> flush out all the data when the printk thread is sleeping?

printk-kthread does not schedule with the console_sem locked. one
of the changes to console_unlock() introduced with printk-kthread,
which we can't have without offloading.


> Now we could have something that uses both nicely. When the
> printk_thread wakes up (we need to figure out when to do that), then it
> could constantly take over.

certainly we can have a better hand-off scheme in printk-kthread patch set.


> 
> 	CPU1				CPU2
> 	----				----
>    console_unlock()
>      start printing a lot
>      (more than one, wake up printk_thread)
> 
> 					printk thread wakes up
> 
> 					becomes the waiter
> 
>    sees waiter hands off
> 
> 					starts printing
> 
>    printk()
>      becomes waiter
> 
> 					sees waiter hands off
> 					then becomes new waiter! <-- key
> 
>     starts printing
>     sees waiter hands off
> 					continues printing

there are corners cases here. learned the hard way. real reproducers
do exist.

wake_up_process() may enqueue printk_thread on the same rq that
current printk task is running on. so if your printk(), for instance,
is from IRQ then offloading won't happen.


> That is, we keep the waiter logic, and if anyone starts printing too
> much, it wakes up the printk thread (hopefully on another CPU, or the
> printk thread should migrate)  when the printk thread starts running it

it must migrate, yes. currently I'm playing games with the affinity
mask of printk-kthread when I do offloading.


> becomes the new waiter if the console lock is still held (just like in
> printk). Then it gets handed off the printk. We could just have the
> printk thread keep going, though I'm not sure I would want to let it
> schedule while printing. 

yes, scheduling under console_sem is not right. we don't want this.
not anymore, at least.


> But it could also hand off printks (like
> above), but then take it back immediately. This would mean that a
> printk caller from a "critical" path will only get to do one message,
> before the printk thread asks for it again.
> 
> Perhaps we could have more than one printk thread that migrates around,
> and they each hand off the printing. This makes sure the printing
> always happens and that it never stops due to the console_lock holder
> sleeping and we never lock up one CPU that does printing. This would
> work with just two printk threads. When one starts a printk loop,
> another one wakes up on another CPU and becomes the waiter to get the
> handoff of the console_lock. Then the first could schedule out (migrate
> if the current CPU is busy), and take over. In  fact, this would
> basically have two CPUs bouncing back and forth to do the printing.

can be. I pushed it much further, once. [probably too far].
and had per-CPU printk-kthreads :)


> This gives us our cake and we get to eat it too.
> 
> One, printing never stops (no scheduling out), as there's two threads
> to share the load (obiously only on SMP machines).
> 
> There's no lock up. There's two threads that print a little, pass off
> the console lock, do a cond_resched(), then takes over again.
> 
> Bascially, what I'm saying is that this is not two different solutions.
> There is two algorithms that can work together to give us reliable
> output and not lock up the system in doing so.

sure, I understand.

	-ss

  reply	other threads:[~2017-11-09  5:34 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-02 17:45 [PATCH v3] printk: Add console owner and waiter logic to load balance console writes Steven Rostedt
2017-11-02 22:16 ` Vlastimil Babka
2017-11-02 22:16   ` Vlastimil Babka
2017-11-03  3:15   ` Steven Rostedt
2017-11-03  3:15     ` Steven Rostedt
2017-11-04  3:13     ` Sergey Senozhatsky
2017-11-04  3:13       ` Sergey Senozhatsky
2017-11-03  4:09   ` John Hubbard
2017-11-03 11:21     ` Steven Rostedt
2017-11-03 11:21       ` Steven Rostedt
2017-11-03 11:54       ` Steven Rostedt
2017-11-03 11:54         ` Steven Rostedt
2017-11-03 11:54         ` Steven Rostedt
2017-11-03 11:54           ` Steven Rostedt
2017-11-03 21:46         ` John Hubbard
2017-11-04  3:34           ` John Hubbard
2017-11-04  8:32             ` [PATCH v3] printk: Add console owner and waiter logic to loadbalance " Tetsuo Handa
2017-11-04  8:32               ` Tetsuo Handa
2017-11-04  8:43               ` Tetsuo Handa
2017-11-04  8:43                 ` Tetsuo Handa
2017-11-06 12:06 ` [PATCH v3] printk: Add console owner and waiter logic to load balance " Tetsuo Handa
2017-11-06 12:06   ` Tetsuo Handa
2017-11-07  1:40   ` Sergey Senozhatsky
2017-11-07  1:40     ` Sergey Senozhatsky
2017-11-07 11:05     ` [PATCH v3] printk: Add console owner and waiter logic to loadbalance " Tetsuo Handa
2017-11-07 11:05       ` Tetsuo Handa
2017-11-08  5:19     ` [PATCH v3] printk: Add console owner and waiter logic to load balance " Sergey Senozhatsky
2017-11-08  5:19       ` Sergey Senozhatsky
2017-11-08 14:29       ` Steven Rostedt
2017-11-08 14:29         ` Steven Rostedt
2017-11-09  0:56         ` Sergey Senozhatsky
2017-11-09  0:56           ` Sergey Senozhatsky
2017-11-09  3:29           ` Steven Rostedt
2017-11-09  3:29             ` Steven Rostedt
2017-11-09  4:45             ` Sergey Senozhatsky
2017-11-09  4:45               ` Sergey Senozhatsky
2017-11-09  5:06               ` Steven Rostedt
2017-11-09  5:06                 ` Steven Rostedt
2017-11-09  5:33                 ` Sergey Senozhatsky [this message]
2017-11-09  5:33                   ` Sergey Senozhatsky

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=20171109053358.GD775@jagdpanzerIV \
    --to=sergey.senozhatsky.work@gmail.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=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tj@kernel.org \
    --cc=vbabka@suse.cz \
    --cc=xiyou.wangcong@gmail.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: link
Be 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.