Re: [patch 1/5] mm: memcontrol: convert reclaim iterator to simple css refcounting

[Michal Hocko <mhocko@suse.cz>]

On Tue 14-10-14 12:20:33, Johannes Weiner wrote:
> The memcg reclaim iterators use a complicated weak reference scheme to
> prevent pinning cgroups indefinitely in the absence of memory pressure.
> 
> However, during the ongoing cgroup core rework, css lifetime has been
> decoupled such that a pinned css no longer interferes with removal of
> the user-visible cgroup, and all this complexity is now unnecessary.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
>  mm/memcontrol.c | 250 +++++++++++++++++---------------------------------------
>  1 file changed, 76 insertions(+), 174 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index b62972c80055..67dabe8b0aa6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
[...]
> +		do {
> +			pos = ACCESS_ONCE(iter->position);
> +			/*
> +			 * A racing update may change the position and
> +			 * put the last reference, hence css_tryget(),
> +			 * or retry to see the updated position.
> +			 */
> +		} while (pos && !css_tryget(&pos->css));
> +	}
[...]
> +	if (reclaim) {
> +		if (cmpxchg(&iter->position, pos, memcg) == pos && memcg)
> +			css_get(&memcg->css);
> +
> +		if (pos)
> +			css_put(&pos->css);

This looks like a reference leak. css_put pairs with the above
css_tryget but no css_put pairs with css_get for the cached one. We
need:
---