Message-ID: <5187A397.9040102@intel.com>
Date: Mon, 06 May 2013 20:35:35 +0800
From: Alex Shi <alex.shi@intel.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120912 Thunderbird/15.0.1
MIME-Version: 1.0
To: Phil Carmody <pc+lkml@asdf.org>
CC: linux-kernel@vger.kernel.org
Subject: Re: [patch v7 05/21] sched: log the cpu utilization at rq
References: <20130506120318.GM21274@fatphil.org>
In-Reply-To: <20130506120318.GM21274@fatphil.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1111
Lines: 29

On 05/06/2013 08:03 PM, Phil Carmody wrote:
>> > +     period = rq->avg.runnable_avg_period ? rq->avg.runnable_avg_period : 1;
>> > +     rq->util = (u64)(rq->avg.runnable_avg_sum << SCHED_POWER_SHIFT)
>> > +                             / period;
> Greetings, Alex.
> 
> That cast achieves nothing where it is. If the shift has overflowed,
> then you've already lost information; and if it can't overflow, then
> it's not needed at all. 
> 
> It's itsy-bitsy, but note that there exists a div_u64(u64 dividend,
> u32 divisor) helper which may be implemented to be superior to just '/'.
> (And also note that the assignment to ``period'' is a good candidate for
> gcc's ``?:'' operator.)
> 
> If you pull the cast inside the brackets, then you may add my
> Reviewed-by: Phil Carmody <pc+lkml@asdf.org>

You are right, Phil!
Thanks for review! :)

-- 
Thanks
    Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/