[Intel-gfx] [PATCH] [RFC] kernel/cpu: Use lockref for online CPU reference counting
Joonas Lahtinen
joonas.lahtinen at linux.intel.com
Tue Feb 16 10:51:03 UTC 2016
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
More information about the Intel-gfx
mailing list