Re: [PATCH] mm/mempolicy: support MPOL_MF_STRICT for huge page mapping

[Mike Kravetz <mike.kravetz@oracle.com>]

On 1/30/20 5:33 PM, Li Xinhai wrote:
> MPOL_MF_STRICT is used in mbind() for purposes:
> (1) MPOL_MF_STRICT is set alone without MPOL_MF_MOVE or MPOL_MF_MOVE_ALL,
>     to check if there is misplaced page and return -EIO;
> (2) MPOL_MF_STRICT is set with MPOL_MF_MOVE or MPOL_MF_MOVE_ALL, to check
>     if there is misplaced page which is failed to isolate, or page is
>     success on isolate but failed to move, and return -EIO.
> 
> For non hugepage mapping, (1) and (2) are implemented as expectation.
> For hugepage mapping, (1) is not implemented. And in (2), the part about
> failed to isolate and report -EIO is not implemented.
> 
> This patch implements the missed parts for hugepage mapping. Benefits
> with it applied:
> - User space can apply same code logic to handle mbind() on hugepage and
>   non hugepage mapping;
> - Reliably using MPOL_MF_STRICT alone to check whether there is misplaced
>   page or not when bind policy on address range, especially for address
>   range which contains both hugepage and non hugepage mapping.
> 
> Analysis of potential impact on existing users:
> - For users who using MPOL_MF_STRICT alone, since mbind() don't report
>   reliable answer about misplaced page, their existing code have to
>   utilize other facilities, e.g. numa_maps of proc, to check misplaced
>   page. After this patch applied, that dedicated checking will still work
>   without been impacted;

I do not agree with this characterization of impact to existing users.  If
someone uses MPOL_MF_STRICT alone with hugetlb pages today, they will never
get EIO.  After this patch, they will get EIO if the hugetlb pages do not
follow the policy.

Yes, this is the desired behavior after the change with updated documentation.
However, it is a potential change for existing users.  I honestly do not
believe anyone will notice.  But, it is a change in behavior.

> - For users who using MPOL_MF_STRICT with MPOL_MF_MOVE or
>   MPOL_MF_MOVE_ALL, the semantic about some pages could not be moved will
>   not be changed by this patch, because failed to isolate and failed to
>   move have same effects to users, so their existing code will not be
>   impacted.
> 
> In mbind man page, the note about 'MPOL_MF_STRICT is ignored on huge page
> mappings' can be removed after this patch is applied.
> 
> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: linux-man <linux-man@vger.kernel.org>
> ---
> Link to relevant discussion:
> https://lore.kernel.org/linux-mm/1578993378-10860-2-git-send-email-lixinhai.lxh@gmail.com/
> 

Thanks for doing this and the commit message.  I do not see any issues with
the code.

I believe removing the special case for hugetlb pages in mbind is a good
thing.  It is unfortunate that it will cause a change in behavior.  My
belief is that this change will go unnoticed.  Providing consistent
behavior that matches the (updated) documentation is better that maintaining
the current functionality into the future.

-- 
Mike Kravetz