Re: [patch] oom: print triggering task's cpuset and mems allowed

[Li Zefan <lizf@cn.fujitsu.com>]

> oom: print triggering task's cpuset and mems allowed
> 
> When cpusets are enabled, it's necessary to print the triggering task's
> set of allowable nodes so the subsequently printed meminfo can be
> interpreted correctly.
> 
> We also print the task's cpuset name for informational purposes.
> 
> Cc: Paul Menage <menage@google.com>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  include/linux/cpuset.h |    6 ++++++
>  kernel/cpuset.c        |   34 ++++++++++++++++++++++++++++++++++
>  mm/oom_kill.c          |    1 +
>  3 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> --- a/include/linux/cpuset.h
> +++ b/include/linux/cpuset.h
> @@ -80,6 +80,8 @@ extern int current_cpuset_is_being_rebound(void);
>  
>  extern void rebuild_sched_domains(void);
>  
> +extern void cpuset_print_task_mems_allowed(struct task_struct *p);
> +
>  #else /* !CONFIG_CPUSETS */
>  
>  static inline int cpuset_init_early(void) { return 0; }
> @@ -163,6 +165,10 @@ static inline void rebuild_sched_domains(void)
>  	partition_sched_domains(1, NULL, NULL);
>  }
>  
> +static inline void cpuset_print_task_mems_allowed(struct task_struct *p)
> +{
> +}
> +
>  #endif /* !CONFIG_CPUSETS */
>  
>  #endif /* _LINUX_CPUSET_H */
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -239,6 +239,17 @@ static struct cpuset top_cpuset = {
>  static DEFINE_MUTEX(callback_mutex);
>  
>  /*
> + * cpuset_buffer_lock protects both the cpuset_name and cpuset_nodelist
> + * buffers.  They are statically allocated to prevent using excess stack
> + * when calling cpuset_print_task_mems_allowed().
> + */
> +#define CPUSET_NAME_LEN		(128)
> +#define	CPUSET_NODELIST_LEN	(256)
> +static char cpuset_name[CPUSET_NAME_LEN];
> +static char cpuset_nodelist[CPUSET_NODELIST_LEN];
> +static DEFINE_SPINLOCK(cpuset_buffer_lock);
> +
> +/*
>   * This is ugly, but preserves the userspace API for existing cpuset
>   * users. If someone tries to mount the "cpuset" filesystem, we
>   * silently switch it to mount "cgroup" instead
> @@ -2339,6 +2350,29 @@ int cpuset_mems_allowed_intersects(const struct task_struct *tsk1,
>  	return nodes_intersects(tsk1->mems_allowed, tsk2->mems_allowed);
>  }
>  
> +/**
> + * cpuset_print_task_mems_allowed - prints task's cpuset and mems_allowed
> + * @task: pointer to task_struct of some task.
> + *
> + * Description: Prints @task's name, cpuset name, and cached copy of its
> + * mems_allowed to the kernel log.  Must hold task_lock(task) to allow
> + * dereferencing task_cs(task).
> + */

But none of the callers of this function holds task_lock nor rcu_read_lock,
they hold tasklist_lock. Have you confirmed this is ok? It seems racy that
the task may be moved to another cgroup and the original cgroup gets removed.

> +void cpuset_print_task_mems_allowed(struct task_struct *tsk)
> +{
> +	struct dentry *dentry;
> +
> +	dentry = task_cs(tsk)->css.cgroup->dentry;
> +	spin_lock(&cpuset_buffer_lock);
> +	snprintf(cpuset_name, CPUSET_NAME_LEN,
> +		 dentry ? (const char *)dentry->d_name.name : "/");
> +	nodelist_scnprintf(cpuset_nodelist, CPUSET_NODELIST_LEN,
> +			   tsk->mems_allowed);
> +	printk(KERN_INFO "%s cpuset=%s mems_allowed=%s\n",
> +	       tsk->comm, cpuset_name, cpuset_nodelist);
> +	spin_unlock(&cpuset_buffer_lock);
> +}
> +
>  /*
>   * Collection of memory_pressure is suppressed unless
>   * this flag is enabled by writing "1" to the special
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -391,6 +391,7 @@ static int oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
>  		printk(KERN_WARNING "%s invoked oom-killer: "
>  			"gfp_mask=0x%x, order=%d, oomkilladj=%d\n",
>  			current->comm, gfp_mask, order, current->oomkilladj);
> +		cpuset_print_task_mems_allowed(current);
>  		dump_stack();
>  		show_mem();
>  		if (sysctl_oom_dump_tasks)
> --