2013/8/15 Peter Zijlstra <pet...@infradead.org>:
> On Tue, Aug 06, 2013 at 05:36:43PM +0900, Joonsoo Kim wrote:
>> There is no reason to maintain separate variables for this_group
>> and busiest_group in sd_lb_stat, except saving some space.
>> But this structure is always allocated in stack, so this saving
>> isn't really benificial.
>>
>> This patch unify these variables, so IMO, readability may be improved.
>>
>> Signed-off-by: Joonsoo Kim <iamjoonsoo....@lge.com>
>
> So I like the idea I had to reformat some of the code and I think we can
> do less memsets. See how the below reduces the sds memset by the two
> sgs. If we never set busiest we'll never look at sds->busiest_stat so we
> don't need to clear that. And if we make the sgs memset in
> update_sd_lb_stats() unconditional we'll cover sds->this_stats while
> reducing complexity.
At first glance, below changes look good.
When I return to the office on next Monday, I will look at it in detail.
Just one comment below.
> @@ -4890,13 +4893,12 @@ static inline void calculate_imbalance(s
> * return the least loaded group whose CPUs can be
> * put to idle by rebalancing its tasks onto our group.
> */
> -static struct sched_group *
> -find_busiest_group(struct lb_env *env)
> +static struct sched_group *find_busiest_group(struct lb_env *env)
> {
> - struct sd_lb_stats sds;
> struct sg_lb_stats *this, *busiest;
> + struct sd_lb_stats sds;
>
> - memset(&sds, 0, sizeof(sds));
> + memset(&sds, 0, sizeof(sds) - 2*sizeof(struct sg_lb_stats));
How about using offsetof() macro, instead of using subtraction to
calculate size?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
