From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Tejun Heo <tj@kernel.org>, Jan Kara <jack@suse.com>,
Kyle McMartin <kyle@kernel.org>,
Dave Jones <davej@codemonkey.org.uk>,
Calvin Owens <calvinowens@fb.com>,
linux-kernel@vger.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers
Date: Thu, 21 Jan 2016 14:51:46 +0900 [thread overview]
Message-ID: <20160121055146.GA398@swordfish> (raw)
In-Reply-To: <20160121012551.GA594@swordfish>
On (01/21/16 10:25), Sergey Senozhatsky wrote:
[..]
> > First, the message "This stops the holder of console_sem just where we
> > want him" is suspitious.
>
> this comment is irrelevant, as of today. it was, a long time ago, because
> the entire thing was a bit different (linux-2.4.21 kernel/printk.c)
>
> /* This stops the holder of console_sem just where we want him */
> spin_lock_irqsave(&logbuf_lock, flags);
>
> logbuf_lock does stop the holder, local_irq_save() does not, you are right.
I meant 'irrelevant on its current place'.
[..]
> > As a result, I think that we do not need the extra checks
> > for the save context in printk(). IMHO, it is safe to remove
> > all the console_may_schedule stuff and also remove the extra
> > preempt_disable/preempt_enable() in vprintk_emit().
> >
> > Or did I miss anything?
>
> hm... I suspect the reason we have console_may_schedule is
> console_conditional_schedule() - console_sem owner may want
> to have an internal logic to re-schedule [fwiw], while still
> holding the console_sem. tty/vt/vt.c or video/console/fbcon.c
> for example. (in 2.4 kernel: video/fbcon.c and char/console.c).
>
> cond_resched() helps in console_unlock(); console_conditional_schedule()
> is called after console_lock() and _before_ console_unlock()....
for CONFIG_PREEMPT_COUNT kernel we can do something like
+void __sched console_conditional_schedule(void)
+{
+ if (!oops_in_progress && preemptible() && !rcu_preempt_depth())
+ cond_resched();
+}
and in console_unlock()
- if (do_cond_resched)
- cond_resched();
+ console_conditional_schedule();
but for !CONFIG_PREEMPT_COUNT we can't. because of currently held spin_locks/etc
that we don't know about.
`console_may_schedule' carries a bit of important information for
console_conditional_schedule() caller. if it has acquired console_sem
via console_lock() - then it can schedule, if via console_trylock() - it cannot.
the last `if via console_trylock() - it cannot' rule is not always true,
we clearly can have printk()->console_unlock() from non-atomic contexts
(if we know that its non-atomic, which is not the case with !PREEMPT_COUNT).
-ss
next prev parent reply other threads:[~2016-01-21 5:50 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-14 4:57 [RFC][PATCH -next 0/2] cond_resched() some of console_trylock callers Sergey Senozhatsky
2016-01-14 4:57 ` [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk Sergey Senozhatsky
2016-01-14 5:59 ` [PATCH " Sergey Senozhatsky
2016-01-18 15:42 ` [RFC][PATCH -next " Petr Mladek
2016-01-19 0:42 ` Sergey Senozhatsky
2016-01-19 13:31 ` Petr Mladek
2016-01-19 15:00 ` Sergey Senozhatsky
2016-01-19 16:16 ` Petr Mladek
2016-01-20 4:18 ` Sergey Senozhatsky
2016-01-20 10:09 ` Petr Mladek
2016-01-14 4:57 ` [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers Sergey Senozhatsky
2016-01-17 14:11 ` Sergey Senozhatsky
2016-01-17 14:23 ` Sergey Senozhatsky
2016-01-18 16:17 ` Petr Mladek
2016-01-19 1:15 ` Sergey Senozhatsky
2016-01-19 15:18 ` Petr Mladek
2016-01-20 3:50 ` Sergey Senozhatsky
2016-01-20 11:51 ` Sergey Senozhatsky
2016-01-20 12:38 ` Petr Mladek
2016-01-20 12:31 ` Petr Mladek
2016-01-21 1:25 ` Sergey Senozhatsky
2016-01-21 5:51 ` Sergey Senozhatsky [this message]
2016-01-22 9:48 ` Petr Mladek
2016-01-23 4:46 ` 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=20160121055146.GA398@swordfish \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=calvinowens@fb.com \
--cc=davej@codemonkey.org.uk \
--cc=jack@suse.com \
--cc=kyle@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--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.