Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On Thu, May 20, 2021 at 4:01 PM Peter Xu wrote: > > On Thu, May 20, 2021 at 03:06:30PM -0400, Zi Yan wrote: > > On 20 May 2021, at 10:57, Peter Xu wrote: > > > > > On Thu, May 20, 2021 at 07:07:57PM +0530, Aneesh Kumar K.V wrote: > > >> "Aneesh Kumar K.V" writes: > > >> > > >>> On 5/20/21 6:16 PM, Peter Xu wrote: > > On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote: > > >> This seems to work at least for my userfaultfd test on shmem, > > >> however I don't > > >> fully understand the commit message [1] on: How do we guarantee > > >> we're not > > >> moving a thp pte? > > >> > > > > > > move_page_tables() checks for pmd_trans_huge() and ends up calling > > > move_huge_pmd if it is a THP entry. > > > > Sorry to be unclear: what if a huge pud thp? > > > > >>> > > >>> I am still checking. Looking at the code before commit > > >>> c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling > > >>> huge pud thp. I haven't studied huge pud thp enough to understand > > >>> whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that > > >>> support. > > >>> > > >>> We can do a move_huge_pud() like we do for huge pmd thp. But I am not > > >>> sure whether we handle those VMA's earlier and restrict mremap on them? > > >> > > >> something like this? (not even compile tested). I am still not sure > > >> whether this is really needed or we handle DAX VMA's in some other form. > > > > > > Yeah maybe (you may want to at least drop that extra "case HPAGE_PUD"). > > > > > > It's just that if with CONFIG_HAVE_MOVE_PUD (x86 and arm64 enables it by > > > default so far) it does seem to work even with huge pud, while after this > > > patch > > > it seems to be not working anymore, even with your follow up fix. > > > > > > Indeed I saw CONFIG_HAVE_MOVE_PUD is introduced a few months ago so > > > breaking > > > someone seems to be unlikely, perhaps no real user yet to mremap() a huge > > > pud > > > for dax or whatever backend? > > > > > > Ideally maybe rework this patch (or series?) and repost it for a better > > > review? > > > Agree the risk seems low. I'll leave that to you and Andrew to decide.. > > > > It seems that the mremap function for 1GB DAX THP was not added when 1GB > > DAX THP > > was implemented[1]. > > Yes, but trickily as I mentioned it seems Android's CONFIG_HAVE_MOVE_PUD has > done this right (with no intention I guess) with the set_pud_at() before this > patch is merged, so we might have a short period that this might start to > work.. > It may have coincidentally handled the huge PUD case, but I hadn't considered huge PUDs when implementing the HAVE_MOVE_PUD patchset. Or as Zi suggested, huge PUD mremap may be unused atm, I haven't seen any related breakages since enabling HAVE_MOVE_PUD for x86 and arm64 > > I guess no one is using mremap on 1GB DAX THP. Maybe we want > > to at least add a warning or VM_BUG_ON to catch this or use Aneesh’s > > move_huge_pud() > > to handle the situation properly? > > Agreed, if we decide to go with the patches, some warning (or even VM_BUG_ON, > which iiuc should be very not-suggested in most cases) looks better than > pgtable corruption reports. > > -- > Peter Xu >
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On Thu, May 20, 2021 at 03:06:30PM -0400, Zi Yan wrote: > On 20 May 2021, at 10:57, Peter Xu wrote: > > > On Thu, May 20, 2021 at 07:07:57PM +0530, Aneesh Kumar K.V wrote: > >> "Aneesh Kumar K.V" writes: > >> > >>> On 5/20/21 6:16 PM, Peter Xu wrote: > On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote: > >> This seems to work at least for my userfaultfd test on shmem, however > >> I don't > >> fully understand the commit message [1] on: How do we guarantee we're > >> not > >> moving a thp pte? > >> > > > > move_page_tables() checks for pmd_trans_huge() and ends up calling > > move_huge_pmd if it is a THP entry. > > Sorry to be unclear: what if a huge pud thp? > > >>> > >>> I am still checking. Looking at the code before commit > >>> c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling > >>> huge pud thp. I haven't studied huge pud thp enough to understand > >>> whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that > >>> support. > >>> > >>> We can do a move_huge_pud() like we do for huge pmd thp. But I am not > >>> sure whether we handle those VMA's earlier and restrict mremap on them? > >> > >> something like this? (not even compile tested). I am still not sure > >> whether this is really needed or we handle DAX VMA's in some other form. > > > > Yeah maybe (you may want to at least drop that extra "case HPAGE_PUD"). > > > > It's just that if with CONFIG_HAVE_MOVE_PUD (x86 and arm64 enables it by > > default so far) it does seem to work even with huge pud, while after this > > patch > > it seems to be not working anymore, even with your follow up fix. > > > > Indeed I saw CONFIG_HAVE_MOVE_PUD is introduced a few months ago so breaking > > someone seems to be unlikely, perhaps no real user yet to mremap() a huge > > pud > > for dax or whatever backend? > > > > Ideally maybe rework this patch (or series?) and repost it for a better > > review? > > Agree the risk seems low. I'll leave that to you and Andrew to decide.. > > It seems that the mremap function for 1GB DAX THP was not added when 1GB DAX > THP > was implemented[1]. Yes, but trickily as I mentioned it seems Android's CONFIG_HAVE_MOVE_PUD has done this right (with no intention I guess) with the set_pud_at() before this patch is merged, so we might have a short period that this might start to work.. > I guess no one is using mremap on 1GB DAX THP. Maybe we want > to at least add a warning or VM_BUG_ON to catch this or use Aneesh’s > move_huge_pud() > to handle the situation properly? Agreed, if we decide to go with the patches, some warning (or even VM_BUG_ON, which iiuc should be very not-suggested in most cases) looks better than pgtable corruption reports. -- Peter Xu
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On 20 May 2021, at 10:57, Peter Xu wrote: > On Thu, May 20, 2021 at 07:07:57PM +0530, Aneesh Kumar K.V wrote: >> "Aneesh Kumar K.V" writes: >> >>> On 5/20/21 6:16 PM, Peter Xu wrote: On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote: >> This seems to work at least for my userfaultfd test on shmem, however I >> don't >> fully understand the commit message [1] on: How do we guarantee we're not >> moving a thp pte? >> > > move_page_tables() checks for pmd_trans_huge() and ends up calling > move_huge_pmd if it is a THP entry. Sorry to be unclear: what if a huge pud thp? >>> >>> I am still checking. Looking at the code before commit >>> c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling >>> huge pud thp. I haven't studied huge pud thp enough to understand >>> whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that >>> support. >>> >>> We can do a move_huge_pud() like we do for huge pmd thp. But I am not >>> sure whether we handle those VMA's earlier and restrict mremap on them? >> >> something like this? (not even compile tested). I am still not sure >> whether this is really needed or we handle DAX VMA's in some other form. > > Yeah maybe (you may want to at least drop that extra "case HPAGE_PUD"). > > It's just that if with CONFIG_HAVE_MOVE_PUD (x86 and arm64 enables it by > default so far) it does seem to work even with huge pud, while after this > patch > it seems to be not working anymore, even with your follow up fix. > > Indeed I saw CONFIG_HAVE_MOVE_PUD is introduced a few months ago so breaking > someone seems to be unlikely, perhaps no real user yet to mremap() a huge pud > for dax or whatever backend? > > Ideally maybe rework this patch (or series?) and repost it for a better > review? > Agree the risk seems low. I'll leave that to you and Andrew to decide.. It seems that the mremap function for 1GB DAX THP was not added when 1GB DAX THP was implemented[1]. I guess no one is using mremap on 1GB DAX THP. Maybe we want to at least add a warning or VM_BUG_ON to catch this or use Aneesh’s move_huge_pud() to handle the situation properly? [1] https://lore.kernel.org/linux-ext4/148545012634.17912.13951763606410303827.st...@djiang5-desk3.ch.intel.com/ — Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On Thu, May 20, 2021 at 07:07:57PM +0530, Aneesh Kumar K.V wrote: > "Aneesh Kumar K.V" writes: > > > On 5/20/21 6:16 PM, Peter Xu wrote: > >> On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote: > This seems to work at least for my userfaultfd test on shmem, however I > don't > fully understand the commit message [1] on: How do we guarantee we're not > moving a thp pte? > > >>> > >>> move_page_tables() checks for pmd_trans_huge() and ends up calling > >>> move_huge_pmd if it is a THP entry. > >> > >> Sorry to be unclear: what if a huge pud thp? > >> > > > > I am still checking. Looking at the code before commit > > c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling > > huge pud thp. I haven't studied huge pud thp enough to understand > > whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that > > support. > > > > We can do a move_huge_pud() like we do for huge pmd thp. But I am not > > sure whether we handle those VMA's earlier and restrict mremap on them? > > something like this? (not even compile tested). I am still not sure > whether this is really needed or we handle DAX VMA's in some other form. Yeah maybe (you may want to at least drop that extra "case HPAGE_PUD"). It's just that if with CONFIG_HAVE_MOVE_PUD (x86 and arm64 enables it by default so far) it does seem to work even with huge pud, while after this patch it seems to be not working anymore, even with your follow up fix. Indeed I saw CONFIG_HAVE_MOVE_PUD is introduced a few months ago so breaking someone seems to be unlikely, perhaps no real user yet to mremap() a huge pud for dax or whatever backend? Ideally maybe rework this patch (or series?) and repost it for a better review? Agree the risk seems low. I'll leave that to you and Andrew to decide.. -- Peter Xu
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
"Aneesh Kumar K.V" writes:
> On 5/20/21 6:16 PM, Peter Xu wrote:
>> On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote:
This seems to work at least for my userfaultfd test on shmem, however I
don't
fully understand the commit message [1] on: How do we guarantee we're not
moving a thp pte?
>>>
>>> move_page_tables() checks for pmd_trans_huge() and ends up calling
>>> move_huge_pmd if it is a THP entry.
>>
>> Sorry to be unclear: what if a huge pud thp?
>>
>
> I am still checking. Looking at the code before commit
> c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling
> huge pud thp. I haven't studied huge pud thp enough to understand
> whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that
> support.
>
> We can do a move_huge_pud() like we do for huge pmd thp. But I am not
> sure whether we handle those VMA's earlier and restrict mremap on them?
something like this? (not even compile tested). I am still not sure
whether this is really needed or we handle DAX VMA's in some other form.
diff --git a/mm/mremap.c b/mm/mremap.c
index 47c255b60150..037a7bd311f1 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -324,10 +324,51 @@ static inline bool move_normal_pud(struct vm_area_struct
*vma,
}
#endif
+
+static bool move_huge_pud(struct vm_area_struct *vma, unsigned long old_addr,
+ unsigned long new_addr, pud_t *old_pud, pud_t
*new_pud)
+{
+ spinlock_t *old_ptl, *new_ptl;
+ struct mm_struct *mm = vma->vm_mm;
+ pud_t pud;
+
+ /*
+* The destination pud shouldn't be established, free_pgtables()
+* should have released it.
+*/
+ if (WARN_ON_ONCE(!pud_none(*new_pud)))
+ return false;
+
+ /*
+* We don't have to worry about the ordering of src and dst
+* ptlocks because exclusive mmap_lock prevents deadlock.
+*/
+ old_ptl = pud_lock(vma->vm_mm, old_pud);
+ new_ptl = pud_lockptr(mm, new_pud);
+ if (new_ptl != old_ptl)
+ spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
+
+ /* Clear the pud */
+ pud = *old_pud;
+ pud_clear(old_pud);
+
+ VM_BUG_ON(!pud_none(*new_pud));
+
+ /* Set the new pud */
+ set_pud_at(mm, new_addr, new_pud, pud);
+ flush_pud_tlb_range(vma, old_addr, old_addr + HPAGE_PUD_SIZE);
+ if (new_ptl != old_ptl)
+ spin_unlock(new_ptl);
+ spin_unlock(old_ptl);
+
+ return true;
+}
+
enum pgt_entry {
NORMAL_PMD,
HPAGE_PMD,
NORMAL_PUD,
+ HPAGE_PUD,
};
/*
@@ -347,6 +388,7 @@ static __always_inline unsigned long get_extent(enum
pgt_entry entry,
mask = PMD_MASK;
size = PMD_SIZE;
break;
+ case HPAGE_PUD:
case NORMAL_PUD:
mask = PUD_MASK;
size = PUD_SIZE;
@@ -395,6 +437,12 @@ static bool move_pgt_entry(enum pgt_entry entry, struct
vm_area_struct *vma,
move_huge_pmd(vma, old_addr, new_addr, old_entry,
new_entry);
break;
+ case HPAGE_PUD:
+ moved = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE_PUD) &&
+ move_huge_pud(vma, old_addr, new_addr, old_entry,
+ new_entry);
+ break;
+
default:
WARN_ON_ONCE(1);
break;
@@ -429,15 +477,23 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
* PUD level if possible.
*/
extent = get_extent(NORMAL_PUD, old_addr, old_end, new_addr);
- if (IS_ENABLED(CONFIG_HAVE_MOVE_PUD) && extent == PUD_SIZE) {
- pud_t *old_pud, *new_pud;
- old_pud = get_old_pud(vma->vm_mm, old_addr);
- if (!old_pud)
+ old_pud = get_old_pud(vma->vm_mm, old_addr);
+ if (!old_pud)
+ continue;
+ new_pud = alloc_new_pud(vma->vm_mm, vma, new_addr);
+ if (!new_pud)
+ break;
+ if (pud_trans_huge(*old_pud) || pud_devmap(*old_pud)) {
+ if (extent == HPAGE_PUD_SIZE) {
+ move_pgt_entry(HPAGE_PUD, vma, old_addr,
new_addr,
+ old_pud, new_pud,
need_rmap_locks);
+ /* We ignore and continue on error? */
continue;
- new_pud = alloc_new_pud(vma->vm_mm, vma, new_addr);
- if (!new_pud)
- break;
+ }
+ } else if (IS_ENABLED(CONFIG_HAVE_MOVE_PUD) && extent ==
PUD_SIZE) {
+ pud_t *old_pud, *new_pud;
+
if (move_pgt_entry(NORMAL_PUD, vma, old_addr,
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On 5/20/21 6:16 PM, Peter Xu wrote: On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote: This seems to work at least for my userfaultfd test on shmem, however I don't fully understand the commit message [1] on: How do we guarantee we're not moving a thp pte? move_page_tables() checks for pmd_trans_huge() and ends up calling move_huge_pmd if it is a THP entry. Sorry to be unclear: what if a huge pud thp? I am still checking. Looking at the code before commit c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling huge pud thp. I haven't studied huge pud thp enough to understand whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that support. We can do a move_huge_pud() like we do for huge pmd thp. But I am not sure whether we handle those VMA's earlier and restrict mremap on them? Are huge pud thp only allowed with DAX vmas? -aneesh
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote: > > This seems to work at least for my userfaultfd test on shmem, however I > > don't > > fully understand the commit message [1] on: How do we guarantee we're not > > moving a thp pte? > > > > move_page_tables() checks for pmd_trans_huge() and ends up calling > move_huge_pmd if it is a THP entry. Sorry to be unclear: what if a huge pud thp? -- Peter Xu
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On 5/20/21 7:48 AM, Peter Xu wrote: On Wed, May 19, 2021 at 10:16:07AM +0530, Aneesh Kumar K.V wrote: On Thu, Apr 22, 2021 at 11:13:17AM +0530, Aneesh Kumar K.V wrote: pmd/pud_populate is the right interface to be used to set the respective page table entries. Some architectures like ppc64 do assume that set_pmd/pud_at can only be used to set a hugepage PTE. Since we are not setting up a hugepage PTE here, use the pmd/pud_populate interface. [1] Can you try this change? modified mm/mremap.c @@ -279,7 +279,7 @@ static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr, pmd_clear(old_pmd); VM_BUG_ON(!pmd_none(*new_pmd)); - pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); + pmd_populate(mm, new_pmd, pmd_pgtable(pmd)); if (new_ptl != old_ptl) spin_unlock(new_ptl); I reported this issue today somewhere else: https://lore.kernel.org/linux-mm/YKVemB5DuSqLFmmz@t490s/ And came to this same line after the bisection. This seems to work at least for my userfaultfd test on shmem, however I don't fully understand the commit message [1] on: How do we guarantee we're not moving a thp pte? move_page_tables() checks for pmd_trans_huge() and ends up calling move_huge_pmd if it is a THP entry. -aneesh
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On Wed, May 19, 2021 at 10:16:07AM +0530, Aneesh Kumar K.V wrote: > > On Thu, Apr 22, 2021 at 11:13:17AM +0530, Aneesh Kumar K.V wrote: > >> pmd/pud_populate is the right interface to be used to set the respective > >> page table entries. Some architectures like ppc64 do assume that > >> set_pmd/pud_at > >> can only be used to set a hugepage PTE. Since we are not setting up a > >> hugepage > >> PTE here, use the pmd/pud_populate interface. [1] > Can you try this change? > > modified mm/mremap.c > @@ -279,7 +279,7 @@ static bool move_normal_pmd(struct vm_area_struct *vma, > unsigned long old_addr, > pmd_clear(old_pmd); > > VM_BUG_ON(!pmd_none(*new_pmd)); > - pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); > + pmd_populate(mm, new_pmd, pmd_pgtable(pmd)); > > if (new_ptl != old_ptl) > spin_unlock(new_ptl); I reported this issue today somewhere else: https://lore.kernel.org/linux-mm/YKVemB5DuSqLFmmz@t490s/ And came to this same line after the bisection. This seems to work at least for my userfaultfd test on shmem, however I don't fully understand the commit message [1] on: How do we guarantee we're not moving a thp pte? -- Peter Xu
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
On 5/18/2021 9:46 PM, Aneesh Kumar K.V wrote: Nathan Chancellor writes: Hi Aneesh, On Thu, Apr 22, 2021 at 11:13:17AM +0530, Aneesh Kumar K.V wrote: pmd/pud_populate is the right interface to be used to set the respective page table entries. Some architectures like ppc64 do assume that set_pmd/pud_at can only be used to set a hugepage PTE. Since we are not setting up a hugepage PTE here, use the pmd/pud_populate interface. Signed-off-by: Aneesh Kumar K.V --- mm/mremap.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/mm/mremap.c b/mm/mremap.c index ec8f840399ed..574287f9bb39 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -26,6 +26,7 @@ #include #include +#include #include "internal.h" @@ -257,9 +258,8 @@ static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr, pmd_clear(old_pmd); VM_BUG_ON(!pmd_none(*new_pmd)); + pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); - /* Set the new pmd */ - set_pmd_at(mm, new_addr, new_pmd, pmd); flush_tlb_range(vma, old_addr, old_addr + PMD_SIZE); if (new_ptl != old_ptl) spin_unlock(new_ptl); @@ -306,8 +306,7 @@ static bool move_normal_pud(struct vm_area_struct *vma, unsigned long old_addr, VM_BUG_ON(!pud_none(*new_pud)); - /* Set the new pud */ - set_pud_at(mm, new_addr, new_pud, pud); + pud_populate(mm, new_pud, (pmd_t *)pud_page_vaddr(pud)); flush_tlb_range(vma, old_addr, old_addr + PUD_SIZE); if (new_ptl != old_ptl) spin_unlock(new_ptl); -- 2.30.2 This commit causes my WSL2 VM to close when compiling something memory intensive, such as an x86_64_defconfig + CONFIG_LTO_CLANG_FULL=y kernel or LLVM/Clang. Unfortunately, I do not have much further information to provide since I do not see any sort of splat in dmesg right before it closes and I have found zero information about getting the previous kernel message in WSL2 (custom init so no systemd or anything). The config file is the stock one from Microsoft: https://github.com/microsoft/WSL2-Linux-Kernel/blob/a571dc8cedc8e0e56487c0dc93243e0b5db8960a/Microsoft/config-wsl I have attached my .config anyways, which includes CONFIG_DEBUG_VM, which does not appear to show anything out of the ordinary. I have also attached a dmesg just in case anything sticks out. I am happy to provide any additional information or perform additional debugging steps as needed. Can you try this change? Thank you for the quick diff! This resolves my issue. Tested-by: Nathan Chancellor modified mm/mremap.c @@ -279,7 +279,7 @@ static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr, pmd_clear(old_pmd); VM_BUG_ON(!pmd_none(*new_pmd)); - pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); + pmd_populate(mm, new_pmd, pmd_pgtable(pmd)); if (new_ptl != old_ptl) spin_unlock(new_ptl);
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
Nathan Chancellor writes: > Hi Aneesh, > > On Thu, Apr 22, 2021 at 11:13:17AM +0530, Aneesh Kumar K.V wrote: >> pmd/pud_populate is the right interface to be used to set the respective >> page table entries. Some architectures like ppc64 do assume that >> set_pmd/pud_at >> can only be used to set a hugepage PTE. Since we are not setting up a >> hugepage >> PTE here, use the pmd/pud_populate interface. >> >> Signed-off-by: Aneesh Kumar K.V >> --- >> mm/mremap.c | 7 +++ >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/mm/mremap.c b/mm/mremap.c >> index ec8f840399ed..574287f9bb39 100644 >> --- a/mm/mremap.c >> +++ b/mm/mremap.c >> @@ -26,6 +26,7 @@ >> >> #include >> #include >> +#include >> >> #include "internal.h" >> >> @@ -257,9 +258,8 @@ static bool move_normal_pmd(struct vm_area_struct *vma, >> unsigned long old_addr, >> pmd_clear(old_pmd); >> >> VM_BUG_ON(!pmd_none(*new_pmd)); >> +pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); >> >> -/* Set the new pmd */ >> -set_pmd_at(mm, new_addr, new_pmd, pmd); >> flush_tlb_range(vma, old_addr, old_addr + PMD_SIZE); >> if (new_ptl != old_ptl) >> spin_unlock(new_ptl); >> @@ -306,8 +306,7 @@ static bool move_normal_pud(struct vm_area_struct *vma, >> unsigned long old_addr, >> >> VM_BUG_ON(!pud_none(*new_pud)); >> >> -/* Set the new pud */ >> -set_pud_at(mm, new_addr, new_pud, pud); >> +pud_populate(mm, new_pud, (pmd_t *)pud_page_vaddr(pud)); >> flush_tlb_range(vma, old_addr, old_addr + PUD_SIZE); >> if (new_ptl != old_ptl) >> spin_unlock(new_ptl); >> -- >> 2.30.2 >> >> > > This commit causes my WSL2 VM to close when compiling something memory > intensive, such as an x86_64_defconfig + CONFIG_LTO_CLANG_FULL=y kernel > or LLVM/Clang. Unfortunately, I do not have much further information to > provide since I do not see any sort of splat in dmesg right before it > closes and I have found zero information about getting the previous > kernel message in WSL2 (custom init so no systemd or anything). > > The config file is the stock one from Microsoft: > > https://github.com/microsoft/WSL2-Linux-Kernel/blob/a571dc8cedc8e0e56487c0dc93243e0b5db8960a/Microsoft/config-wsl > > I have attached my .config anyways, which includes CONFIG_DEBUG_VM, > which does not appear to show anything out of the ordinary. I have also > attached a dmesg just in case anything sticks out. I am happy to provide > any additional information or perform additional debugging steps as > needed. > Can you try this change? modified mm/mremap.c @@ -279,7 +279,7 @@ static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr, pmd_clear(old_pmd); VM_BUG_ON(!pmd_none(*new_pmd)); - pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); + pmd_populate(mm, new_pmd, pmd_pgtable(pmd)); if (new_ptl != old_ptl) spin_unlock(new_ptl);
Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
Hi Aneesh, On Thu, Apr 22, 2021 at 11:13:17AM +0530, Aneesh Kumar K.V wrote: > pmd/pud_populate is the right interface to be used to set the respective > page table entries. Some architectures like ppc64 do assume that > set_pmd/pud_at > can only be used to set a hugepage PTE. Since we are not setting up a hugepage > PTE here, use the pmd/pud_populate interface. > > Signed-off-by: Aneesh Kumar K.V > --- > mm/mremap.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/mm/mremap.c b/mm/mremap.c > index ec8f840399ed..574287f9bb39 100644 > --- a/mm/mremap.c > +++ b/mm/mremap.c > @@ -26,6 +26,7 @@ > > #include > #include > +#include > > #include "internal.h" > > @@ -257,9 +258,8 @@ static bool move_normal_pmd(struct vm_area_struct *vma, > unsigned long old_addr, > pmd_clear(old_pmd); > > VM_BUG_ON(!pmd_none(*new_pmd)); > + pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); > > - /* Set the new pmd */ > - set_pmd_at(mm, new_addr, new_pmd, pmd); > flush_tlb_range(vma, old_addr, old_addr + PMD_SIZE); > if (new_ptl != old_ptl) > spin_unlock(new_ptl); > @@ -306,8 +306,7 @@ static bool move_normal_pud(struct vm_area_struct *vma, > unsigned long old_addr, > > VM_BUG_ON(!pud_none(*new_pud)); > > - /* Set the new pud */ > - set_pud_at(mm, new_addr, new_pud, pud); > + pud_populate(mm, new_pud, (pmd_t *)pud_page_vaddr(pud)); > flush_tlb_range(vma, old_addr, old_addr + PUD_SIZE); > if (new_ptl != old_ptl) > spin_unlock(new_ptl); > -- > 2.30.2 > > This commit causes my WSL2 VM to close when compiling something memory intensive, such as an x86_64_defconfig + CONFIG_LTO_CLANG_FULL=y kernel or LLVM/Clang. Unfortunately, I do not have much further information to provide since I do not see any sort of splat in dmesg right before it closes and I have found zero information about getting the previous kernel message in WSL2 (custom init so no systemd or anything). The config file is the stock one from Microsoft: https://github.com/microsoft/WSL2-Linux-Kernel/blob/a571dc8cedc8e0e56487c0dc93243e0b5db8960a/Microsoft/config-wsl I have attached my .config anyways, which includes CONFIG_DEBUG_VM, which does not appear to show anything out of the ordinary. I have also attached a dmesg just in case anything sticks out. I am happy to provide any additional information or perform additional debugging steps as needed. Cheers, Nathan $ git bisect log # bad: [cd557f1c605fc5a2c0eb0b540610f50dc67dd849] Add linux-next specific files for 20210514 # good: [315d99318179b9cd5077ccc9f7f26a164c9fa998] Merge tag 'pm-5.13-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm git bisect start 'cd557f1c605fc5a2c0eb0b540610f50dc67dd849' '315d99318179b9cd5077ccc9f7f26a164c9fa998' # good: [9634d7cb3c506ae886a5136d12b4af696b9cee8a] Merge remote-tracking branch 'drm-misc/for-linux-next' git bisect good 9634d7cb3c506ae886a5136d12b4af696b9cee8a # good: [294636a24ae819a7caf0807d05d8eb5b964ec06f] Merge remote-tracking branch 'rcu/rcu/next' git bisect good 294636a24ae819a7caf0807d05d8eb5b964ec06f # good: [cb753d0611f912439c8e814f4254d15fa8fa1d75] Merge remote-tracking branch 'gpio-brgl/gpio/for-next' git bisect good cb753d0611f912439c8e814f4254d15fa8fa1d75 # bad: [b1e7389449084b74a044a70860c6a1c7466781cb] lib/string_helpers: switch to use BIT() macro git bisect bad b1e7389449084b74a044a70860c6a1c7466781cb # bad: [bf5570ed0654a21000e5dad9243ea1ba30bfe208] kasan: use dump_stack_lvl(KERN_ERR) to print stacks git bisect bad bf5570ed0654a21000e5dad9243ea1ba30bfe208 # good: [4a292ff7a819404039588c7a9af272aca22c869e] fixup! mm: gup: pack has_pinned in MMF_HAS_PINNED git bisect good 4a292ff7a819404039588c7a9af272aca22c869e # good: [5ed68c90c7fb884c3c493d5529aca79dcf125848] mm: memcontrol: move obj_cgroup_uncharge_pages() out of css_set_lock git bisect good 5ed68c90c7fb884c3c493d5529aca79dcf125848 # good: [f96ae2c1e63b71134e216e9940df3f2793a9a4b1] mm/memory.c: fix comment of finish_mkwrite_fault() git bisect good f96ae2c1e63b71134e216e9940df3f2793a9a4b1 # bad: [5b0a28a7f9f5fdc2fe5a5e2cce7ea17b98e5eaeb] mm/mremap: use range flush that does TLB and page walk cache flush git bisect bad 5b0a28a7f9f5fdc2fe5a5e2cce7ea17b98e5eaeb # bad: [dbee97d1f49a2f2f1f5c26bf15151cc998572e89] mm/mremap: use pmd/pud_poplulate to update page table entries git bisect bad dbee97d1f49a2f2f1f5c26bf15151cc998572e89 # good: [c4c8a76d96a7d38d1ec8732e3f852418d18a7424] selftest/mremap_test: avoid crash with static build git bisect good c4c8a76d96a7d38d1ec8732e3f852418d18a7424 # first bad commit: [dbee97d1f49a2f2f1f5c26bf15151cc998572e89] mm/mremap: use pmd/pud_poplulate to update page table entries # # Automatically generated file; DO NOT EDIT. # Linux/x86 5.13.0-rc2 Kernel Configuration # CONFIG_CC_VERSION_TEXT="gcc (GCC) 11.1.0" CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=110100 CONFIG_CLANG_VERSION=0
[PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
pmd/pud_populate is the right interface to be used to set the respective page table entries. Some architectures like ppc64 do assume that set_pmd/pud_at can only be used to set a hugepage PTE. Since we are not setting up a hugepage PTE here, use the pmd/pud_populate interface. Signed-off-by: Aneesh Kumar K.V --- mm/mremap.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/mm/mremap.c b/mm/mremap.c index ec8f840399ed..574287f9bb39 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -26,6 +26,7 @@ #include #include +#include #include "internal.h" @@ -257,9 +258,8 @@ static bool move_normal_pmd(struct vm_area_struct *vma, unsigned long old_addr, pmd_clear(old_pmd); VM_BUG_ON(!pmd_none(*new_pmd)); + pmd_populate(mm, new_pmd, (pgtable_t)pmd_page_vaddr(pmd)); - /* Set the new pmd */ - set_pmd_at(mm, new_addr, new_pmd, pmd); flush_tlb_range(vma, old_addr, old_addr + PMD_SIZE); if (new_ptl != old_ptl) spin_unlock(new_ptl); @@ -306,8 +306,7 @@ static bool move_normal_pud(struct vm_area_struct *vma, unsigned long old_addr, VM_BUG_ON(!pud_none(*new_pud)); - /* Set the new pud */ - set_pud_at(mm, new_addr, new_pud, pud); + pud_populate(mm, new_pud, (pmd_t *)pud_page_vaddr(pud)); flush_tlb_range(vma, old_addr, old_addr + PUD_SIZE); if (new_ptl != old_ptl) spin_unlock(new_ptl); -- 2.30.2
