Merge branch 'sched-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip

Pull scheduler fixes from Thomas Gleixner: "Two fixes for the fair scheduling class: - Prevent freeing memory which is accessible by concurrent readers - Make the RCU annotations for numa groups consistent" * 'sched-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: sched/fair: Use RCU accessors consistently for ->numa_group sched/fair: Don't free p->numa_faults with concurrent readers

Linus Torvalds committed 7 years ago

Commit e24ce842 parents 750991f cb361d85 files · +114 −48Browse files

Unified · Split

5 files

fs/exec.c +1 −1
include/linux/sched.h +9 −1
include/linux/sched/numa_balancing.h +2 −2
kernel/fork.c +1 −1
kernel/sched/fair.c +101 −43

fs/exec.c+1−1

@@ -1828,7 +1828,7 @@ static int __do_execve_file(int fd, struct filename *filename, · 1827 unmodified lines
1828	membarrier_execve(current);	1828	membarrier_execve(current);
1829	rseq_execve(current);	1829	rseq_execve(current);
1830	acct_update_integrals(current);	1830	acct_update_integrals(current);
1831	task_numa_free(current);	1831	task_numa_free(current, false);
1832	free_bprm(bprm);	1832	free_bprm(bprm);
1833	kfree(pathbuf);	1833	kfree(pathbuf);
1834	if (filename)	1834	if (filename)

include/linux/sched.h+9−1

@@ -1092,7 +1092,15 @@ struct task_struct { · 1091 unmodified lines
1092	u64 last_sum_exec_runtime;	1092	u64 last_sum_exec_runtime;
1093	struct callback_head numa_work;	1093	struct callback_head numa_work;
1094		1094
1095	struct numa_group *numa_group;	1095	/*
		1096	* This pointer is only modified for current in syscall and
		1097	* pagefault context (and for tasks being destroyed), so it can be read
		1098	* from any of the following contexts:
		1099	* - RCU read-side critical section
		1100	* - current->numa_group from everywhere
		1101	* - task's runqueue locked, task not running
		1102	*/
		1103	struct numa_group __rcu *numa_group;
1096		1104
1097	/*	1105	/*
1098	* numa_faults is an array split into four regions:	1106	* numa_faults is an array split into four regions:

include/linux/sched/numa_balancing.h+2−2

@@ -19,7 +19,7 @@ · 18 unmodified lines
19	extern void task_numa_fault(int last_node, int node, int pages, int flags);	19	extern void task_numa_fault(int last_node, int node, int pages, int flags);
20	extern pid_t task_numa_group_id(struct task_struct *p);	20	extern pid_t task_numa_group_id(struct task_struct *p);
21	extern void set_numabalancing_state(bool enabled);	21	extern void set_numabalancing_state(bool enabled);
22	extern void task_numa_free(struct task_struct *p);	22	extern void task_numa_free(struct task_struct *p, bool final);
23	extern bool should_numa_migrate_memory(struct task_struct p, struct page page,	23	extern bool should_numa_migrate_memory(struct task_struct p, struct page page,
24	int src_nid, int dst_cpu);	24	int src_nid, int dst_cpu);
25	#else	25	#else
@@ -34,7 +34,7 @@ static inline pid_t task_numa_group_id(struct task_struct *p) · 8 unmodified lines
34	static inline void set_numabalancing_state(bool enabled)	34	static inline void set_numabalancing_state(bool enabled)
35	{	35	{
36	}	36	}
37	static inline void task_numa_free(struct task_struct *p)	37	static inline void task_numa_free(struct task_struct *p, bool final)
38	{	38	{
39	}	39	}
40	static inline bool should_numa_migrate_memory(struct task_struct *p,	40	static inline bool should_numa_migrate_memory(struct task_struct *p,

kernel/fork.c+1−1

@@ -726,7 +726,7 @@ void __put_task_struct(struct task_struct *tsk) · 725 unmodified lines
726	WARN_ON(tsk == current);	726	WARN_ON(tsk == current);
727		727
728	cgroup_free(tsk);	728	cgroup_free(tsk);
729	task_numa_free(tsk);	729	task_numa_free(tsk, true);
730	security_task_free(tsk);	730	security_task_free(tsk);
731	exit_creds(tsk);	731	exit_creds(tsk);
732	delayacct_tsk_free(tsk);	732	delayacct_tsk_free(tsk);

kernel/sched/fair.c+101−43

 	unsigned long faults[0];
 };
+/*
+ * For functions that can be called in multiple contexts that permit reading
+ * ->numa_group (see struct task_struct for locking rules).
+ */
+static struct numa_group *deref_task_numa_group(struct task_struct *p)
+{
+	return rcu_dereference_check(p->numa_group, p == current ||
+		(lockdep_is_held(&task_rq(p)->lock) && !READ_ONCE(p->on_cpu)));
+}
+static struct numa_group *deref_curr_numa_group(struct task_struct *p)
+{
+	return rcu_dereference_protected(p->numa_group, p == current);
+}
 static inline unsigned long group_faults_priv(struct numa_group *ng);
 static inline unsigned long group_faults_shared(struct numa_group *ng);
 {
 	unsigned long smin = task_scan_min(p);
 	unsigned long period = smin;
+	struct numa_group *ng;
 	/* Scale the maximum scan period with the amount of shared memory. */
-	if (p->numa_group) {
+	rcu_read_lock();
-		struct numa_group *ng = p->numa_group;
+	ng = rcu_dereference(p->numa_group);
+	if (ng) {
 		unsigned long shared = group_faults_shared(ng);
 		unsigned long private = group_faults_priv(ng);
 		period *= shared + 1;
 		period /= private + shared + 1;
 	}
+	rcu_read_unlock();
 	return max(smin, period);
 }
 {
 	unsigned long smin = task_scan_min(p);
 	unsigned long smax;
+	struct numa_group *ng;
 	/* Watch for min being lower than max due to floor calculations */
 	smax = sysctl_numa_balancing_scan_period_max / task_nr_scan_windows(p);
 	/* Scale the maximum scan period with the amount of shared memory. */
-	if (p->numa_group) {
+	ng = deref_curr_numa_group(p);
-		struct numa_group *ng = p->numa_group;
+	if (ng) {
 		unsigned long shared = group_faults_shared(ng);
 		unsigned long private = group_faults_priv(ng);
 		unsigned long period = smax;
 	p->numa_scan_period		= sysctl_numa_balancing_scan_delay;
 	p->numa_work.next		= &p->numa_work;
 	p->numa_faults			= NULL;
-	p->numa_group			= NULL;
+	RCU_INIT_POINTER(p->numa_group, NULL);
 	p->last_task_numa_placement	= 0;
 	p->last_sum_exec_runtime	= 0;
 pid_t task_numa_group_id(struct task_struct *p)
 {
-	return p->numa_group ? p->numa_group->gid : 0;
+	struct numa_group *ng;
+	pid_t gid = 0;
+	rcu_read_lock();
+	ng = rcu_dereference(p->numa_group);
+	if (ng)
+		gid = ng->gid;
+	rcu_read_unlock();
+	return gid;
 }
 /*
 static inline unsigned long group_faults(struct task_struct *p, int nid)
 {
-	if (!p->numa_group)
+	struct numa_group *ng = deref_task_numa_group(p);
+	if (!ng)
 		return 0;
-	return p->numa_group->faults[task_faults_idx(NUMA_MEM, nid, 0)] +
+	return ng->faults[task_faults_idx(NUMA_MEM, nid, 0)] +
-		p->numa_group->faults[task_faults_idx(NUMA_MEM, nid, 1)];
+		ng->faults[task_faults_idx(NUMA_MEM, nid, 1)];
 }
 static inline unsigned long group_faults_cpu(struct numa_group *group, int nid)
 static inline unsigned long group_weight(struct task_struct *p, int nid,
 					 int dist)
 {
+	struct numa_group *ng = deref_task_numa_group(p);
 	unsigned long faults, total_faults;
-	if (!p->numa_group)
+	if (!ng)
 		return 0;
-	total_faults = p->numa_group->total_faults;
+	total_faults = ng->total_faults;
 	if (!total_faults)
 		return 0;
 bool should_numa_migrate_memory(struct task_struct *p, struct page * page,
 				int src_nid, int dst_cpu)
 {
-	struct numa_group *ng = p->numa_group;
+	struct numa_group *ng = deref_curr_numa_group(p);
 	int dst_nid = cpu_to_node(dst_cpu);
 	int last_cpupid, this_cpupid;
 static void task_numa_compare(struct task_numa_env *env,
 			      long taskimp, long groupimp, bool maymove)
 {
+	struct numa_group *cur_ng, *p_ng = deref_curr_numa_group(env->p);
 	struct rq *dst_rq = cpu_rq(env->dst_cpu);
+	long imp = p_ng ? groupimp : taskimp;
 	struct task_struct *cur;
 	long src_load, dst_load;
-	long load;
-	long imp = env->p->numa_group ? groupimp : taskimp;
-	long moveimp = imp;
 	int dist = env->dist;
+	long moveimp = imp;
+	long load;
 	if (READ_ONCE(dst_rq->numa_migrate_on))
 		return;
 	 * If dst and source tasks are in the same NUMA group, or not
 	 * in any group then look only at task weights.
 	 */
-	if (cur->numa_group == env->p->numa_group) {
+	cur_ng = rcu_dereference(cur->numa_group);
+	if (cur_ng == p_ng) {
 		imp = taskimp + task_weight(cur, env->src_nid, dist) -
 		      task_weight(cur, env->dst_nid, dist);
 		/*
 		 * Add some hysteresis to prevent swapping the
 		 * tasks within a group over tiny differences.
 		 */
-		if (cur->numa_group)
+		if (cur_ng)
 			imp -= imp / 16;
 	} else {
 		/*
 		 * Compare the group weights. If a task is all by itself
 		 * (not part of a group), use the task weight instead.
 		 */
-		if (cur->numa_group && env->p->numa_group)
+		if (cur_ng && p_ng)
 			imp += group_weight(cur, env->src_nid, dist) -
 			       group_weight(cur, env->dst_nid, dist);
 		else
 		.best_imp = 0,
 		.best_cpu = -1,
 	};
+	unsigned long taskweight, groupweight;
 	struct sched_domain *sd;
+	long taskimp, groupimp;
+	struct numa_group *ng;
 	struct rq *best_rq;
-	unsigned long taskweight, groupweight;
 	int nid, ret, dist;
-	long taskimp, groupimp;
 	/*
 	 * Pick the lowest SD_NUMA domain, as that would have the smallest
 	 *   multiple NUMA nodes; in order to better consolidate the group,
 	 *   we need to check other locations.
 	 */
-	if (env.best_cpu == -1 || (p->numa_group && p->numa_group->active_nodes > 1)) {
+	ng = deref_curr_numa_group(p);
+	if (env.best_cpu == -1 || (ng && ng->active_nodes > 1)) {
 		for_each_online_node(nid) {
 			if (nid == env.src_nid || nid == p->numa_preferred_nid)
 				continue;
 	 * A task that migrated to a second choice node will be better off
 	 * trying for a better one later. Do not set the preferred node here.
 	 */
-	if (p->numa_group) {
+	if (ng) {
 		if (env.best_cpu == -1)
 			nid = env.src_nid;
 		else
 	unsigned long total_faults;
 	u64 runtime, period;
 	spinlock_t *group_lock = NULL;
+	struct numa_group *ng;
 	/*
 	 * The p->mm->numa_scan_seq field gets updated without
 	runtime = numa_get_avg_runtime(p, &period);
 	/* If the task is part of a group prevent parallel updates to group stats */
-	if (p->numa_group) {
+	ng = deref_curr_numa_group(p);
-		group_lock = &p->numa_group->lock;
+	if (ng) {
+		group_lock = &ng->lock;
 		spin_lock_irq(group_lock);
 	}
 			p->numa_faults[cpu_idx] += f_diff;
 			faults += p->numa_faults[mem_idx];
 			p->total_numa_faults += diff;
-			if (p->numa_group) {
+			if (ng) {
 				/*
 				 * safe because we can only change our own group
 				 *
 				 * nid and priv in a specific region because it
 				 * is at the beginning of the numa_faults array.
 				 */
-				p->numa_group->faults[mem_idx] += diff;
+				ng->faults[mem_idx] += diff;
-				p->numa_group->faults_cpu[mem_idx] += f_diff;
+				ng->faults_cpu[mem_idx] += f_diff;
-				p->numa_group->total_faults += diff;
+				ng->total_faults += diff;
-				group_faults += p->numa_group->faults[mem_idx];
+				group_faults += ng->faults[mem_idx];
 			}
 		}
-		if (!p->numa_group) {
+		if (!ng) {
 			if (faults > max_faults) {
 				max_faults = faults;
 				max_nid = nid;
 		}
 	}
-	if (p->numa_group) {
+	if (ng) {
-		numa_group_count_active_nodes(p->numa_group);
+		numa_group_count_active_nodes(ng);
 		spin_unlock_irq(group_lock);
 		max_nid = preferred_group_nid(p, max_nid);
 	}
 	int cpu = cpupid_to_cpu(cpupid);
 	int i;
-	if (unlikely(!p->numa_group)) {
+	if (unlikely(!deref_curr_numa_group(p))) {
 		unsigned int size = sizeof(struct numa_group) +
 				    4*nr_node_ids*sizeof(unsigned long);
 	if (!grp)
 		goto no_join;
-	my_grp = p->numa_group;
+	my_grp = deref_curr_numa_group(p);
 	if (grp == my_grp)
 		goto no_join;
 	return;
 }
-void task_numa_free(struct task_struct *p)
+/*
+ * Get rid of NUMA staticstics associated with a task (either current or dead).
+ * If @final is set, the task is dead and has reached refcount zero, so we can
+ * safely free all relevant data structures. Otherwise, there might be
+ * concurrent reads from places like load balancing and procfs, and we should
+ * reset the data back to default state without freeing ->numa_faults.
+ */
+void task_numa_free(struct task_struct *p, bool final)
 {
-	struct numa_group *grp = p->numa_group;
+	/* safe: p either is current or is being freed by current */
-	void *numa_faults = p->numa_faults;
+	struct numa_group *grp = rcu_dereference_raw(p->numa_group);
+	unsigned long *numa_faults = p->numa_faults;
 	unsigned long flags;
 	int i;
+	if (!numa_faults)
+		return;
 	if (grp) {
 		spin_lock_irqsave(&grp->lock, flags);
 		for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++)
 		put_numa_group(grp);
 	}
-	p->numa_faults = NULL;
+	if (final) {
-	kfree(numa_faults);
+		p->numa_faults = NULL;
+		kfree(numa_faults);
+	} else {
+		p->total_numa_faults = 0;
+		for (i = 0; i < NR_NUMA_HINT_FAULT_STATS * nr_node_ids; i++)
+			numa_faults[i] = 0;
+	}
 }
 /*
 	 * actively using should be counted as local. This allows the
 	 * scan rate to slow down when a workload has settled down.
 	 */
-	ng = p->numa_group;
+	ng = deref_curr_numa_group(p);
 	if (!priv && !local && ng && ng->active_nodes > 1 &&
 				numa_is_active_node(cpu_node, ng) &&
 				numa_is_active_node(mem_node, ng))
 {
 	int node;
 	unsigned long tsf = 0, tpf = 0, gsf = 0, gpf = 0;
+	struct numa_group *ng;
+	rcu_read_lock();
+	ng = rcu_dereference(p->numa_group);
 	for_each_online_node(node) {
 		if (p->numa_faults) {
 			tsf = p->numa_faults[task_faults_idx(NUMA_MEM, node, 0)];
 			tpf = p->numa_faults[task_faults_idx(NUMA_MEM, node, 1)];
 		}
-		if (p->numa_group) {
+		if (ng) {
-			gsf = p->numa_group->faults[task_faults_idx(NUMA_MEM, node, 0)],
+			gsf = ng->faults[task_faults_idx(NUMA_MEM, node, 0)],
-			gpf = p->numa_group->faults[task_faults_idx(NUMA_MEM, node, 1)];
+			gpf = ng->faults[task_faults_idx(NUMA_MEM, node, 1)];
 		}
 		print_numa_stats(m, node, tsf, tpf, gsf, gpf);
 	}
+	rcu_read_unlock();
 }
 #endif /* CONFIG_NUMA_BALANCING */
 #endif /* CONFIG_SCHED_DEBUG */

Merge branch 'sched-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code