lkml.org 
[lkml]   [2015]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/9] x86/intel_cqm: Modify hot cpu notification handling


On Wed, 29 Jul 2015, Peter Zijlstra wrote:

> On Wed, Jul 01, 2015 at 03:21:02PM -0700, Vikas Shivappa wrote:
>> +/*
>> + * Temporary cpumask used during hot cpu notificaiton handling. The usage
>> + * is serialized by hot cpu locks.
>> + */
>> +static cpumask_t tmp_cpumask;
>
> So the problem with this is that its 512 bytes on your general distro
> config. And this patch set includes at least 3 of them
>
> So you've just shot 1k5 bytes of .data for no reason.
>
> I know tglx whacked you over the head for this, but is this really worth
> it? I mean, nobody sane should care about hotplug performance, so who
> cares if we iterate a bunch of cpus on the abysmal slow path called
> hotplug.

We did this so that we dont keep looping on every cpu to check if it
belongs to a particular package. especially the cost being linear with more and
more cpus getting added and on large systems.
Would it not make sense to use the mask which would tell you all the
cores on a particular core's package ? I realized to use the mask
topology_core_cpumask only after seeing tglx's pseudo code because the name is
definitely confusing and earlier I assumed such mask doesnt exist and hence we
had to just loop through.
I know you pointed out to not put the mask on the stack , but the static usage
cost should be reasonable to avoid the cost of looping through all the
available cpus..

also it doesnt mean we put more crap when we see crapy code as per tglx as well
? so why contradict that.

>


\
 
 \ /
  Last update: 2015-08-01 01:41    [W:0.117 / U:1.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site