linux-kernel - Re: [PATCH v4] printk: Add console owner and waiter logic to load balance console writes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171128062326.GA1907@jagdpanzerIV>
Date:   Tue, 28 Nov 2017 15:23:26 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Byungchul Park <byungchul.park@....com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        LKML <linux-kernel@...r.kernel.org>, akpm@...ux-foundation.org,
        linux-mm@...ck.org, Cong Wang <xiyou.wangcong@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...nel.org>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        yuwang.yuwang@...babab-inc.com,
        Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Jan Kara <jack@...e.cz>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        rostedt@...e.goodmis.org, kernel-team@....com
Subject: Re: [PATCH v4] printk: Add console owner and waiter logic to load
 balance console writes

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