linux-kernel - Re: [PATCH 4/6] sched,idle: Clear polling before descheduling the idle thread

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] [day] [month] [year] [list]

Message-ID: <20140604160351.GP30445@twins.programming.kicks-ass.net>
Date:	Wed, 4 Jun 2014 18:03:51 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Mike Galbraith <umgwanakikbuti@...il.com>,
	Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>, nicolas.pitre@...aro.org,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/6] sched,idle: Clear polling before descheduling the
 idle thread

On Wed, Jun 04, 2014 at 07:57:07AM -0700, Andy Lutomirski wrote:
> On Wed, Jun 4, 2014 at 12:44 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> > On Tue, Jun 03, 2014 at 05:29:50PM -0700, Andy Lutomirski wrote:
> >> Currently, the only real guarantee provided by the polling bit is
> >> that, if you hold rq->lock and the polling bit is set, then you can
> >> set need_resched to force a reschedule.
> >>
> >> The only reason the lock is needed is that the idle thread might not
> >> be running at all when setting its need_resched bit, and rq->lock
> >> keeps it pinned.
> >>
> >> This is easy to fix: just clear the polling bit before scheduling.
> >> Now the polling bit is only ever set when rq->curr == rq->idle.
> >
> > Yah, except of course:
> >
> >   lkml.kernel.org/r/20131120162736.508462614@...radead.org
> 
> Wow, that code was ugly.
> 
> Can you be persuaded to hold off on that patch until after this series
> is done?  I think the cpu_startup_entry change will just muddy the
> waters for now.

Sure, no problem with that.

> > which I really need to rebase and post again :/
> >
> > In any case, this is useful even with that, although then we really must
> > do something like:
> >
> >   rcu_read_lock();
> >   if (!set_nr_if_polling(rq->curr))
> >         smp_send_reschedule(rq->cpu);
> >   rcu_read_unlock();
> >
> > Because there's other tasks than rq->idle which might be 'idle', joy!
> 
> Is this really a problem?
> 
> It's certainly the case that a non-idle (in the sense of != rq->idle)
> task can have the polling bit set, but so what?  I'm perfectly happy
> to wake these ersatz idle things via IPI instead of via
> TIF_NEED_RESCHED.  I think that all of the code that plays with the
> polling bit after all these patches are applied either holds rq->lock
> (which prevents the task from going away) or acts directly on
> rq->idle, which really has no business being deallocated.
> 
> I think that the RCU solution is actually racy, too.  Consider:

Indeed, ok so since I really hate the idle injection crap anyhow, I
don't particularly care if we send them extra IPIs if that means we get
extra pain for the sane case.

So I'll drop that part of the rebased patch and we'll focus on rq->idle.



Content of type "application/pgp-signature" skipped