At first, thank you for review.
On Wed, 31 Oct 2007 15:12:34 -0700
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> > +static inline void __mem_cgroup_stat_add(struct mem_cgroup_stat *stat,
> > + enum mem_cgroup_stat_index idx, int val)
> > +{
> > + int cpu = smp_processor_id();
> > + preempt_disable();
> > + stat->cpustat[cpu].count[idx] += val;
> > + preempt_enable();
> > +}
>
> This is clearly doing smp_processor_id() in preemptible code. (or the
> preempt_disable() just isn't needed). I fixed it up.
>
Thanks,
> Please ensure that you test with all runtime debugging options enabled -
> you should have seen a warning here.
>
Sorry, I'll take care.
> > +/*
> > + * For accounting under irq disable, no need for increment preempt count.
> > + */
> > +static inline void __mem_cgroup_stat_add_safe(struct mem_cgroup_stat *stat,
> > + enum mem_cgroup_stat_index idx, int val)
> > +{
> > + int cpu = smp_processor_id();
> > + stat->cpustat[cpu].count[idx] += val;
> > +}
>
> There's a wild amount of inlining in that file. Please, just don't do it -
> inline is a highly specialised thing and is rarely needed.
>
> When I removed the obviously-wrong inline statements, the size of
> mm/memcontrol.o went from 3823 bytes down to 3495.
>
> It also caused this:
>
> mm/memcontrol.c:65: warning: '__mem_cgroup_stat_add' defined but not used
>
> so I guess I'll just remove that.
>
ok. I'll add again if it is needed again.
Thanks,
-Kame
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
