Re: [PATCH][V2] Make get_user_pages interruptible

David Rientjes
2008-11-22T00:21:26Z

On Fri, 21 Nov 2008, Ying Han wrote:

> Signed-off-by: Paul Menage <menage@google.com>
> Ying Han <yinghan@google.com>
>

That should be:

Signed-off-by: Paul Menage <menage@google.com>
Signed-off-by: Ying Han <yinghan@google.com>

and the first signed-off line is usually indicative of who wrote the
original change. If Paul wrote this code, please add:

From: Paul Menage <menage@google.com>

as the first line of the email so that the proper authorship gets
attributed in the commit.

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index b483f39..f9c6a8a 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1790,6 +1790,7 @@ extern void sched_dead(struct task_struct *p);
> extern int in_group_p(gid_t);
> extern int in_egroup_p(gid_t);
>
> +extern int sigkill_pending(struct task_struct *tsk);
> extern void proc_caches_init(void);
> extern void flush_signals(struct task_struct *);
> extern void ignore_signals(struct task_struct *);

Interesting way around your email client's line truncation.

> diff --git a/mm/memory.c b/mm/memory.c
> index 164951c..5d3db5e 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1218,12 +1218,11 @@ int __get_user_pages(struct task_struct *tsk, struct m
> struct page *page;
>
> /*
> - * If tsk is ooming, cut off its access to large memory
> - * allocations. It has a pending SIGKILL, but it can't
> - * be processed until returning to user space.
> + * If we have a pending SIGKILL, don't keep
> + * allocating memory.
> */
> - if (unlikely(test_tsk_thread_flag(tsk, TIF_MEMDIE)))
> - return i ? i : -ENOMEM;
> + if (sigkill_pending(current))
> + return i ? i : -ERESTARTSYS;
>
> if (write)
> foll_flags |= FOLL_WRITE;
>

We previously tested tsk for TIF_MEMDIE and not current (in fact, nothing
in __get_user_pages() operates on current). So why are we introducing
this check on current and not tsk?

Do we want to avoid branch prediction now because there's data suggesting
tsk will be SIGKILL'd more frequently in this path other than by the oom
killer?