Re: [BUGFIX][PATCH -mm] fix bad call of memcg_oom_recover at cancelmove.

From: Balbir Singh
Date: Thu Jun 17 2010 - 23:15:55 EST


* KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> [2010-06-18 11:17:35]:

> On Fri, 18 Jun 2010 10:57:41 +0900
> Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx> wrote:
>
> > > May I recommend the following change instead
> > >
> > >
> > > Don't crash on a null memcg being passed, check if memcg
> > > is NULL and handle the condition gracefully
> > >
> > > Signed-off-by: Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx>
> > > ---
> > > mm/memcontrol.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > index c6ece0a..d71c488 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> > > @@ -1370,7 +1370,7 @@ static void memcg_wakeup_oom(struct mem_cgroup *mem)
> > >
> > > static void memcg_oom_recover(struct mem_cgroup *mem)
> > > {
> > > - if (mem->oom_kill_disable && atomic_read(&mem->oom_lock))
> > > + if (mem && mem->oom_kill_disable && atomic_read(&mem->oom_lock))
> > > memcg_wakeup_oom(mem);
> > > }
> > >
> > I agree to this fix.
> >
> > Acked-by: Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx>
> >
>
> I tend to dislike band-aid in callee. but it's not important here.
>
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
>

The reason is just to make the reading easier

if (cond)
func(cond)

if (cond2)
func(cond2)

It is easier to read

func(cond)
...
func(cond2)

Provided it is valid for us to test the condition inside func()

This way new callers don't have to worry about using func(). This is
very much like how the free calls work today, they can tolerate a NULL
argument and return gracefully.


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