Re: [RFC][PATCH v2 1/2] printk: Make printk() completely async

[Tetsuo Handa]

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@xxxxxxxxxx ).

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?