[v3,0/3] Fix CONT-PTE/PMD size hugetlb issue when unmapping or migrating

Message

Baolin Wang May 10, 2022, 3:45 a.m. UTC

Hi,

Now migrating a hugetlb page or unmapping a poisoned hugetlb page, we'll
use ptep_clear_flush() and set_pte_at() to nuke the page table entry
and remap it, and this is incorrect for CONT-PTE or CONT-PMD size hugetlb
page, which will cause potential data consistent issue. This patch set
will change to use hugetlb related APIs to fix this issue, please find
details in each patch. Thanks.

Note: Mike pointed out the huge_ptep_get() will only return the one specific
value, and it would not take into account the dirty or young bits of CONT-PTE/PMDs
like the huge_ptep_get_and_clear() [1]. This inconsistent issue is not introduced
by this patch set, and will address this issue in another thread [2]. Meanwhile
the uffd for hugetlb case [3] pointed by Gerald also need another patch to address.

[1] https://lore.kernel.org/linux-mm/[e-mail]/
[2] https://lore.kernel.org/all/[e-mail]/
[3] https://lore.kernel.org/linux-mm/20220503120343.6264e126@thinkpad/

Changes from v2:
 - Collect reviewed tags from Muchun and Mike.
 - Drop the unnecessary casting in hugetlb.c.
 - Fix building errors with adding dummy functions for !CONFIG_HUGETLB_PAGE.

Changes from v1:
 - Add acked tag from Mike.
 - Update some commit message.
 - Add VM_BUG_ON in try_to_unmap() for hugetlb case.
 - Add an explict void casting for huge_ptep_clear_flush() in hugetlb.c.

Baolin Wang (3):
  mm: change huge_ptep_clear_flush() to return the original pte
  mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
  mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when unmapping

 arch/arm64/include/asm/hugetlb.h   |  4 +--
 arch/arm64/mm/hugetlbpage.c        | 12 +++-----
 arch/ia64/include/asm/hugetlb.h    |  4 +--
 arch/mips/include/asm/hugetlb.h    |  9 ++++--
 arch/parisc/include/asm/hugetlb.h  |  4 +--
 arch/powerpc/include/asm/hugetlb.h |  9 ++++--
 arch/s390/include/asm/hugetlb.h    |  6 ++--
 arch/sh/include/asm/hugetlb.h      |  4 +--
 arch/sparc/include/asm/hugetlb.h   |  4 +--
 include/asm-generic/hugetlb.h      |  4 +--
 include/linux/hugetlb.h            | 11 +++++++
 mm/rmap.c                          | 63 ++++++++++++++++++++++++--------------
 12 files changed, 83 insertions(+), 51 deletions(-)

Comments

Andrew Morton May 10, 2022, 4:04 a.m. UTC | #1

On Tue, 10 May 2022 11:45:57 +0800 Baolin Wang <[e-mail]> wrote:

> Hi,
> 
> Now migrating a hugetlb page or unmapping a poisoned hugetlb page, we'll
> use ptep_clear_flush() and set_pte_at() to nuke the page table entry
> and remap it, and this is incorrect for CONT-PTE or CONT-PMD size hugetlb
> page,

It would be helpful to describe why it's wrong.  Something like "should
use huge_ptep_clear_flush() and huge_ptep_clear_flush() for this
purpose"?

> which will cause potential data consistent issue. This patch set
> will change to use hugetlb related APIs to fix this issue, please find
> details in each patch. Thanks.

Is a cc:stable needed here?  And are we able to identify a target for a
Fixes: tag?

Baolin Wang May 10, 2022, 4:26 a.m. UTC | #2

On 5/10/2022 12:04 PM, Andrew Morton wrote:
> On Tue, 10 May 2022 11:45:57 +0800 Baolin Wang <[e-mail]> wrote:
> 
>> Hi,
>>
>> Now migrating a hugetlb page or unmapping a poisoned hugetlb page, we'll
>> use ptep_clear_flush() and set_pte_at() to nuke the page table entry
>> and remap it, and this is incorrect for CONT-PTE or CONT-PMD size hugetlb
>> page,
> 
> It would be helpful to describe why it's wrong.  Something like "should
> use huge_ptep_clear_flush() and huge_ptep_clear_flush() for this
> purpose"?

Sorry for the confusing description. I described the problem explicitly 
in each patch's commit message.

https://lore.kernel.org/all/ea5[e-mail]/
https://lore.kernel.org/all/730[e-mail]/

> 
>> which will cause potential data consistent issue. This patch set
>> will change to use hugetlb related APIs to fix this issue, please find
>> details in each patch. Thanks.
> 
> Is a cc:stable needed here?  And are we able to identify a target for a
> Fixes: tag?

I think need a cc:stable tag, however I am not sure the target fixes 
tag, since we should trace back to the introduction of CONT-PTE/PMD 
hugetlb? 66b3923a1a0f ("arm64: hugetlb: add support for PTE contiguous bit")