Re: [PATCH v2 2/2] mm: fix missing cache flush for all tail pages of THP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/24/22 22:01, Muchun Song wrote:
> On Tue, Jan 25, 2022 at 10:42 AM Zi Yan <ziy@xxxxxxxxxx> wrote:
>>
>> On 24 Jan 2022, at 20:55, Muchun Song wrote:
>>
>>> On Tue, Jan 25, 2022 at 3:22 AM Zi Yan <ziy@xxxxxxxxxx> wrote:
>>>>
>>>> On 24 Jan 2022, at 13:11, David Rientjes wrote:
>>>>
>>>>> On Mon, 24 Jan 2022, Muchun Song wrote:
>>>>>
>>>>>> The D-cache maintenance inside move_to_new_page() only consider one page,
>>>>>> there is still D-cache maintenance issue for tail pages of THP. Fix this
>>>>>> by not using flush_dcache_folio() since it is not backportable.
>>>>>>
>>>>>
>>>>> The mention of being backportable suggests that we should backport this,
>>>>> likely to 4.14+.  So should it be marked as stable?
>>>>
>>>> Hmm, after more digging, I am not sure if the bug exists. For THP migration,
>>>> flush_cache_range() is used in remove_migration_pmd(). The flush_dcache_page()
>>>> was added by Lars Persson (cc’d) to solve the data corruption on MIPS[1],
>>>> but THP migration is only enabled on x86_64, PPC_BOOK3S_64, and ARM64.
>>>
>>> I only mention the THP case. After some more thinking, I think the HugeTLB
>>> should also be considered, Right? The HugeTLB is enabled on arm, arm64,
>>> mips, parisc, powerpc, riscv, s390 and sh.
>>>
>>
>> +Mike for HugeTLB
>>
>> If HugeTLB page migration also misses flush_dcache_page() on its tail pages,
>> you will need a different patch for the commit introducing hugetlb page migration.
> 
> Agree. I think arm (see the following commit) has handled this issue, while most
> others do not.
> 
>   commit 0b19f93351dd ("ARM: mm: Add support for flushing HugeTLB pages.")
> 
> But I do not have any real devices to test if this issue exists on other archs.
> In theory, it exists.
> 

Thanks for adding me to the discussion.

I agree that this issue exists at least in theory for hugetlb pages as well.
This made me look at other places with similar code for hugetlb.  i.e.
Allocating a new page, copying data to new page and then establishing a
mapping (pte) to the new page.

- hugetlb_cow calls copy_user_huge_page() which ends up calling
  copy_user_highpage that includes dcache flushing of the target for some
  architectures, but not all.
- userfaultfd calls copy_huge_page_from_user which does not appear to do
  any dcache flushing for the target page.

Do you think these code paths have the same potential issue?
-- 
Mike Kravetz





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux