linux-kernel - Re: Warning from swake_up_all in 4.14.15-rt13 non-RT

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:   Fri, 9 Mar 2018 23:26:43 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     Corey Minyard <cminyard@...sta.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-rt-users <linux-rt-users@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Tejun Heo <tj@...nel.org>
Subject: Re: Warning from swake_up_all in 4.14.15-rt13 non-RT

On Fri, Mar 09, 2018 at 09:25:50PM +0100, Sebastian Andrzej Siewior wrote:
> Is it just about the irqsave() usage or something else? I doubt it is
> the list walk. It is still unbound if not called from irq-off region.

The current list walk is preemptible. You put the entire iteration (of
unbound length) inside a single critical section which destroy RT.

> But it is now possible, I agree. The wake_q usage should be cheaper
> compared to IRQ off+on in each loop. And we wanted to do the wake ups
> with enabled interrupts - there is still the list_splice() from that
> attempt. Now it can be.

Unbound is still unbound, inf/n := inf. A 'cheaper' unbound doesn't RT
make.

> > Yes, wake_up_all() is crap, it is also fundamentally incompatible with
> > in-*irq usage. Nothing to be done about that.
> I still have (or need) completions which are swait based and do
> complete_all(). 

That's fine, as long as they're done from preemptible context. Back when
we introduced swait this was an explicit design goal/limitation. And
there were no in-irq users of this.

> There are complete_all() caller which wake more than one
> waiter (that is PM and crypto from the reports I got once I added the
> WARN_ON())).
> The in-IRQ usage is !RT only and was there before.

Then that's broken and needs to be undone. Also, why did you need the
WARN, lockdep should've equally triggered on this, no?

> > So NAK on this.
> So I need completions to be swait based and do complete_all() from IRQ
> (on !RT, not RT). I have this one call which breaks the usage on !RT and
> has wake_up_all() in it in vanilla which needs an swait equivalent since
> it calls its callback from an rcu-sched section.

Why isn't this a problem on RT?