Re: [PATCH][V2] Make get_user_pages interruptible

Ying Han
2008-11-22T00:43:03Z

On Fri, Nov 21, 2008 at 4:21 PM, David Rientjes <rientjes@google.com> wrote:
> 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.
thanks for your info. fixed.
>
>> 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?
Initially, the patch is merely to cause a process stuck in mlock to
honour a pending sigkill. And in mlock case, tsk==current.

>
> 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?
>
any specific example?