On Fri, Jan 17, 2020 at 01:02:34PM +0300, Kirill Tkhai wrote:
> On 17.01.2020 02:59, Minchan Kim wrote:
> > This patch factor out madvise's core functionality so that upcoming
> > patch can reuse it without duplication. It shouldn't change any behavior.
> >
> > Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> > ---
> > mm/madvise.c | 194 +++++++++++++++++++++++++++++----------------------
> > 1 file changed, 111 insertions(+), 83 deletions(-)
> >
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index bcdb6a042787..0c901de531e4 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -35,6 +35,7 @@
> > struct madvise_walk_private {
> > struct mmu_gather *tlb;
> > bool pageout;
> > + struct task_struct *task;
> > };
> >
> > /*
> > @@ -306,12 +307,13 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
> > bool pageout = private->pageout;
> > struct mm_struct *mm = tlb->mm;
> > struct vm_area_struct *vma = walk->vma;
> > + struct task_struct *task = private->task;
> > pte_t *orig_pte, *pte, ptent;
> > spinlock_t *ptl;
> > struct page *page = NULL;
> > LIST_HEAD(page_list);
> >
> > - if (fatal_signal_pending(current))
> > + if (fatal_signal_pending(task))
> > return -EINTR;
>
> This EINTR may confuse userspace. Users will think the syscall was interrupted,
> and it may be restarted, but this is not true.
madvise_[pageout|cold] doesn't propagate the error to userspace.
>
> What we care here? Current task received fatal signal, while walk_page_range(..&cold_walk_ops..)
> is a long cycle. So, this check allows to break the cycle faster.
>
> Iteration over remote task's mm may also be long, and we still may need to break
> it if current received a signal.
>
> So, we'd better left fatal_signal_pending(current) here.
>
> Maybe we need both tasks fatal_signal_pending() checks and different retvals here,
> but it's up to you.
Yub, let's check both processes here to bail out.
Thanks for the review!
