linux-kernel - Re: [PATCH 16/20] sched/idle: Use explicit broadcast oneshot control function

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: <21687949.Wq8byZT4f8@vostro.rjw.lan>
Date:	Tue, 28 Apr 2015 16:14:33 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	"Rafael J. Wysocki" <rafael@...nel.org>,
	Sudeep Holla <sudeep.holla@....com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Linux PM list <linux-pm@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH 16/20] sched/idle: Use explicit broadcast oneshot control function

On Tuesday, April 28, 2015 03:37:44 PM Rafael J. Wysocki wrote:
> On Tuesday, April 28, 2015 03:31:54 PM Rafael J. Wysocki wrote:
> > On Tuesday, April 28, 2015 02:37:10 PM Linus Walleij wrote:
> > > On Tue, Apr 28, 2015 at 2:19 PM, Rafael J. Wysocki <rafael@...nel.org> wrote:
> > > > Sudeep:
> > > >> At-least I observed issue only when I am using hardware broadcast timer.
> > > >> It doesn't hang when I am using hrtimer as broadcast timer in which case
> > > >> one of the cpu will be not enter deeper idle states that lose timer.
> > > >> I will rerun on v4.1-rc1 and post the complete log.
> > > >
> > > > So the bug here is that cpuidle_enter() enables interrupts, so the
> > > > assumption about them being not enabled made by
> > > > tick_broadcast_oneshot_control() is actually not valid.
> > > >
> > > > It looks like we need to acquire the clockevents_lock at least in this
> > > > particular case.  Let me see where to put it and I'll send a patch for
> > > > testing.
> > > 
> > > Aha that looks very much like it. Put me on the patch and I'll
> > > take it for a spin.
> > 
> > OK, so something like the below for starters (the _irqsave variant is used to
> > avoid adding one more WARN_ON(irqs_disabled()) in there).
> > 
> > I haven't tested it, but then I can't reproduce the original issue in the
> > first place.
> 
> Of course, the whole "broadcast" thing could be done from cpuidle_enter()
> in the first place, but then we could not avoid the problem with the cpuidle
> *callback* enabling interrupts possibly in there anyway (not to mention the
> "coupled" stuff).

That said, if the given state is marked with CPUIDLE_FLAG_TIMER_STOP, I really
wouldn't expect it to re-enable interrupts on exit and the "coupled" thing
seems to be fundamentally at odds with that flag either.

So it should be possible to move the "broadcast" logic into the cpuidle layer,
which I'm going to try to do.

Please test the patch I've sent, though, as it should bring the code back to
where it was before the clockevents_notify() removal and it'd be good to verify
that.

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/