* [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry()
@ 2018-10-12 16:09 jglisse
2018-10-12 16:20 ` Zi Yan
2018-10-12 16:55 ` Michal Hocko
0 siblings, 2 replies; 7+ messages in thread
From: jglisse @ 2018-10-12 16:09 UTC (permalink / raw)
To: linux-mm
Cc: Andrew Morton, linux-kernel, Jérôme Glisse,
Greg Kroah-Hartman, Zi Yan, Kirill A . Shutemov, H. Peter Anvin,
Anshuman Khandual, Dave Hansen, David Nellans, Ingo Molnar,
Mel Gorman, Minchan Kim, Naoya Horiguchi, Thomas Gleixner,
Vlastimil Babka, Michal Hocko, Andrea Arcangeli
From: Jérôme Glisse <jglisse@redhat.com>
Inside set_pmd_migration_entry() we are holding page table locks and
thus we can not sleep so we can not call invalidate_range_start/end()
So remove call to mmu_notifier_invalidate_range_start/end() and add
call to mmu_notifier_invalidate_range(). Note that we are already
calling mmu_notifier_invalidate_range_start/end() inside the function
calling set_pmd_migration_entry() (see try_to_unmap_one()).
Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
Reported-by: Andrea Arcangeli <aarcange@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Zi Yan <zi.yan@cs.rutgers.edu>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: David Nellans <dnellans@nvidia.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Andrea Arcangeli <aarcange@redhat.com>
---
mm/huge_memory.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 533f9b00147d..93cb80fe12cb 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2885,9 +2885,6 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
if (!(pvmw->pmd && !pvmw->pte))
return;
- mmu_notifier_invalidate_range_start(mm, address,
- address + HPAGE_PMD_SIZE);
-
flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
pmdval = *pvmw->pmd;
pmdp_invalidate(vma, address, pvmw->pmd);
@@ -2898,11 +2895,9 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
if (pmd_soft_dirty(pmdval))
pmdswp = pmd_swp_mksoft_dirty(pmdswp);
set_pmd_at(mm, address, pvmw->pmd, pmdswp);
+ mmu_notifier_invalidate_range(mm, address, address + HPAGE_PMD_SIZE);
page_remove_rmap(page, true);
put_page(page);
-
- mmu_notifier_invalidate_range_end(mm, address,
- address + HPAGE_PMD_SIZE);
}
void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)
--
2.17.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry()
2018-10-12 16:09 [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry() jglisse
@ 2018-10-12 16:20 ` Zi Yan
2018-10-12 17:24 ` Andrea Arcangeli
2018-10-12 16:55 ` Michal Hocko
1 sibling, 1 reply; 7+ messages in thread
From: Zi Yan @ 2018-10-12 16:20 UTC (permalink / raw)
To: jglisse
Cc: linux-mm, Andrew Morton, linux-kernel, Greg Kroah-Hartman,
Kirill A . Shutemov, H. Peter Anvin, Anshuman Khandual,
Dave Hansen, David Nellans, Ingo Molnar, Mel Gorman, Minchan Kim,
Naoya Horiguchi, Thomas Gleixner, Vlastimil Babka, Michal Hocko,
Andrea Arcangeli
[-- Attachment #1: Type: text/plain, Size: 2819 bytes --]
On 12 Oct 2018, at 12:09, jglisse@redhat.com wrote:
> From: Jérôme Glisse <jglisse@redhat.com>
>
> Inside set_pmd_migration_entry() we are holding page table locks and
> thus we can not sleep so we can not call invalidate_range_start/end()
>
> So remove call to mmu_notifier_invalidate_range_start/end() and add
> call to mmu_notifier_invalidate_range(). Note that we are already
> calling mmu_notifier_invalidate_range_start/end() inside the function
> calling set_pmd_migration_entry() (see try_to_unmap_one()).
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Reported-by: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Zi Yan <zi.yan@cs.rutgers.edu>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: David Nellans <dnellans@nvidia.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> ---
> mm/huge_memory.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 533f9b00147d..93cb80fe12cb 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2885,9 +2885,6 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> if (!(pvmw->pmd && !pvmw->pte))
> return;
>
> - mmu_notifier_invalidate_range_start(mm, address,
> - address + HPAGE_PMD_SIZE);
> -
> flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
> pmdval = *pvmw->pmd;
> pmdp_invalidate(vma, address, pvmw->pmd);
> @@ -2898,11 +2895,9 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> if (pmd_soft_dirty(pmdval))
> pmdswp = pmd_swp_mksoft_dirty(pmdswp);
> set_pmd_at(mm, address, pvmw->pmd, pmdswp);
> + mmu_notifier_invalidate_range(mm, address, address + HPAGE_PMD_SIZE);
> page_remove_rmap(page, true);
> put_page(page);
> -
> - mmu_notifier_invalidate_range_end(mm, address,
> - address + HPAGE_PMD_SIZE);
> }
>
> void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)
> --
> 2.17.2
Yes, these are the redundant calls to mmu_notifier_invalidate_range_start/end()
in set_pmd_migration_entry(). Thanks for the patch.
Fixes: 616b8371539a6 (mm: thp: enable thp migration in generic path)
Reviewed-by: Zi Yan <zi.yan@cs.rutgers.edu>
—
Best Regards,
Yan Zi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry()
2018-10-12 16:09 [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry() jglisse
2018-10-12 16:20 ` Zi Yan
@ 2018-10-12 16:55 ` Michal Hocko
2018-10-12 17:05 ` Jerome Glisse
1 sibling, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2018-10-12 16:55 UTC (permalink / raw)
To: jglisse
Cc: linux-mm, Andrew Morton, linux-kernel, Greg Kroah-Hartman,
Zi Yan, Kirill A . Shutemov, H. Peter Anvin, Anshuman Khandual,
Dave Hansen, David Nellans, Ingo Molnar, Mel Gorman, Minchan Kim,
Naoya Horiguchi, Thomas Gleixner, Vlastimil Babka,
Andrea Arcangeli
On Fri 12-10-18 12:09:53, jglisse@redhat.com wrote:
> From: Jérôme Glisse <jglisse@redhat.com>
>
> Inside set_pmd_migration_entry() we are holding page table locks and
> thus we can not sleep so we can not call invalidate_range_start/end()
>
> So remove call to mmu_notifier_invalidate_range_start/end() and add
> call to mmu_notifier_invalidate_range(). Note that we are already
> calling mmu_notifier_invalidate_range_start/end() inside the function
> calling set_pmd_migration_entry() (see try_to_unmap_one()).
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Reported-by: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Zi Yan <zi.yan@cs.rutgers.edu>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: David Nellans <dnellans@nvidia.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
Is this worth backporting to stable trees?
The patch looks good to me
Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> mm/huge_memory.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 533f9b00147d..93cb80fe12cb 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2885,9 +2885,6 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> if (!(pvmw->pmd && !pvmw->pte))
> return;
>
> - mmu_notifier_invalidate_range_start(mm, address,
> - address + HPAGE_PMD_SIZE);
> -
> flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
> pmdval = *pvmw->pmd;
> pmdp_invalidate(vma, address, pvmw->pmd);
> @@ -2898,11 +2895,9 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> if (pmd_soft_dirty(pmdval))
> pmdswp = pmd_swp_mksoft_dirty(pmdswp);
> set_pmd_at(mm, address, pvmw->pmd, pmdswp);
> + mmu_notifier_invalidate_range(mm, address, address + HPAGE_PMD_SIZE);
> page_remove_rmap(page, true);
> put_page(page);
> -
> - mmu_notifier_invalidate_range_end(mm, address,
> - address + HPAGE_PMD_SIZE);
> }
>
> void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)
> --
> 2.17.2
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry()
2018-10-12 16:55 ` Michal Hocko
@ 2018-10-12 17:05 ` Jerome Glisse
0 siblings, 0 replies; 7+ messages in thread
From: Jerome Glisse @ 2018-10-12 17:05 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, linux-kernel, stable,
Greg Kroah-Hartman, Zi Yan, Kirill A . Shutemov, H. Peter Anvin,
Anshuman Khandual, Dave Hansen, David Nellans, Ingo Molnar,
Mel Gorman, Minchan Kim, Naoya Horiguchi, Thomas Gleixner,
Vlastimil Babka, Andrea Arcangeli
On Fri, Oct 12, 2018 at 06:55:48PM +0200, Michal Hocko wrote:
> On Fri 12-10-18 12:09:53, jglisse@redhat.com wrote:
> > From: Jérôme Glisse <jglisse@redhat.com>
> >
> > Inside set_pmd_migration_entry() we are holding page table locks and
> > thus we can not sleep so we can not call invalidate_range_start/end()
> >
> > So remove call to mmu_notifier_invalidate_range_start/end() and add
> > call to mmu_notifier_invalidate_range(). Note that we are already
> > calling mmu_notifier_invalidate_range_start/end() inside the function
> > calling set_pmd_migration_entry() (see try_to_unmap_one()).
> >
> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > Reported-by: Andrea Arcangeli <aarcange@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Zi Yan <zi.yan@cs.rutgers.edu>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> > Cc: Dave Hansen <dave.hansen@intel.com>
> > Cc: David Nellans <dnellans@nvidia.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Mel Gorman <mgorman@techsingularity.net>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Michal Hocko <mhocko@kernel.org>
> > Cc: Andrea Arcangeli <aarcange@redhat.com>
>
> Is this worth backporting to stable trees?
Yes it is i forgot to cc stable :(
>
> The patch looks good to me
> Acked-by: Michal Hocko <mhocko@suse.com>
>
> > ---
> > mm/huge_memory.c | 7 +------
> > 1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 533f9b00147d..93cb80fe12cb 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -2885,9 +2885,6 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> > if (!(pvmw->pmd && !pvmw->pte))
> > return;
> >
> > - mmu_notifier_invalidate_range_start(mm, address,
> > - address + HPAGE_PMD_SIZE);
> > -
> > flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
> > pmdval = *pvmw->pmd;
> > pmdp_invalidate(vma, address, pvmw->pmd);
> > @@ -2898,11 +2895,9 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> > if (pmd_soft_dirty(pmdval))
> > pmdswp = pmd_swp_mksoft_dirty(pmdswp);
> > set_pmd_at(mm, address, pvmw->pmd, pmdswp);
> > + mmu_notifier_invalidate_range(mm, address, address + HPAGE_PMD_SIZE);
> > page_remove_rmap(page, true);
> > put_page(page);
> > -
> > - mmu_notifier_invalidate_range_end(mm, address,
> > - address + HPAGE_PMD_SIZE);
> > }
> >
> > void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)
> > --
> > 2.17.2
>
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry()
2018-10-12 16:20 ` Zi Yan
@ 2018-10-12 17:24 ` Andrea Arcangeli
2018-10-12 17:35 ` Jerome Glisse
0 siblings, 1 reply; 7+ messages in thread
From: Andrea Arcangeli @ 2018-10-12 17:24 UTC (permalink / raw)
To: Zi Yan
Cc: jglisse, linux-mm, Andrew Morton, linux-kernel,
Greg Kroah-Hartman, Kirill A . Shutemov, H. Peter Anvin,
Anshuman Khandual, Dave Hansen, David Nellans, Ingo Molnar,
Mel Gorman, Minchan Kim, Naoya Horiguchi, Thomas Gleixner,
Vlastimil Babka, Michal Hocko
Hello,
On Fri, Oct 12, 2018 at 12:20:54PM -0400, Zi Yan wrote:
> On 12 Oct 2018, at 12:09, jglisse@redhat.com wrote:
>
> > From: Jérôme Glisse <jglisse@redhat.com>
> >
> > Inside set_pmd_migration_entry() we are holding page table locks and
> > thus we can not sleep so we can not call invalidate_range_start/end()
> >
> > So remove call to mmu_notifier_invalidate_range_start/end() and add
> > call to mmu_notifier_invalidate_range(). Note that we are already
Why the call to mmu_notifier_invalidate_range if we're under
range_start and followed by range_end? (it's not _range_only_end, if
it was _range_only_end the above would be needed)
> > calling mmu_notifier_invalidate_range_start/end() inside the function
> > calling set_pmd_migration_entry() (see try_to_unmap_one()).
> >
> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > Reported-by: Andrea Arcangeli <aarcange@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Zi Yan <zi.yan@cs.rutgers.edu>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> > Cc: Dave Hansen <dave.hansen@intel.com>
> > Cc: David Nellans <dnellans@nvidia.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Mel Gorman <mgorman@techsingularity.net>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Michal Hocko <mhocko@kernel.org>
> > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > ---
> > mm/huge_memory.c | 7 +------
> > 1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 533f9b00147d..93cb80fe12cb 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -2885,9 +2885,6 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> > if (!(pvmw->pmd && !pvmw->pte))
> > return;
> >
> > - mmu_notifier_invalidate_range_start(mm, address,
> > - address + HPAGE_PMD_SIZE);
> > -
> > flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
> > pmdval = *pvmw->pmd;
> > pmdp_invalidate(vma, address, pvmw->pmd);
> > @@ -2898,11 +2895,9 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> > if (pmd_soft_dirty(pmdval))
> > pmdswp = pmd_swp_mksoft_dirty(pmdswp);
> > set_pmd_at(mm, address, pvmw->pmd, pmdswp);
> > + mmu_notifier_invalidate_range(mm, address, address + HPAGE_PMD_SIZE);
It's not obvious why it's needed, if it's needed maybe a comment can
be added.
> > page_remove_rmap(page, true);
> > put_page(page);
> > -
> > - mmu_notifier_invalidate_range_end(mm, address,
> > - address + HPAGE_PMD_SIZE);
> > }
> >
> > void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)
> > --
> > 2.17.2
>
> Yes, these are the redundant calls to mmu_notifier_invalidate_range_start/end()
> in set_pmd_migration_entry(). Thanks for the patch.
They're not just redundant, it's called in non blockable path with
__mmu_notifier_invalidate_range_start(blockable=true).
Furthermore mmu notifier API doesn't support nesting.
KVM is actually robust against the nesting:
kvm->mmu_notifier_count++;
kvm->mmu_notifier_count--;
and KVM is always fine with non blockable calls, but that's not
universally true for all mmu notifier users.
Thanks,
Andrea
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry()
2018-10-12 17:24 ` Andrea Arcangeli
@ 2018-10-12 17:35 ` Jerome Glisse
2018-10-12 17:58 ` Andrea Arcangeli
0 siblings, 1 reply; 7+ messages in thread
From: Jerome Glisse @ 2018-10-12 17:35 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Zi Yan, linux-mm, Andrew Morton, linux-kernel,
Greg Kroah-Hartman, Kirill A . Shutemov, H. Peter Anvin,
Anshuman Khandual, Dave Hansen, David Nellans, Ingo Molnar,
Mel Gorman, Minchan Kim, Naoya Horiguchi, Thomas Gleixner,
Vlastimil Babka, Michal Hocko
On Fri, Oct 12, 2018 at 01:24:22PM -0400, Andrea Arcangeli wrote:
> Hello,
>
> On Fri, Oct 12, 2018 at 12:20:54PM -0400, Zi Yan wrote:
> > On 12 Oct 2018, at 12:09, jglisse@redhat.com wrote:
> >
> > > From: Jérôme Glisse <jglisse@redhat.com>
> > >
> > > Inside set_pmd_migration_entry() we are holding page table locks and
> > > thus we can not sleep so we can not call invalidate_range_start/end()
> > >
> > > So remove call to mmu_notifier_invalidate_range_start/end() and add
> > > call to mmu_notifier_invalidate_range(). Note that we are already
>
> Why the call to mmu_notifier_invalidate_range if we're under
> range_start and followed by range_end? (it's not _range_only_end, if
> it was _range_only_end the above would be needed)
I wanted to be extra safe and accept to over invalidate. You are right
that it is not strictly necessary. I am fine with removing it.
>
> > > calling mmu_notifier_invalidate_range_start/end() inside the function
> > > calling set_pmd_migration_entry() (see try_to_unmap_one()).
> > >
> > > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > > Reported-by: Andrea Arcangeli <aarcange@redhat.com>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Zi Yan <zi.yan@cs.rutgers.edu>
> > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > > Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> > > Cc: Dave Hansen <dave.hansen@intel.com>
> > > Cc: David Nellans <dnellans@nvidia.com>
> > > Cc: Ingo Molnar <mingo@elte.hu>
> > > Cc: Mel Gorman <mgorman@techsingularity.net>
> > > Cc: Minchan Kim <minchan@kernel.org>
> > > Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Vlastimil Babka <vbabka@suse.cz>
> > > Cc: Michal Hocko <mhocko@kernel.org>
> > > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > > ---
> > > mm/huge_memory.c | 7 +------
> > > 1 file changed, 1 insertion(+), 6 deletions(-)
> > >
> > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > > index 533f9b00147d..93cb80fe12cb 100644
> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -2885,9 +2885,6 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> > > if (!(pvmw->pmd && !pvmw->pte))
> > > return;
> > >
> > > - mmu_notifier_invalidate_range_start(mm, address,
> > > - address + HPAGE_PMD_SIZE);
> > > -
> > > flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
> > > pmdval = *pvmw->pmd;
> > > pmdp_invalidate(vma, address, pvmw->pmd);
> > > @@ -2898,11 +2895,9 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> > > if (pmd_soft_dirty(pmdval))
> > > pmdswp = pmd_swp_mksoft_dirty(pmdswp);
> > > set_pmd_at(mm, address, pvmw->pmd, pmdswp);
> > > + mmu_notifier_invalidate_range(mm, address, address + HPAGE_PMD_SIZE);
>
> It's not obvious why it's needed, if it's needed maybe a comment can
> be added.
We can remove it. Should i post a v2 without it ?
Cheers,
Jérôme
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry()
2018-10-12 17:35 ` Jerome Glisse
@ 2018-10-12 17:58 ` Andrea Arcangeli
0 siblings, 0 replies; 7+ messages in thread
From: Andrea Arcangeli @ 2018-10-12 17:58 UTC (permalink / raw)
To: Jerome Glisse
Cc: Zi Yan, linux-mm, Andrew Morton, linux-kernel,
Greg Kroah-Hartman, Kirill A . Shutemov, H. Peter Anvin,
Anshuman Khandual, Dave Hansen, David Nellans, Ingo Molnar,
Mel Gorman, Minchan Kim, Naoya Horiguchi, Thomas Gleixner,
Vlastimil Babka, Michal Hocko
On Fri, Oct 12, 2018 at 01:35:19PM -0400, Jerome Glisse wrote:
> On Fri, Oct 12, 2018 at 01:24:22PM -0400, Andrea Arcangeli wrote:
> > Hello,
> >
> > On Fri, Oct 12, 2018 at 12:20:54PM -0400, Zi Yan wrote:
> > > On 12 Oct 2018, at 12:09, jglisse@redhat.com wrote:
> > >
> > > > From: Jérôme Glisse <jglisse@redhat.com>
> > > >
> > > > Inside set_pmd_migration_entry() we are holding page table locks and
> > > > thus we can not sleep so we can not call invalidate_range_start/end()
> > > >
> > > > So remove call to mmu_notifier_invalidate_range_start/end() and add
> > > > call to mmu_notifier_invalidate_range(). Note that we are already
> >
> > Why the call to mmu_notifier_invalidate_range if we're under
> > range_start and followed by range_end? (it's not _range_only_end, if
> > it was _range_only_end the above would be needed)
>
> I wanted to be extra safe and accept to over invalidate. You are right
> that it is not strictly necessary. I am fine with removing it.
If it's superfluous, I'd generally prefer strict code unless there's a
very explicit comment about it that says it's actually superfluous.
Otherwise after a while we don't know why it was added there.
> We can remove it. Should i post a v2 without it ?
That's fine with me yes.
Thanks,
Andrea
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-10-12 17:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12 16:09 [PATCH] mm/thp: fix call to mmu_notifier in set_pmd_migration_entry() jglisse
2018-10-12 16:20 ` Zi Yan
2018-10-12 17:24 ` Andrea Arcangeli
2018-10-12 17:35 ` Jerome Glisse
2018-10-12 17:58 ` Andrea Arcangeli
2018-10-12 16:55 ` Michal Hocko
2018-10-12 17:05 ` Jerome Glisse
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).