All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Byungchul Park <byungchul.park@lge.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jan Kara <jack@suse.com>, Petr Mladek <pmladek@suse.com>,
	Tejun Heo <tj@kernel.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	linux-kernel@vger.kernel.org, Jan Kara <jack@suse.cz>
Subject: Re: [RFC][PATCH v5 1/2] printk: Make printk() completely async
Date: Mon, 21 Mar 2016 18:28:48 +0900	[thread overview]
Message-ID: <20160321092848.GB504@swordfish> (raw)
In-Reply-To: <20160321084743.GB2279@X58A-UD3R>

On (03/21/16 17:47), Byungchul Park wrote:
[..]
> > Is there any reason why you don't put the wake_up_process() out of the
> > critical section with my suggestion, even though it can solve the infinite
> > recuresion you worried about?
> 
> Just to be sure, whether you take my suggestion or not is not important.
> I just suggested it in this thread since it can solve what you worried
> about. That's all. I can post it in another thread though. Why don't you
> consider it so that yours don't miss any printk message? Do you think there
> are any problems in my suggestion?

we have 2 spin locks in vprintk_emit() -- logbuf_lock and sem->lock. and N
CPUs can concurrently lockup on those two locks, which already makes a
single static pointer in spiun_dump() questionable.

logbug_lock *theoretically* can detect and handle recursive printk()s,
there is no way to catch sem->lock spin_dump() at the moment (but that's
not the point).

there are 2 new spin locks in vprintk_emit() -- p->pi_lock and rq->lock.
what I want is to put those locks inside the "we can detect recursion 100%"
region. so these two locks will not add any new possibilities of recursive
printks, they are covered by the existing recursion detection code thanks
to logbuf lock and static logbuf_cpu. so we still can say that we have 5
places where printk recursion can happen

-- lock + unlock logbuf_lock
   printk() recursion detection code can't help here

-- inside of logbuf_lock critical section
   printk() recursion detection code works here

-- lock + unlock sem->lock
   printk() recursion detection code can't help here


note how "inside of logbuf_lock critical section" takes care of nested
'lock + unlock p->pi_lock' and 'lock + unlock rq->lock'.

moreover, printk() will switch to synchronous mode in recursion handler and
two misbehaving spin locks (4 places where recursion can happen) will not be
executed anymore.


what you want to have -- 4 independent spin locks and 9 places where
recursion can happen, only 1 of which is covered by printk recursion code.

-- lock + unlock logbuf_lock
   printk() recursion detection code can't help here

-- inside of logbuf_lock critical section
   printk() recursion detection code works here

-- lock + unlock p->pi_lock
   printk() recursion detection code can't help here

-- lock + unlock rq->lock
   printk() recursion detection code can't help here

-- lock + unlock sem->lock
   printk() recursion detection code can't help here

and there is a static pointer to fix everything up? what if 2
CPUs will simultaneously printk-recurse in 2 different places?
why this is better?

	-ss

  reply	other threads:[~2016-03-21  9:29 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-20 14:13 [RFC][PATCH v5 0/2] printk: Make printk() completely async Sergey Senozhatsky
2016-03-20 14:13 ` [RFC][PATCH v5 1/2] " Sergey Senozhatsky
2016-03-21  0:06   ` Byungchul Park
2016-03-21  0:43     ` Sergey Senozhatsky
2016-03-21  0:56       ` Byungchul Park
2016-03-21  7:35         ` Sergey Senozhatsky
2016-03-21  8:07           ` Byungchul Park
2016-03-21  8:47             ` Byungchul Park
2016-03-21  9:28               ` Sergey Senozhatsky [this message]
2016-03-21 14:32                 ` Jan Kara
2016-03-21 14:58                   ` Sergey Senozhatsky
2016-03-21 15:33                     ` Jan Kara
2016-03-21 17:11                       ` Sergey Senozhatsky
2016-03-22  2:18                         ` Byungchul Park
2016-03-22  2:13                 ` Byungchul Park
2016-03-22  5:52                   ` Sergey Senozhatsky
2016-03-22  6:57                     ` Byungchul Park
2016-03-22  7:43                       ` Sergey Senozhatsky
2016-03-21  8:51             ` Sergey Senozhatsky
2016-03-20 14:13 ` [RFC][PATCH v5 2/2] printk: Make wake_up_klogd_work_func() async 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=20160321092848.GB504@swordfish \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=byungchul.park@lge.com \
    --cc=jack@suse.com \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=pmladek@suse.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tj@kernel.org \
    /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.