All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: sergey.senozhatsky.work@gmail.com, akpm@linux-foundation.org
Cc: jack@suse.com, pmladek@suse.com, tj@kernel.org,
	linux-kernel@vger.kernel.org, sergey.senozhatsky@gmail.com,
	jack@suse.cz
Subject: Re: [RFC][PATCH v2 1/2] printk: Make printk() completely async
Date: Sun, 6 Mar 2016 16:18:29 +0900	[thread overview]
Message-ID: <201603061618.GED43232.MtOQOFSLOFHJFV@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20160306063251.GA493@swordfish>

Sergey Senozhatsky wrote:
> printk() is expected to work under different conditions and in different
> scenarios, including corner cases of OOM when all of the workers are busy
> (e.g. allocating memory). Thus by default printk() uses its own dedicated
> workqueue with WQ_MEM_RECLAIM bit set. It falls back to system_long_wq
> (console_unlock() is time unbound) only if it has failed to allocate
> printk_wq. Another thing to mention, is that deferred printk() messages
> may appear before printk_wq is allocated, so in the very beginning we
> have to printk deferred messages the old way -- in IRQ context.

I think we should not depend on system_long_wq which does not have
WQ_MEM_RECLAIM bit. If workqueue allocation upon boot fails (due to ENOMEM),
such systems won't be able to start userspace programs.

Moreover, I don't like use of a workqueue even if printk_wq was allocated
with WQ_MEM_RECLAIM bit. As you can see in the discussion of the OOM reaper,
the OOM reaper chose a dedicated kernel thread rather than a workqueue
( http://lkml.kernel.org/r/1454505240-23446-2-git-send-email-mhocko@kernel.org ).

Blocking actual printing until ongoing workqueue item calls schedule_timeout_*()
is not nice (see commit 373ccbe59270 and 564e81a57f97). Use of WQ_MEM_RECLAIM
means we add a task_struct for that workqueue. Thus, using a kernel thread does
not change total number of task_struct compared to WQ_MEM_RECLAIM approach.
I think actual printing should occur as soon as possible rather than randomly
deferred until workqueue item sleeps.

> +static void printing_work_func(struct work_struct *work)
> +{
> +	console_lock();
> +	console_unlock();
> +}

Is this safe? If somebody invokes the OOM killer between console_lock()
and console_unlock(), won't this cause OOM killer messages not printed?

  reply	other threads:[~2016-03-06  7:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-05 10:55 [RFC][PATCH v2 0/2] printk: Make printk() completely async Sergey Senozhatsky
2016-03-05 10:55 ` [RFC][PATCH v2 1/2] " Sergey Senozhatsky
2016-03-06  6:32   ` Sergey Senozhatsky
2016-03-06  7:18     ` Tetsuo Handa [this message]
2016-03-06  9:35       ` Sergey Senozhatsky
2016-03-06 11:06         ` Tetsuo Handa
2016-03-06 13:27           ` Sergey Senozhatsky
2016-03-06 14:54             ` Tetsuo Handa
2016-03-07  8:22             ` Jan Kara
2016-03-07 10:12               ` Sergey Senozhatsky
2016-03-07 10:52                 ` Jan Kara
2016-03-07 12:16                   ` Jan Kara
2016-03-07 12:37                     ` Tetsuo Handa
2016-03-07 15:10                     ` Sergey Senozhatsky
2016-03-07 15:49                       ` Tejun Heo
2016-03-08 10:21                         ` Sergey Senozhatsky
2016-03-11 17:22                           ` Tejun Heo
2016-03-12  5:01                             ` Sergey Senozhatsky
2016-03-09  6:09                     ` Sergey Senozhatsky
2016-03-10  9:27                       ` Jan Kara
2016-03-10 15:48                         ` Sergey Senozhatsky
2016-03-10  9:53                       ` Petr Mladek
2016-03-10 16:26                         ` Sergey Senozhatsky
2016-03-07 14:40                   ` Sergey Senozhatsky
2016-03-07 11:10                 ` Tetsuo Handa
2016-03-07 14:36                   ` Sergey Senozhatsky
2016-03-07 15:42               ` Tejun Heo
2016-03-05 10:55 ` [RFC][PATCH v2 2/2] printk: Skip messages on oops 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=201603061618.GED43232.MtOQOFSLOFHJFV@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=akpm@linux-foundation.org \
    --cc=jack@suse.com \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=sergey.senozhatsky.work@gmail.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.