Messages in this thread | | | Date | Thu, 22 Dec 2016 14:31:19 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [PATCHv6 6/7] printk: use printk_safe buffers in printk |
| |
Hello,
On (12/21/16 23:36), Sergey Senozhatsky wrote: > Use printk_safe per-CPU buffers in printk recursion-prone blocks: > -- around logbuf_lock protected sections in vprintk_emit() and > console_unlock() > -- around down_trylock_console_sem() and up_console_sem() > > Note that this solution addresses deadlocks caused by printk() > recursive calls only. That is vprintk_emit() and console_unlock().
several questions.
so my plan was to introduce printk-safe and to switch vprintk_emit() and console_sem related functions (like console_unlock(), etc.) to printk-safe first. and switch the remaining logbuf_lock users, like devkmsg_open()/syslog_print()/etc, in a followup, pretty much mechanical "find logbuf_lock - add printk_safe", patch. but that followup patch is bigger than I expected (still mechanical tho); so I want to re-group.
there are 9 raw_spin_lock_irq(&logbuf_lock) 7 raw_spin_lock_irqsave(&logbuf_lock, flags) and 12 raw_spin_lock_irq(&logbuf_lock) 8 raw_spin_unlock_irqrestore(&logbuf_lock, flags)
wrapping each one of them in printk_safe_enter()/printk_safe_enter_irq() and printk_safe_exit()/printk_safe_exit_irq() is a bit boring. so I have several options: one of them is to add printk_safe_{enter,exit}_irq() and, along with it, a bunch of help macros (to printk.c):
(questions below)
/* * Helper macros to lock/unlock logbuf_lock in deadlock safe * manner (logbuf_lock may spin_dump() in lock/unlock). */ #define lock_logbuf(flags) \ do { \ printk_safe_enter(flags); \ raw_spin_lock(&logbuf_lock); \ } while (0)
#define unlock_logbuf(flags) \ do { \ raw_spin_unlock(&logbuf_lock); \ printk_safe_exit(flags); \ } while (0)
#define lock_logbuf_irq() \ do { \ printk_safe_enter_irq(); \ raw_spin_lock(&logbuf_lock); \ } while (0)
#define unlock_logbuf_irq() \ do { \ raw_spin_unlock(&logbuf_lock); \ printk_safe_exit_irq(); \ } while (0)
so this
printk_safe_enter_irq(); raw_spin_lock(&logbuf_lock); ... raw_spin_unlock(&logbuf_lock); printk_safe_exit(flags);
or this
printk_safe_enter_irq(); raw_spin_lock(&logbuf_lock); ... raw_spin_unlock(&logbuf_lock); printk_safe_exit_irq();
becomes this
lock_logbuf(flags); ... unlock_logbuf(flags);
and this
lock_logbuf_irq(); ... unlock_logbuf_irq();
questions:
-- the approach another solution? switch those raw_spin_{lock,unlock}_irq to irqsave/irqrestore (?) and use the existing printk_safe_enter()/printk_safe_exit(), so *_irq() versions of lock_logbuf/printk_safe macros will not be needed?
-- the naming are lock_logbuf()/unlock_logbuf() and lock_logbuf_irq()/unlock_logbuf_irq() good enough? (if good at all)
-ss
| |