[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5284940D.6050402@linux.vnet.ibm.com>
Date: Thu, 14 Nov 2013 14:42:45 +0530
From: Preeti U Murthy <preeti@...ux.vnet.ibm.com>
To: Peter Zijlstra <peterz@...radead.org>
CC: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
Preeti Murthy <preeti.lkml@...il.com>, mingo@...nel.org,
hpa@...or.com, linux-kernel@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>, mikey@...ling.org,
linux-tip-commits@...r.kernel.org
Subject: Re: [PATCH v2] sched: Check sched_domain before computing group power.
Hi Peter,
On 11/14/2013 02:00 PM, Peter Zijlstra wrote:
> On Thu, Nov 14, 2013 at 11:36:27AM +0530, Preeti U Murthy wrote:
>> However I was thinking that a better fix would be to reorder the way we call
>> update_group_power() and cpu_attach_domain(). Why do we need to do
>> update_group_power() of the groups of the sched domains that would probably
>> degenerate in cpu_attach_domain()? So it seemed best to move update_group_power()
>> to after cpu_attach_domain() so that it saves unnecessary iterations over
>> sched domains which could degenerate, and it fixes the issue that you have brought out
>> as well. See below for the patch:
>
> So how is publishing the domain tree before we've set these values at
> all going to help avoid the divide-by-zero problem?
We are still doing initialization of cpu power and power_orig during
building of sched domains right? Except that it is being done after CPUs
have base domains attached to them.
But if you are talking about the check in sched_debug_one() on if
power_orig has been initialized, then yes, this patch fails. I am sorry
I overlooked the sched_debug() checks in cpu_attach_domain().
>
> Also its just terribly bad form to publish something before you're done
> with initialization.
You are right. cpu_rq(cpu)->sd is going to be used by anyone intending
to iterate through the sched domains. By the time we publish this, every
parameter related to sched domains and groups need to be initialized.
The fact that sched_domain_debug() is going to be called by
cpu_attach_domain() to do one final sanity check on all the parameters
of the sched domains further emphasizes that we cannot have anything
un-initialised at this stage. So clearly my patch is incorrect.
Please add my Reviewed-by to Srikar's patch.
Thanks.
Regards
Preeti U. Murthy
>
--
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/
Powered by blists - more mailing lists