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]
Date:   Mon, 27 Nov 2017 11:49:48 +0000
From:   Will Deacon <will.deacon@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Sebastian Ott <sebott@...ux.vnet.ibm.com>,
        Ingo Molnar <mingo@...nel.org>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [bisected] system hang after boot

Hi Peter,

On Wed, Nov 22, 2017 at 09:22:17PM +0100, Peter Zijlstra wrote:
> On Wed, Nov 22, 2017 at 06:26:59PM +0000, Will Deacon wrote:
> 
> > Now, I can't see what the break_lock is doing here other than causing
> > problems. Is there a good reason for it, or can you just try removing it
> > altogether? Patch below.
> 
> The main use is spin_is_contended(), which in turn ends up used in
> __cond_resched_lock() through spin_needbreak().
> 
> This allows better lock wait times for PREEMPT kernels on platforms
> where the lock implementation itself cannot provide 'contended' state.
> 
> In that capacity the write-write race shouldn't be a problem though.

I'm not sure why it isn't a problem: given that the break_lock variable
can read as 1 for a lock that is no longer contended and 0 for a lock that
is currently contended, then the __cond_resched_lock is likely to see a
value of 0 (i.e. spin_needbreak always return false) more often than no
since it's checked by the lock holder.

> That said, I'd not be horribly sad to see this go, I've always found it
> to be quite the ugly hack and taking it out should provide some
> incentive for better lock implementations for the archs relying on this.

Right, and they can always implement arch_spin_is_contended if they have
a good way to do it.

I'll post this diff as a full patch, since it's clearly needed to get some
s390 systems booting against with 4.15.

Will

Powered by blists - more mailing lists