Re: [PATCH][V2] Make get_user_pages interruptible

David Rientjes
2008-11-22T00:50:54Z

On Fri, 21 Nov 2008, Ying Han wrote:

> >> 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.
>

It doesn't matter, __get_user_pages() acts on 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?
> >
> any specific example?
>

I'm asking why you removed the unlikely() wrapper.