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

[Jan Kara]

On Sun 06-03-16 22:27:03, Sergey Senozhatsky wrote:
> On (03/06/16 20:06), Tetsuo Handa wrote:
> [..]
> > > do you mean a new worker allocation delay and a MAYDAY timer delay?
> > > 
> > 
> > I don't know what MAYDAY is. I'm talking about a situation where printing_work
> > work item is not processed (i.e. printing_work_func() is not called) until
> > current work item calls schedule_timeout_*().
> > 
> > We had a problem that since vmstat_work work item was using system_wq,
> > vmstat_work work item was not processed (i.e. vmstat_update() was not called) if
> > kworker was looping inside memory allocator without calling schedule_timeout_*()
> > due to disk_events_workfn() doing GFP_NOIO allocation).
> 
> hm, just for note, none of system-wide wqs seem to have a ->rescuer thread
> (WQ_MEM_RECLAIM).
> 
> [..]
> > Even if you use printk_wq with WQ_MEM_RECLAIM for printing_work work item,
> > printing_work_func() will not be called until current work item calls
> > schedule_timeout_*(). That will be an undesirable random delay. If you use
> > a dedicated kernel thread rather than a dedicated workqueue with WQ_MEM_RECLAIM,
> > we can avoid this random delay.
> 
> hm. yes, seems that it may take some time until workqueue wakeup() a ->rescuer thread.
> need to look more.

Yes, it takes some time (0.1s or 2 jiffies) before workqueue code gives up
creating a worker process and wakes up rescuer thread. However I don't see
that as a problem...

> > > 	console_lock();
> > > 	for (...) {
> > > 		do_foo() {
> > > 			...
> > > 				pr_err(" ... foo message ...\n");
> > > 			...
> > > 		}
> > > 	}
> > > 	console_unlock();
> > > 
> > > then yes, nothing will be printed until that process executes console_unlock(),
> > > because it's console_unlock() that pushes the messages to console drivers.
> > 
> > Yes, I meant a sequence like
> > 
> >   console_lock();
> >   ptr = kmalloc(GFP_KERNEL);
> >   kfree(ptr);
> >   console_unlock();
> > 
> > and kmalloc() prints OOM killer messages rather than failing that allocation.
> > Are we sure that there is no such usage?
> 
> such usage is quite possible.
> 
> problems that I have with console_lock()/console_unlock() is that
> these functions serve a double purpose: exclusive printk() lock and a
> console_drivers list lock.

Well, but changing how console locking works is a separate issue, isn't it?
So please as a separate patch set if you want to try it.

Actually I don't think changing the locking will be so easy.
console_lock/unlock is used e.g. for console blanking where you need to
block any printing while you call ->unblank() for each console. That being
said I don't think improvement is impossible, just given my experiences
with console / printk code there will be surprises waiting for you :).

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR