Commit 619d094b authored by KAMEZAWA Hiroyuki's avatar KAMEZAWA Hiroyuki Committed by Linus Torvalds
Browse files

memcg: simplify move_account() check 



In memcg, for avoiding take-lock-irq-off at accessing page_cgroup, a
logic, flag + rcu_read_lock(), is used.  This works as following

     CPU-A                     CPU-B
                             rcu_read_lock()
    set flag
                             if(flag is set)
                                   take heavy lock
                             do job.
    synchronize_rcu()        rcu_read_unlock()
    take heavy lock.

In recent discussion, it's argued that using per-cpu value for this flag
just complicates the code because 'set flag' is very rare.

This patch changes 'flag' implementation from percpu to atomic_t.  This
will be much simpler.

Signed-off-by: default avatarKAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: default avatarGreg Thelen <gthelen@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Ying Han <yinghan@google.com>
Cc: "Paul E. McKenney" <paulmck@us.ibm.com>
Acked-by: default avatarJohannes Weiner <hannes@cmpxchg.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 9e335790

mm/memcontrol.c

+30 −40
Original line number Diff line number Diff line
@@ -89,7 +89,6 @@ enum mem_cgroup_stat_index { 
	MEM_CGROUP_STAT_FILE_MAPPED,  /* # of pages charged as file rss */

	MEM_CGROUP_STAT_SWAPOUT, /* # of pages, swapped out */

	MEM_CGROUP_STAT_DATA, /* end of data requires synchronization */

	MEM_CGROUP_ON_MOVE,	/* someone is moving account between groups */

	MEM_CGROUP_STAT_NSTATS,

};
@@ -297,6 +296,10 @@ struct mem_cgroup { 
	 * mem_cgroup ? And what type of charges should we move ?

	 */

	unsigned long 	move_charge_at_immigrate;

	/*

	 * set > 0 if pages under this cgroup are moving to other cgroup.

	 */

	atomic_t	moving_account;

	/*

	 * percpu counter.

	 */
@@ -1287,35 +1290,36 @@ int mem_cgroup_swappiness(struct mem_cgroup *memcg) 
	return memcg->swappiness;

}



/*

 * memcg->moving_account is used for checking possibility that some thread is

 * calling move_account(). When a thread on CPU-A starts moving pages under

 * a memcg, other threads should check memcg->moving_account under

 * rcu_read_lock(), like this:

 *

 *         CPU-A                                    CPU-B

 *                                              rcu_read_lock()

 *         memcg->moving_account+1              if (memcg->mocing_account)

 *                                                   take heavy locks.

 *         synchronize_rcu()                    update something.

 *                                              rcu_read_unlock()

 *         start move here.

 */

static void mem_cgroup_start_move(struct mem_cgroup *memcg)

{

	int cpu;



	get_online_cpus();

	spin_lock(&memcg->pcp_counter_lock);

	for_each_online_cpu(cpu)

		per_cpu(memcg->stat->count[MEM_CGROUP_ON_MOVE], cpu) += 1;

	memcg->nocpu_base.count[MEM_CGROUP_ON_MOVE] += 1;

	spin_unlock(&memcg->pcp_counter_lock);

	put_online_cpus();



	atomic_inc(&memcg->moving_account);

	synchronize_rcu();

}



static void mem_cgroup_end_move(struct mem_cgroup *memcg)

{

	int cpu;



	if (!memcg)

		return;

	get_online_cpus();

	spin_lock(&memcg->pcp_counter_lock);

	for_each_online_cpu(cpu)

		per_cpu(memcg->stat->count[MEM_CGROUP_ON_MOVE], cpu) -= 1;

	memcg->nocpu_base.count[MEM_CGROUP_ON_MOVE] -= 1;

	spin_unlock(&memcg->pcp_counter_lock);

	put_online_cpus();

	/*

	 * Now, mem_cgroup_clear_mc() may call this function with NULL.

	 * We check NULL in callee rather than caller.

	 */

	if (memcg)

		atomic_dec(&memcg->moving_account);

}



/*

 * 2 routines for checking "mem" is under move_account() or not.

 *
@@ -1331,7 +1335,7 @@ static void mem_cgroup_end_move(struct mem_cgroup *memcg) 
static bool mem_cgroup_stealed(struct mem_cgroup *memcg)

{

	VM_BUG_ON(!rcu_read_lock_held());

	return this_cpu_read(memcg->stat->count[MEM_CGROUP_ON_MOVE]) > 0;

	return atomic_read(&memcg->moving_account) > 0;

}



static bool mem_cgroup_under_move(struct mem_cgroup *memcg)
@@ -1882,8 +1886,8 @@ bool mem_cgroup_handle_oom(struct mem_cgroup *memcg, gfp_t mask, int order) 
 * by flags.

 *

 * Considering "move", this is an only case we see a race. To make the race

 * small, we check MEM_CGROUP_ON_MOVE percpu value and detect there are

 * possibility of race condition. If there is, we take a lock.

 * small, we check mm->moving_account and detect there are possibility of race

 * If there is, we take a lock.

 */



void mem_cgroup_update_page_stat(struct page *page,
@@ -2100,17 +2104,6 @@ static void mem_cgroup_drain_pcp_counter(struct mem_cgroup *memcg, int cpu) 
		per_cpu(memcg->stat->events[i], cpu) = 0;

		memcg->nocpu_base.events[i] += x;

	}

	/* need to clear ON_MOVE value, works as a kind of lock. */

	per_cpu(memcg->stat->count[MEM_CGROUP_ON_MOVE], cpu) = 0;

	spin_unlock(&memcg->pcp_counter_lock);

}



static void synchronize_mem_cgroup_on_move(struct mem_cgroup *memcg, int cpu)

{

	int idx = MEM_CGROUP_ON_MOVE;



	spin_lock(&memcg->pcp_counter_lock);

	per_cpu(memcg->stat->count[idx], cpu) = memcg->nocpu_base.count[idx];

	spin_unlock(&memcg->pcp_counter_lock);

}
@@ -2122,11 +2115,8 @@ static int __cpuinit memcg_cpu_hotplug_callback(struct notifier_block *nb, 
	struct memcg_stock_pcp *stock;

	struct mem_cgroup *iter;



	if ((action == CPU_ONLINE)) {

		for_each_mem_cgroup(iter)

			synchronize_mem_cgroup_on_move(iter, cpu);

	if (action == CPU_ONLINE)

		return NOTIFY_OK;

	}



	if ((action != CPU_DEAD) || action != CPU_DEAD_FROZEN)

		return NOTIFY_OK;

CRA Git | Maintained and supported by SUSTech CRA and CCSE