lkml.org 
[lkml]   [2016]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] [RFC] kernel/cpu: Use lockref for online CPU reference counting
From
Date
On ti, 2016-02-16 at 10:14 +0100, Peter Zijlstra wrote:
> On Tue, Feb 16, 2016 at 10:49:36AM +0200, Joonas Lahtinen wrote:
> > I originally thought of implementing this more similar to what you
> > specify, but then I came across a discussion in the mailing list where
> > it was NAKed adding more members to task_struct;
> >
> > http://comments.gmane.org/gmane.linux.kernel/970273
> >
> > Adding proper recursion (the way my initial implementation was going)
> > got ugly without modifying task_struct because get_online_cpus() is a
> > speed critical code path.
>
> Yeah, just don't let Linus hear you say that. get_online_cpus() is _not_
> considered performance critical.

Oh well, at least changes to it added quite noticeably to the bootup
time of a system.

>
> > So I'm all for fixing the current code in a different way if that will
> > then be merged.
>
> So I'm not sure why you're poking at this horror show to begin with.
> ISTR you mentioning a lockdep splat for SKL, but failed to provide
> detail.
>

Quoting my original patch;

"See the Bugzilla link for more details.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93294"

The improvement my patch implements is to use lockref for locked
reference counting (hotplug code previously rolled its own mutex +
atomic combo), which gets rid of the deadlock scenario described and
linked in the initial patch. Trace for the scenario;

https://bugs.freedesktop.org/attachment.cgi?id=121490

I think using lockref makes it substantially less special, lockref code
being a lot more battle-tested in the FS code than the previous
cpu_hotplug.lock mess.

> Making the hotplug lock _more_ special to fix that is just wrong. Fix
> the retarded locking that lead to it.
>

I do agree that it's still not pretty, but now it does correctly what
the previous code was trying to do with custom mutex + atomic.

I'm all for fixing the code further, but prior to proceeding there
needs to be some sort of an agreement on either making
get_online_cpus() slower (which does not seem like a good idea) or
adding more members to task_struct.

Regards, Joonas

>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

\
 
 \ /
  Last update: 2016-02-16 12:41    [W:0.367 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site