Re: Possible regression with cgroups in 3.11

[Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

Hello, Michal.

On Tue, Nov 12, 2013 at 03:58:24PM +0100, Michal Hocko wrote:
> Hmm, there is CSS_ONLINE and it has a different meaning that we would
> need.
> css_alive() would be a better fit. Something like the following:
> 
> static inline bool percpu_ref_alive(struct percpu_ref *ref)
> {
> 	int ret = false;
> 	
> 	rcu_lockdep_assert(rcu_read_lock_held(), "percpu_ref_alive needs an RCU lock");
> 	return REF_STATUS(pcpu_count) == PCPU_REF_PTR;
> }

I'd really like to avoid exposing percpu-ref locking details to its
users.  e.g. percpu_ref switched from normal RCU to sched RCU
recently.

> static inline bool css_alive(struct cgroup_subsys_state *css)
> {
> 	return percpu_ref_alive(&css->refcnt);
> }
> 
> and for our use we would have something like the following in
> __mem_cgroup_try_charge_swapin:
> 
> 	memcg = try_get_mem_cgroup_from_page
> 	[...]
> 	__mem_cgroup_try_charge(...);
> 
> 	/*
>          * try_get_mem_cgroup_from_page and the actual charge are not
>          * done in the same RCU read section which means that the memcg
>          * might get offlined before res counter is charged if the
>          * current charger is not a memcg memeber so we have to recheck
>          * the memcg's css status again here and revert that charge as
>          * we cannot be sure it was accounted properly.
> 	 */
> 	if (!ret) {
> 		rcu_read_lock();
> 		if (!css_alive(&memcg->css)) {
> 			__mem_cgroup_cancel_charge(memcg, 1);
> 			rcu_read_unlock();
> 			css_put(&memcg->css);
> 			*memcgp = NULL;
> 			goto charge_cur_mm;
> 		}
> 		rcu_read_unlock();
> 	}

Without going into memcg details, the general cgroup policy now is to
make each controller responsible for its own synchronization so that
we don't end up entangling synchronization schemes of different
controllers.  cgroup core invokes the appropriate callback on each
state transition and provides certain guarantees so that controllers
can implement proper synchronization from those callbacks.  During
shutdown, ->css_offline() is where a css transits from life to death
and memcg should be able to implement proper synchronization from
there if necessary.

Thanks.

-- 
tejun