Re: [PATCH] mm/vmscan: fix incorrect return type for cgroup_reclaim()

[Baoquan He <bhe@redhat.com>]

On 03/19/20 at 05:07pm, Michal Hocko wrote:
> On Thu 19-03-20 23:50:26, qiwuchen55@gmail.com wrote:
> > From: chenqiwu <chenqiwu@xiaomi.com>
> > 
> > The return type of cgroup_reclaim() is bool, but the correct type
> > should be struct mem_cgroup pointer. As a result, cgroup_reclaim()
> > can be used to wrap sc->target_mem_cgroup in vmscan code.
> 
> Why is this an improvement? While we can hide the scan_control
> dereference I am not seeing why this would be so much better.
> Quite worse TBH because cgroup_reclaim is a predicate while you make it
> return an object. This might be highly subjective thing, though.
> 
> Does the patch result in a better code generation at least. Because
> without any obvious improvement I am not sure this is worth inclusion.

I tend to agree with Michal. The returned bool looks good.

If you really care about the internal conversion, maybe a slight change
as below?

static struct mem_cgroup *cgroup_reclaim(struct scan_control *sc)
{
	return !!sc->target_mem_cgroup;
}