Hi, On (11/27/17 17:48), Byungchul Park wrote: [..] > > + /* Owner will clear console_waiter on hand off > > */ > > + while (READ_ONCE(console_waiter)) > > + cpu_relax(); > > + > > + spin_release(&console_owner_dep_map, 1, > > _THIS_IP_); > > + printk_safe_exit_irqrestore(flags); > > + > > + /* > > + * The owner passed the console lock to us. > > + * Since we did not spin on console lock, > > annotate > > + * this as a trylock. Otherwise lockdep will > > + * complain. > > + */ > > + mutex_acquire(&console_lock_dep_map, 0, 1, > > _THIS_IP_); > > I'm afraid if it's ok even not to lock(or trylock) actually here. Is there > any problem if you call console_trylock() instead of mutex_acquire() here?
console_trylock() will not work. console_trylock() implies that the current printing process does up(), which a) opens a race with possible console_lock()/console_trylock() from foreign CPUs, and b) additionally wakes up a task from console_sem wait list [if there was one]. so chances are some other CPU potentially can acquire the lock ahead of waiter, forcing the waiter to continue spinning on console_sem. and the bad news are a) it's spinning with local IRQs disabled and b) that another CPU, which has acquired the console_sem, can schedule under console_sem. anyway, we are not going to merge this patch. we already have discussed that in V3 thread: https://marc.info/?l=linux-kernel&m=151019815721161&w=2 https://marc.info/?l=linux-kernel&m=151020275921953&w=2 https://marc.info/?l=linux-kernel&m=151020404622181&w=2 https://marc.info/?l=linux-kernel&m=151020565222469&w=2 I took some parts of the Steven's patch set, tho, and backported them to the current printk_kthread series. -ss
