Re: [patch 02/12] mm: oom_kill: clean up victim marking and exiting interfaces

From: Michal Hocko
Date: Thu Mar 26 2015 - 07:54:50 EST

On Wed 25-03-15 02:17:06, Johannes Weiner wrote:
> Rename unmark_oom_victim() to exit_oom_victim(). Marking and
> unmarking are related in functionality, but the interface is not
> symmetrical at all: one is an internal OOM killer function used during
> the killing, the other is for an OOM victim to signal its own death on
> exit later on. This has locking implications, see follow-up changes.
>
> While at it, rename mark_tsk_oom_victim() to mark_oom_victim(), which
> is easier on the eye.

The reason _tsk_ was used was to be in sync with *tsk_thread_flag API.
I do not mind changing that though.

>
> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>

Acked-by: Michal Hocko <mhocko@xxxxxxx>

> ---
> drivers/staging/android/lowmemorykiller.c | 2 +-
> include/linux/oom.h | 7 ++++---
> kernel/exit.c | 2 +-
> mm/memcontrol.c | 2 +-
> mm/oom_kill.c | 16 +++++++---------
> 5 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
> index feafa172b155..2345ee7342d9 100644
> --- a/drivers/staging/android/lowmemorykiller.c
> +++ b/drivers/staging/android/lowmemorykiller.c
> @@ -165,7 +165,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
> * infrastructure. There is no real reason why the selected
> * task should have access to the memory reserves.
> */
> - mark_tsk_oom_victim(selected);
> + mark_oom_victim(selected);
> send_sig(SIGKILL, selected, 0);
> rem += selected_tasksize;
> }
> diff --git a/include/linux/oom.h b/include/linux/oom.h
> index 44b2f6f7bbd8..a8e6a498cbcb 100644
> --- a/include/linux/oom.h
> +++ b/include/linux/oom.h
> @@ -47,9 +47,7 @@ static inline bool oom_task_origin(const struct task_struct *p)
> return !!(p->signal->oom_flags & OOM_FLAG_ORIGIN);
> }
>
> -extern void mark_tsk_oom_victim(struct task_struct *tsk);
> -
> -extern void unmark_oom_victim(void);
> +extern void mark_oom_victim(struct task_struct *tsk);
>
> extern unsigned long oom_badness(struct task_struct *p,
> struct mem_cgroup *memcg, const nodemask_t *nodemask,
> @@ -75,6 +73,9 @@ extern enum oom_scan_t oom_scan_process_thread(struct task_struct *task,
>
> extern bool out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
> int order, nodemask_t *mask, bool force_kill);
> +
> +extern void exit_oom_victim(void);
> +
> extern int register_oom_notifier(struct notifier_block *nb);
> extern int unregister_oom_notifier(struct notifier_block *nb);
>
> diff --git a/kernel/exit.c b/kernel/exit.c
> index feff10bbb307..4089c2fd373e 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -436,7 +436,7 @@ static void exit_mm(struct task_struct *tsk)
> mm_update_next_owner(mm);
> mmput(mm);
> if (test_thread_flag(TIF_MEMDIE))
> - unmark_oom_victim();
> + exit_oom_victim();
> }
>
> static struct task_struct *find_alive_thread(struct task_struct *p)
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 74a9641d8f9f..aab5604e0ac4 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1536,7 +1536,7 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> * quickly exit and free its memory.
> */
> if (fatal_signal_pending(current) || task_will_free_mem(current)) {
> - mark_tsk_oom_victim(current);
> + mark_oom_victim(current);
> return;
> }
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 73763e489e86..b2f081fe4b1a 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -408,13 +408,13 @@ bool oom_killer_disabled __read_mostly;
> static DECLARE_RWSEM(oom_sem);
>
> /**
> - * mark_tsk_oom_victim - marks the given task as OOM victim.
> + * mark_oom_victim - mark the given task as OOM victim
> * @tsk: task to mark
> *
> * Has to be called with oom_sem taken for read and never after
> * oom has been disabled already.
> */
> -void mark_tsk_oom_victim(struct task_struct *tsk)
> +void mark_oom_victim(struct task_struct *tsk)
> {
> WARN_ON(oom_killer_disabled);
> /* OOM killer might race with memcg OOM */
> @@ -431,11 +431,9 @@ void mark_tsk_oom_victim(struct task_struct *tsk)
> }
>
> /**
> - * unmark_oom_victim - unmarks the current task as OOM victim.
> - *
> - * Wakes up all waiters in oom_killer_disable()
> + * exit_oom_victim - note the exit of an OOM victim
> */
> -void unmark_oom_victim(void)
> +void exit_oom_victim(void)
> {
> if (!test_and_clear_thread_flag(TIF_MEMDIE))
> return;
> @@ -515,7 +513,7 @@ void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
> */
> task_lock(p);
> if (p->mm && task_will_free_mem(p)) {
> - mark_tsk_oom_victim(p);
> + mark_oom_victim(p);
> task_unlock(p);
> put_task_struct(p);
> return;
> @@ -570,7 +568,7 @@ void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
>
> /* mm cannot safely be dereferenced after task_unlock(victim) */
> mm = victim->mm;
> - mark_tsk_oom_victim(victim);
> + mark_oom_victim(victim);
> pr_err("Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB\n",
> task_pid_nr(victim), victim->comm, K(victim->mm->total_vm),
> K(get_mm_counter(victim->mm, MM_ANONPAGES)),
> @@ -728,7 +726,7 @@ static void __out_of_memory(struct zonelist *zonelist, gfp_t gfp_mask,
> */
> if (current->mm &&
> (fatal_signal_pending(current) || task_will_free_mem(current))) {
> - mark_tsk_oom_victim(current);
> + mark_oom_victim(current);
> return;
> }
>
> --
> 2.3.3
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>

--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/