Re: mbind() fails to fail with EIO

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Tue, Mar 19, 2019 at 02:27:33PM +0100, Oscar Salvador wrote:
> +CC Kirill
> 
> On Mon, Mar 18, 2019 at 11:12:19AM -0700, Yang Shi wrote:
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index abe7a67..6ba45aa 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -521,11 +521,14 @@ static int queue_pages_pte_range(pmd_t *pmd,
> > unsigned long addr,
> >                         continue;
> >                 if (!queue_pages_required(page, qp))
> >                         continue;
> > -               migrate_page_add(page, qp->pagelist, flags);
> > +               if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))
> > +                       migrate_page_add(page, qp->pagelist, flags);
> > +               else
> > +                       break;
> >         }
> >         pte_unmap_unlock(pte - 1, ptl);
> >         cond_resched();
> > -       return 0;
> > +       return addr != end ? -EIO : 0;
> >  }
> > 
> >  static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
> 
> This alone is not going to help.
> 
> The problem is that we do skip the vma early in queue_pages_test_walk() in
> case MPOL_MF_MOVE and MPOL_MF_MOVE_ALL are not set.
> 
> walk_page_range
>  walk_page_test
>   queue_pages_test_walk
> 
> 	...
>  	...
> 	/* queue pages from current vma */
> 	if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))
> 		return 0;
> 	return 1;
> 
> So, we skip the vma and keep going.
> 
> Before ("77bf45e78050: mempolicy: do not try to queue pages from !vma_migratable()"),
> queue_pages_test_walk() would not have skipped the vma in case we had MPOL_MF_STRICT
> or MPOL_MF_MOVE | MPOL_MF_MOVE_ALL.
> 
> I did not give it a lot of thought, but it seems to me that we might need to reach
> queue_pages_to_pte_range() in order to see whether the page is in the required node
> or not by calling queue_pages_required(), and if it is not, check for
> MPOL_MF_MOVE | MPOL_MF_MOVE_ALL like the above patch does, so we would be able to
> return -EIO.
> That would imply that we would need to re-add MPOL_MF_STRICT in queue_pages_test_walk().

That's all sounds reasonable.

We only need to make sure the bug fixed by 77bf45e78050 will not be
re-introduced.

-- 
 Kirill A. Shutemov