Messages in this thread | | | Date | Fri, 23 Dec 2016 10:46:43 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [PATCHv6 6/7] printk: use printk_safe buffers in printk |
| |
On (12/22/16 18:10), Petr Mladek wrote: ... > There are many callers. I think that such wrappers make sense. > I would only like to keep naming scheme similar to the classic > locks. I mean: > > printk_safe_enter_irq() > printk_safe_exit_irq() > > printk_safe_enter_irqsave(flags) > printk_safe_exit_irqrestore(flags)
sure.
> and > > logbuf_lock_irq() > logbuf_unlock_irq() > > logbuf_lock_irqsave(flags) > logbuf_lock_irqrestore(flags)
ok.
> I actually like this change. It makes it clear that the operation > has a side effect (disables/enables irq) which was not visible > from the original name.
agree.
> Well, I wonder how many times we need to call printk_save_enter/exit > standalone (ouside these macros).
not every switch to printk_safe is "dictated" by logbuf_lock. down_trylock_console_sem(), for instance, takes semaphore spin_lock which already may be locked on the same CPU (*), so we need to be in safe mode:
vprintk_emit() down_trylock() raw_spin_lock_irqsave(&sem->lock, flags); ... raw_spin_unlock_irqrestore(&sem->lock, flags); spin_dump() printk() vprintk_emit() down_trylock() raw_spin_lock_irqsave(&sem->lock, flags) << deadlock
and so on. IOW, "printk_save_enter()" != "logbuf_lock is acquired".
> The question is if we really need all the variants of > printk_safe_enter()/exit(). Alternative solution would be > to handle only the printk_context in pritnk_safe_enter() > and make sure that it is called with IRQs disabled. > I mean to define only __printk_safe_enter()/exit() > and do: > > #define logbuf_lock_irqsave(flags) \ > do { \ > local_irq_save(flags) \ > __printk_safe_enter(); \ > raw_spin_lock(&logbuf_lock); \ > } while (0)
won't do the trick for console sem spin_lock.
[..] > PS: I still think if we could come with a better name than > printk_safe() but I cannot find one.
well, not that I'm the fan of printk_safe name, but can't think of anything better. we make printk calls safe (deadlock safe) in places where previously it was unsafe... quick-&-dirty name that is implementation-specific -- printk_percpu_enter/exit, or printk_pcpu_enter/exit... dunno.
-ss
| |