* Re: [PATCH 2/2] dax: Fix race between colliding PMD & PTE entries
From: Jan Kara @ 2017-05-22 14:44 UTC (permalink / raw)
To: Ross Zwisler
Cc: Andrew Morton, linux-kernel, Darrick J. Wong, Alexander Viro,
Christoph Hellwig, Dan Williams, Dave Hansen, Jan Kara,
Matthew Wilcox, linux-fsdevel, linux-mm, linux-nvdimm,
Kirill A . Shutemov, Pawel Lebioda, Dave Jiang, Xiong Zhou,
Eryu Guan, stable
In-Reply-To: <20170517171639.14501-2-ross.zwisler@linux.intel.com>
On Wed 17-05-17 11:16:39, Ross Zwisler wrote:
> We currently have two related PMD vs PTE races in the DAX code. These can
> both be easily triggered by having two threads reading and writing
> simultaneously to the same private mapping, with the key being that private
> mapping reads can be handled with PMDs but private mapping writes are
> always handled with PTEs so that we can COW.
>
> Here is the first race:
>
> CPU 0 CPU 1
>
> (private mapping write)
> __handle_mm_fault()
> create_huge_pmd() - FALLBACK
> handle_pte_fault()
> passes check for pmd_devmap()
>
> (private mapping read)
> __handle_mm_fault()
> create_huge_pmd()
> dax_iomap_pmd_fault() inserts PMD
>
> dax_iomap_pte_fault() does a PTE fault, but we already have a DAX PMD
> installed in our page tables at this spot.
>
> Here's the second race:
>
> CPU 0 CPU 1
>
> (private mapping write)
> __handle_mm_fault()
> create_huge_pmd() - FALLBACK
> (private mapping read)
> __handle_mm_fault()
> passes check for pmd_none()
> create_huge_pmd()
>
> handle_pte_fault()
> dax_iomap_pte_fault() inserts PTE
> dax_iomap_pmd_fault() inserts PMD,
> but we already have a PTE at
> this spot.
>
> The core of the issue is that while there is isolation between faults to
> the same range in the DAX fault handlers via our DAX entry locking, there
> is no isolation between faults in the code in mm/memory.c. This means for
> instance that this code in __handle_mm_fault() can run:
>
> if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
> ret = create_huge_pmd(&vmf);
>
> But by the time we actually get to run the fault handler called by
> create_huge_pmd(), the PMD is no longer pmd_none() because a racing PTE
> fault has installed a normal PMD here as a parent. This is the cause of
> the 2nd race. The first race is similar - there is the following check in
> handle_pte_fault():
>
> } else {
> /* See comment in pte_alloc_one_map() */
> if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
> return 0;
>
> So if a pmd_devmap() PMD (a DAX PMD) has been installed at vmf->pmd, we
> will bail and retry the fault. This is correct, but there is nothing
> preventing the PMD from being installed after this check but before we
> actually get to the DAX PTE fault handlers.
>
> In my testing these races result in the following types of errors:
>
> BUG: Bad rss-counter state mm:ffff8800a817d280 idx:1 val:1
> BUG: non-zero nr_ptes on freeing mm: 15
>
> Fix this issue by having the DAX fault handlers verify that it is safe to
> continue their fault after they have taken an entry lock to block other
> racing faults.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Pawel Lebioda <pawel.lebioda@intel.com>
> Cc: stable@vger.kernel.org
>
> ---
>
> I've written a new xfstest for this race, which I will send in response to
> this patch series. This series has also survived an xfstest run without
> any new issues.
>
> ---
> fs/dax.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index c22eaf1..3cc02d1 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1155,6 +1155,15 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
> }
>
> /*
> + * It is possible, particularly with mixed reads & writes to private
> + * mappings, that we have raced with a PMD fault that overlaps with
> + * the PTE we need to set up. Now that we have a locked mapping entry
> + * we can safely unmap the huge PMD so that we can install our PTE in
> + * our page tables.
> + */
> + split_huge_pmd(vmf->vma, vmf->pmd, vmf->address);
> +
Can we just check the PMD and if is isn't as we want it, bail out and retry
the fault? IMHO it will be more obvious that way (and also more in line
like these races are handled for the classical THP). Otherwise the patch
looks good to me.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/2] mm: avoid spurious 'bad pmd' warning messages
From: Jan Kara @ 2017-05-22 14:40 UTC (permalink / raw)
To: Ross Zwisler
Cc: Andrew Morton, linux-kernel, Darrick J. Wong, Alexander Viro,
Christoph Hellwig, Dan Williams, Dave Hansen, Jan Kara,
Matthew Wilcox, linux-fsdevel, linux-mm, linux-nvdimm,
Kirill A . Shutemov, Pawel Lebioda, Dave Jiang, Xiong Zhou,
Eryu Guan, stable
In-Reply-To: <20170517171639.14501-1-ross.zwisler@linux.intel.com>
On Wed 17-05-17 11:16:38, Ross Zwisler wrote:
> When the pmd_devmap() checks were added by:
>
> commit 5c7fb56e5e3f ("mm, dax: dax-pmd vs thp-pmd vs hugetlbfs-pmd")
>
> to add better support for DAX huge pages, they were all added to the end of
> if() statements after existing pmd_trans_huge() checks. So, things like:
>
> - if (pmd_trans_huge(*pmd))
> + if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd))
>
> When further checks were added after pmd_trans_unstable() checks by:
>
> commit 7267ec008b5c ("mm: postpone page table allocation until we have page
> to map")
>
> they were also added at the end of the conditional:
>
> + if (pmd_trans_unstable(fe->pmd) || pmd_devmap(*fe->pmd))
>
> This ordering is fine for pmd_trans_huge(), but doesn't work for
> pmd_trans_unstable(). This is because DAX huge pages trip the bad_pmd()
> check inside of pmd_none_or_trans_huge_or_clear_bad() (called by
> pmd_trans_unstable()), which prints out a warning and returns 1. So, we do
> end up doing the right thing, but only after spamming dmesg with suspicious
> looking messages:
>
> mm/pgtable-generic.c:39: bad pmd ffff8808daa49b88(84000001006000a5)
>
> Reorder these checks so that pmd_devmap() is checked first, avoiding the
> error messages.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Fixes: commit 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
> Cc: stable@vger.kernel.org
With the change requested by Dave this looks good to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> mm/memory.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 6ff5d72..1ee269d 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3061,7 +3061,7 @@ static int pte_alloc_one_map(struct vm_fault *vmf)
> * through an atomic read in C, which is what pmd_trans_unstable()
> * provides.
> */
> - if (pmd_trans_unstable(vmf->pmd) || pmd_devmap(*vmf->pmd))
> + if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
> return VM_FAULT_NOPAGE;
>
> vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
> @@ -3690,7 +3690,7 @@ static int handle_pte_fault(struct vm_fault *vmf)
> vmf->pte = NULL;
> } else {
> /* See comment in pte_alloc_one_map() */
> - if (pmd_trans_unstable(vmf->pmd) || pmd_devmap(*vmf->pmd))
> + if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
> return 0;
> /*
> * A regular pmd is established and it can't morph into a huge
> --
> 2.9.4
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 2/2] dax: Fix race between colliding PMD & PTE entries
From: Jan Kara @ 2017-05-22 14:37 UTC (permalink / raw)
To: Ross Zwisler
Cc: Jan Kara, Andrew Morton, linux-kernel, Darrick J. Wong,
Alexander Viro, Christoph Hellwig, Dan Williams, Dave Hansen,
Matthew Wilcox, linux-fsdevel, linux-mm, linux-nvdimm,
Kirill A . Shutemov, Pawel Lebioda, Dave Jiang, Xiong Zhou,
Eryu Guan, stable
In-Reply-To: <20170518212939.GA28029@linux.intel.com>
On Thu 18-05-17 15:29:39, Ross Zwisler wrote:
> On Thu, May 18, 2017 at 09:50:37AM +0200, Jan Kara wrote:
> > On Wed 17-05-17 11:16:39, Ross Zwisler wrote:
> > > We currently have two related PMD vs PTE races in the DAX code. These can
> > > both be easily triggered by having two threads reading and writing
> > > simultaneously to the same private mapping, with the key being that private
> > > mapping reads can be handled with PMDs but private mapping writes are
> > > always handled with PTEs so that we can COW.
> > >
> > > Here is the first race:
> > >
> > > CPU 0 CPU 1
> > >
> > > (private mapping write)
> > > __handle_mm_fault()
> > > create_huge_pmd() - FALLBACK
> > > handle_pte_fault()
> > > passes check for pmd_devmap()
> > >
> > > (private mapping read)
> > > __handle_mm_fault()
> > > create_huge_pmd()
> > > dax_iomap_pmd_fault() inserts PMD
> > >
> > > dax_iomap_pte_fault() does a PTE fault, but we already have a DAX PMD
> > > installed in our page tables at this spot.
> > >
> > >
> > > Here's the second race:
> > >
> > > CPU 0 CPU 1
> > >
> > > (private mapping write)
> > > __handle_mm_fault()
> > > create_huge_pmd() - FALLBACK
> > > (private mapping read)
> > > __handle_mm_fault()
> > > passes check for pmd_none()
> > > create_huge_pmd()
> > >
> > > handle_pte_fault()
> > > dax_iomap_pte_fault() inserts PTE
> > > dax_iomap_pmd_fault() inserts PMD,
> > > but we already have a PTE at
> > > this spot.
> >
> > So I don't see how this second scenario can happen. dax_iomap_pmd_fault()
> > will call grab_mapping_entry(). That will either find PTE entry in the
> > radix tree -> EEXIST and we retry the fault. Or we will not find PTE entry
> > -> try to insert PMD entry which collides with the PTE entry -> EEXIST and
> > we retry the fault. Am I missing something?
>
> Yep, sorry, I guess I needed a few extra steps in my flow (the initial private
> mapping read by CPU 0):
>
>
> CPU 0 CPU 1
>
> (private mapping read)
> __handle_mm_fault()
> passes check for pmd_none()
> create_huge_pmd()
> dax_iomap_pmd_fault() inserts PMD
>
> (private mapping write)
> __handle_mm_fault()
> create_huge_pmd() - FALLBACK
> (private mapping read)
> __handle_mm_fault()
> passes check for pmd_none()
> create_huge_pmd()
>
> handle_pte_fault()
> dax_iomap_pte_fault() inserts PTE
> dax_iomap_pmd_fault() inserts PMD,
> but we already have a PTE at
> this spot.
>
> So what happens is that CPU 0 inserts a DAX PMD into the radix tree that has
> real storage backing, and all PTE and PMD faults just use that same PMD radix
> tree entry for locking and dirty tracking.
OK, I see now. So essentially it's the same catch as the other case -
grab_mapping_entry() returns PMD entry on CPU0 although we asked for PTE
entry.
> > The first scenario seems to be possible. dax_iomap_pmd_fault() will create
> > PMD entry in the radix tree. Then dax_iomap_pte_fault() will come, do
> > grab_mapping_entry(), there it sees entry is PMD but we are doing PTE fault
> > so I'd think that pmd_downgrade = true... But actually the condition there
> > doesn't trigger in this case. And that's a catch that although we asked
> > grab_mapping_entry() for PTE, we've got PMD back and that screws us later.
>
> Yep, it was a concious decision when implementing the PMD support to allow one
> thread to use PMDs and another to use PTEs in the same range, as long as the
> thread faulting in PMDs is the first to insert into the radix tree. A PMD
> radix tree entry will be inserted and used for locking and dirty tracking, and
> each thread or process can fault in either PTEs or PMDs into its own address
> space as needed.
Well, for *threads* it doesn't really make good sense to mix PMDs and PTEs
as they share page tables. However for *processes* it makes some sense to
allow one process to use PTEs and another process to use PMDs. And I
remember we were discussing this in the past.
> We can revisit this, if you think it is incorrect. The option you outline
> below would basically mean that if any thread were to fault in a PTE in a
> range, all threads and processes would be forced to use PTEs because we would
> use PTEs in the radix tree.
Well, I don't think it is necessarily incorrect. I just think it is more
difficult to get it right (as current bugs show) so I'm just considering
whether the complexity is worth it.
> This is cleaner...I'm not sure if the use case of having two threads accessing
> the same area, one with PTEs and one with PMDs, is actually prevalent. It's
> also maybe a bit weird that the current behavior varies based on which thread
> faulted first - if the PTE thread faults first, it'll insert a PTE into the
> radix tree and everyone will just use PTEs.
So for two *threads*, I don't think that is a sensible use-case. We just
have to get it right. For two *processes* it makes sense - your DB might
want to use PMDs while your backup program may just use PTEs. So thinking
more about it I guess it is worth the effort to make the mixed case work
efficiently.
> > Actually I'm not convinced your patch quite fixes this because
> > dax_load_hole() or dax_insert_mapping_entry() will modify the passed entry
> > with the assumption that it's PTE entry and so they will likely corrupt the
> > entry in the radix tree.
>
> I don't think we can ever call dax_load_hole() if we have a DAX PMD entry in
> the radix tree, because we have a block mapping from the filesystem.
>
> For dax_insert_mapping_entry(), we do the right thing. From the comments
> above the function:
>
> * If we happen to be trying to insert a PTE and there is a PMD
> * already in the tree, we will skip the insertion and just dirty the PMD as
> * appropriate. If we happen to be trying to insert a PTE and there is a PMD
> * already in the tree, we will skip the insertion and just dirty the PMD as
> * appropriate.
Yeah, on the first reading I missed that we won't modify the radix tree in
that particular case. Frankly, I think we should somewhat clean up that
code to make things more obvious but let's leave that for a bit later. For
now the code looks correct.
> > So I think to fix the first case we should rather modify
> > grab_mapping_entry() to properly go through the pmd_downgrade path once we
> > find PMD entry and we do PTE fault.
> >
> > What do you think?
>
> That could also work, though I do think the fix as submitted is correct.
> I think it comes down to whether we want to keep the behavior where a thread
> faulting in a PTEs will use an existing PMD entry in the radix tree, instead
> of making all other threads fall back to PTEs.
>
> I think either way solves this issue for the DAX case...but do you understand
> how this is solved for other fault handlers? They don't have any isolation
> between faults either in the mm/memory.c code, and are susceptible to the same
> races. How do they deal with the fact that by the time they get to their PTE
> fault handler, a racing PMD fault handler in another thread could have
> inserted a PMD into their page tables, and vice versa?
So normal fault path uses alloc_set_pte() for installing new PTE. And that
uses pte_alloc_one_map() which checks whether PMD is still suitable for
inserting a PTE. If not, we return VM_FAULT_NOPAGE. Probably it would be
cleanest to factor our common parts of PTE and PMD insertion so that we can
use these functions both from DAX and generic fault paths.
Anyway, I'll have a look at your fixes with fresh eyes as they could be the
right way to go as a quick fix. Refactoring and cleanups can come later.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] x86/mm: pgds getting out of sync after memory hot remove
From: Kirill A. Shutemov @ 2017-05-22 14:29 UTC (permalink / raw)
Cc: linux-kernel, linux-mm, Kirill A. Shutemov, Andrew Morton,
Michal Hocko, Mel Gorman
In-Reply-To: <20170522141150.GA3813@redhat.com>
On Mon, May 22, 2017 at 10:11:51AM -0400, Jerome Glisse wrote:
> On Mon, May 22, 2017 at 04:12:15PM +0300, Kirill A. Shutemov wrote:
> > On Fri, May 19, 2017 at 02:01:26PM -0400, Jerome Glisse wrote:
> > > After memory hot remove it seems we do not synchronize pgds for kernel
> > > virtual memory range (on vmemmap_free()). This seems bogus to me as it
> > > means we are left with stall entry for process with mm != mm_init
> > >
> > > Yet i am puzzle by the fact that i am only now hitting this issue. It
> > > never was an issue with 4.12 or before ie HMM never triggered following
> > > BUG_ON inside sync_global_pgds():
> > >
> > > if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
> > > BUG_ON(p4d_page_vaddr(*p4d) != p4d_page_vaddr(*p4d_ref));
> > >
> > >
> > > It seems that Kirill 5 level page table changes play a role in this
> > > behavior change. I could not bisect because HMM is painfull to rebase
> > > for each bisection step so that is just my best guess.
> > >
> > >
> > > Am i missing something here ? Am i wrong in assuming that should sync
> > > pgd on vmemmap_free() ? If so anyone have a good guess on why i am now
> > > seeing the above BUG_ON ?
> >
> > What would we gain by syncing pgd on free? Stale pgds are fine as long as
> > they are not referenced (use-after-free case). Syncing is addtional work.
>
> Well then how do i avoid the BUG_ON above ? Because the init_mm pgd is
> clear but none of the stall entry in any other mm. So if i unplug memory
> and replug memory at exact same address it tries to allocate new p4d/pud
> for struct page area and then when sync_global_pgds() is call it goes
> over the list of pgd and BUG_ON() :
>
> if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
> BUG_ON(p4d_page_vaddr(*p4d) != p4d_page_vaddr(*p4d_ref));
>
>
> So to me either above check need to go and we should overwritte pgd no
> matter what or we should restore previous behavior. I don't mind either
> one.
I would prefer to drop the BUG_ON.
Ingo?
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE
From: Mike Rapoport @ 2017-05-22 14:29 UTC (permalink / raw)
To: Michal Hocko
Cc: Kirill A. Shutemov, Andrew Morton, Arnd Bergmann,
Kirill A. Shutemov, Andrea Arcangeli, Pavel Emelyanov, linux-mm,
lkml
In-Reply-To: <20170522135548.GA8514@dhcp22.suse.cz>
On Mon, May 22, 2017 at 03:55:48PM +0200, Michal Hocko wrote:
> On Mon 22-05-17 16:36:00, Mike Rapoport wrote:
> > On Mon, May 22, 2017 at 02:42:43PM +0300, Kirill A. Shutemov wrote:
> > > On Mon, May 22, 2017 at 09:12:42AM +0300, Mike Rapoport wrote:
> > > > Currently applications can explicitly enable or disable THP for a memory
> > > > region using MADV_HUGEPAGE or MADV_NOHUGEPAGE. However, once either of
> > > > these advises is used, the region will always have
> > > > VM_HUGEPAGE/VM_NOHUGEPAGE flag set in vma->vm_flags.
> > > > The MADV_CLR_HUGEPAGE resets both these flags and allows managing THP in
> > > > the region according to system-wide settings.
> > >
> > > Seems reasonable. But could you describe an use-case when it's useful in
> > > real world.
> >
> > My use-case was combination of pre- and post-copy migration of containers
> > with CRIU.
> > In this case we populate a part of a memory region with data that was saved
> > during the pre-copy stage. Afterwards, the region is registered with
> > userfaultfd and we expect to get page faults for the parts of the region
> > that were not yet populated. However, khugepaged collapses the pages and
> > the page faults we would expect do not occur.
>
> I am not sure I undestand the problem. Do I get it right that the
> khugepaged will effectivelly corrupt the memory by collapsing a range
> which is not yet fully populated? If yes shouldn't that be fixed in
> khugepaged rather than adding yet another madvise command? Also how do
> you prevent on races? (say you VM_NOHUGEPAGE, khugepaged would be in the
> middle of the operation and sees a collapsable vma and you get the same
> result)
Probably I didn't explained it too well.
The range is intentionally not populated. When we combine pre- and
post-copy for process migration, we create memory pre-dump without stopping
the process, then we freeze the process without dumping the pages it has
dirtied between pre-dump and freeze, and then, during restore, we populate
the dirtied pages using userfaultfd.
When CRIU restores a process in such scenario, it does something like:
* mmap() memory region
* fill in the pages that were collected during the pre-dump
* do some other stuff
* register memory region with userfaultfd
* populate the missing memory on demand
khugepaged collapses the pages in the partially populated regions before we
have a chance to register these regions with userfaultfd, which would
prevent the collapse.
We could have used MADV_NOHUGEPAGE right after the mmap() call, and then
there would be no race because there would be nothing for khugepaged to
collapse at that point. But the problem is that we have no way to reset
*HUGEPAGE flags after the memory restore is complete.
> --
> Michal Hocko
> SUSE Labs
--
Sincerely yours,
Mike.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] x86/mm: pgds getting out of sync after memory hot remove
From: Jerome Glisse @ 2017-05-22 14:11 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: linux-kernel, linux-mm, Kirill A. Shutemov, Andrew Morton,
Ingo Molnar, Michal Hocko, Mel Gorman
In-Reply-To: <20170522131215.wrnklp4dtemntixz@node.shutemov.name>
On Mon, May 22, 2017 at 04:12:15PM +0300, Kirill A. Shutemov wrote:
> On Fri, May 19, 2017 at 02:01:26PM -0400, Jerome Glisse wrote:
> > After memory hot remove it seems we do not synchronize pgds for kernel
> > virtual memory range (on vmemmap_free()). This seems bogus to me as it
> > means we are left with stall entry for process with mm != mm_init
> >
> > Yet i am puzzle by the fact that i am only now hitting this issue. It
> > never was an issue with 4.12 or before ie HMM never triggered following
> > BUG_ON inside sync_global_pgds():
> >
> > if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
> > BUG_ON(p4d_page_vaddr(*p4d) != p4d_page_vaddr(*p4d_ref));
> >
> >
> > It seems that Kirill 5 level page table changes play a role in this
> > behavior change. I could not bisect because HMM is painfull to rebase
> > for each bisection step so that is just my best guess.
> >
> >
> > Am i missing something here ? Am i wrong in assuming that should sync
> > pgd on vmemmap_free() ? If so anyone have a good guess on why i am now
> > seeing the above BUG_ON ?
>
> What would we gain by syncing pgd on free? Stale pgds are fine as long as
> they are not referenced (use-after-free case). Syncing is addtional work.
Well then how do i avoid the BUG_ON above ? Because the init_mm pgd is
clear but none of the stall entry in any other mm. So if i unplug memory
and replug memory at exact same address it tries to allocate new p4d/pud
for struct page area and then when sync_global_pgds() is call it goes
over the list of pgd and BUG_ON() :
if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
BUG_ON(p4d_page_vaddr(*p4d) != p4d_page_vaddr(*p4d_ref));
So to me either above check need to go and we should overwritte pgd no
matter what or we should restore previous behavior. I don't mind either
one.
Cheers,
Jerome
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] LSM: Make security_hook_heads a local variable.
From: Christoph Hellwig @ 2017-05-22 14:03 UTC (permalink / raw)
To: Tetsuo Handa
Cc: linux-security-module, linux-mm, kernel-hardening, linux-kernel,
Casey Schaufler, Greg KH, Igor Stoppa, James Morris, Kees Cook,
Paul Moore, Stephen Smalley
In-Reply-To: <1495365245-3185-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
On Sun, May 21, 2017 at 08:14:05PM +0900, Tetsuo Handa wrote:
> A sealable memory allocator patch was proposed at
> http://lkml.kernel.org/r/20170519103811.2183-1-igor.stoppa@huawei.com ,
> and is waiting for a follow-on patch showing how any of the kernel
> can be changed to use this new subsystem. So, here it is for LSM hooks.
>
> The LSM hooks ("struct security_hook_heads security_hook_heads" and
> "struct security_hook_list ...[]") will benefit from this allocator via
> protection using set_memory_ro()/set_memory_rw(), and it will remove
> CONFIG_SECURITY_WRITABLE_HOOKS config option.
>
> This means that these structures will be allocated at run time using
> smalloc(), and therefore the address of these structures will be
> determined at run time rather than compile time.
>
> But currently, LSM_HOOK_INIT() macro depends on the address of
> security_hook_heads being known at compile time. But we already
> initialize security_hook_heads as an array of "struct list_head".
>
> Therefore, let's use index number (or relative offset from the head
> of security_hook_heads) instead of absolute address of
> security_hook_heads so that LSM_HOOK_INIT() macro does not need to
> know absolute address of security_hook_heads. Then, security_add_hooks()
> will be able to allocate and copy "struct security_hook_list ...[]" using
> smalloc().
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: Igor Stoppa <igor.stoppa@huawei.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> ---
> include/linux/lsm_hooks.h | 6 +++---
> security/security.c | 10 ++++++++--
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..865c11d 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1884,8 +1884,8 @@ struct security_hook_heads {
> */
> struct security_hook_list {
> struct list_head list;
> - struct list_head *head;
> union security_list_options hook;
> + const unsigned int idx;
> char *lsm;
> };
>
> @@ -1896,9 +1896,9 @@ struct security_hook_list {
> * text involved.
> */
> #define LSM_HOOK_INIT(HEAD, HOOK) \
> - { .head = &security_hook_heads.HEAD, .hook = { .HEAD = HOOK } }
> + { .idx = offsetof(struct security_hook_heads, HEAD) / \
> + sizeof(struct list_head), .hook = { .HEAD = HOOK } }
>
> -extern struct security_hook_heads security_hook_heads;
> extern char *lsm_names;
>
> extern void security_add_hooks(struct security_hook_list *hooks, int count,
> diff --git a/security/security.c b/security/security.c
> index 54b1e39..d6883ce 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -33,7 +33,7 @@
> /* Maximum number of letters for an LSM name string */
> #define SECURITY_NAME_MAX 10
>
> -struct security_hook_heads security_hook_heads __lsm_ro_after_init;
> +static struct security_hook_heads security_hook_heads __lsm_ro_after_init;
> char *lsm_names;
> /* Boot-time LSM user choice */
> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
> @@ -152,10 +152,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> char *lsm)
> {
> int i;
> + struct list_head *list = (struct list_head *) &security_hook_heads;
Eww, struct casts. This whole security_hook_heads scheme stink,
even with the slight improvements from Tetsuo. It has everything we
shouldn't do - function pointers in structures that are not hard
read-only, structure casts, etc.
What's the reason why can't just have good old const function tables?
Yeah, stackable LSM make that a little harder, but they should not be
enable by default anyway. But even with those we can still chain
them together with a list with external linkage.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 4/6] mm/hugetlb: Allow architectures to override huge_pte_clear()
From: Arnd Bergmann @ 2017-05-22 13:59 UTC (permalink / raw)
To: Punit Agrawal
Cc: Andrew Morton, Linux-MM, Linux Kernel Mailing List, Linux ARM,
Catalin Marinas, Will Deacon, n-horiguchi, Kirill A . Shutemov,
mike.kravetz, steve.capper, Mark Rutland, Hillf Danton,
linux-arch, Aneesh Kumar K.V, Martin Schwidefsky, Heiko Carstens
In-Reply-To: <20170522133604.11392-5-punit.agrawal@arm.com>
On Mon, May 22, 2017 at 3:36 PM, Punit Agrawal <punit.agrawal@arm.com> wrote:
> diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
> index 99b490b4d05a..3138e126f43b 100644
> --- a/include/asm-generic/hugetlb.h
> +++ b/include/asm-generic/hugetlb.h
> @@ -31,10 +31,7 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot)
> return pte_modify(pte, newprot);
> }
>
> -static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
> - pte_t *ptep)
> -{
> - pte_clear(mm, addr, ptep);
> -}
> +void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, unsigned long sz);
>
> #endif /* _ASM_GENERIC_HUGETLB_H */
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 0e4d1fb3122f..2b0f6f96f2c1 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3289,6 +3289,12 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
> return ret;
> }
>
> +void __weak huge_pte_clear(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, unsigned long sz)
> +{
> + pte_clear(mm, addr, ptep);
> +}
> +
> void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
> unsigned long start, unsigned long end,
> struct page *ref_page)
I don't really like how this moves the inline version from asm-generic into
a __weak function here. I think it would be better to either stop
using asm-generic/hugetlb.h
on s390, or enclose the generic definition in
#ifndef huge_pte_clear
and then override by defining a macro in s390 as we do in other files
in asm-generic.
Arnd
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v1 00/11] mm/kasan: support per-page shadow memory to reduce memory consumption
From: Andrey Ryabinin @ 2017-05-22 14:00 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin, kernel-team
In-Reply-To: <20170519015348.GA1763@js1304-desktop>
On 05/19/2017 04:53 AM, Joonsoo Kim wrote:
> On Wed, May 17, 2017 at 03:17:13PM +0300, Andrey Ryabinin wrote:
>> On 05/16/2017 04:16 AM, js1304@gmail.com wrote:
>>> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>>>
>>> Hello, all.
>>>
>>> This is an attempt to recude memory consumption of KASAN. Please see
>>> following description to get the more information.
>>>
>>> 1. What is per-page shadow memory
>>>
>>> This patch introduces infrastructure to support per-page shadow memory.
>>> Per-page shadow memory is the same with original shadow memory except
>>> the granualarity. It's one byte shows the shadow value for the page.
>>> The purpose of introducing this new shadow memory is to save memory
>>> consumption.
>>>
>>> 2. Problem of current approach
>>>
>>> Until now, KASAN needs shadow memory for all the range of the memory
>>> so the amount of statically allocated memory is so large. It causes
>>> the problem that KASAN cannot run on the system with hard memory
>>> constraint. Even if KASAN can run, large memory consumption due to
>>> KASAN changes behaviour of the workload so we cannot validate
>>> the moment that we want to check.
>>>
>>> 3. How does this patch fix the problem
>>>
>>> This patch tries to fix the problem by reducing memory consumption for
>>> the shadow memory. There are two observations.
>>>
>>
>>
>> I think that the best way to deal with your problem is to increase shadow scale size.
>>
>> You'll need to add tunable to gcc to control shadow size. I expect that gcc has some
>> places where 8-shadow scale size is hardcoded, but it should be fixable.
>>
>> The kernel also have some small amount of code written with KASAN_SHADOW_SCALE_SIZE == 8 in mind,
>> which should be easy to fix.
>>
>> Note that bigger shadow scale size requires bigger alignment of allocated memory and variables.
>> However, according to comments in gcc/asan.c gcc already aligns stack and global variables and at
>> 32-bytes boundary.
>> So we could bump shadow scale up to 32 without increasing current stack consumption.
>>
>> On a small machine (1Gb) 1/32 of shadow is just 32Mb which is comparable to yours 30Mb, but I expect it to be
>> much faster. More importantly, this will require only small amount of simple changes in code, which will be
>> a *lot* more easier to maintain.
>
> I agree that it is also a good option to reduce memory consumption.
> Nevertheless, there are two reasons that justifies this patchset.
>
> 1) With this patchset, memory consumption isn't increased in
> proportional to total memory size. Please consider my 4Gb system
> example on the below. With increasing shadow scale size to 32, memory
> would be consumed by 128M. However, this patchset consumed 50MB. This
> difference can be larger if we run KASAN with bigger machine.
>
Well, yes, but I assume that bigger machine implies that we can use more memory without
causing a significant change in system's behavior.
> 2) These two optimization can be applied simulatenously. It is just an
> orthogonal feature. If shadow scale size is increased to 32, memory
> consumption will be decreased in case of my patchset, too.
>
> Therefore, I think that this patchset is useful in any case.
These are valid points, but IMO it's not enough to justify this patchset.
Too much of hacky and fragile code.
If our goal is to make KASAN to eat less memory, the first step definitely would be a 1/32 shadow.
Simply because it's the best way to achieve that goal.
And only if it's not enough we could think about something else, like decreasing/turning off quarantine
and/or smaller redzones.
> Note that increasing shadow scale has it's own trade-off. It requires
> that the size of slab object is aligned to shadow scale. It will
> increase memory consumption due to slab.
>
Yes, but I don't think that it will be significant, many objects are aligned already.
I've tried the kernel with 32 ARCH_SLAB_MINALIGN and ARCH_KMALLOC_MINALIGN and
the difference in Slab consumption after booting 1G VM was not significant:
8-byte align:
Slab: 126516 kB
SReclaimable: 31140 kB
SUnreclaim: 95376 kB
32-byte align:
Slab: 126712 kB
SReclaimable: 30912 kB
SUnreclaim: 95800 kB
Numbers slightly vary from boot to boot.
> Thanks.
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE
From: Michal Hocko @ 2017-05-22 13:55 UTC (permalink / raw)
To: Mike Rapoport
Cc: Kirill A. Shutemov, Andrew Morton, Arnd Bergmann,
Kirill A. Shutemov, Andrea Arcangeli, Pavel Emelyanov, linux-mm,
lkml
In-Reply-To: <20170522133559.GE27382@rapoport-lnx>
On Mon 22-05-17 16:36:00, Mike Rapoport wrote:
> On Mon, May 22, 2017 at 02:42:43PM +0300, Kirill A. Shutemov wrote:
> > On Mon, May 22, 2017 at 09:12:42AM +0300, Mike Rapoport wrote:
> > > Currently applications can explicitly enable or disable THP for a memory
> > > region using MADV_HUGEPAGE or MADV_NOHUGEPAGE. However, once either of
> > > these advises is used, the region will always have
> > > VM_HUGEPAGE/VM_NOHUGEPAGE flag set in vma->vm_flags.
> > > The MADV_CLR_HUGEPAGE resets both these flags and allows managing THP in
> > > the region according to system-wide settings.
> >
> > Seems reasonable. But could you describe an use-case when it's useful in
> > real world.
>
> My use-case was combination of pre- and post-copy migration of containers
> with CRIU.
> In this case we populate a part of a memory region with data that was saved
> during the pre-copy stage. Afterwards, the region is registered with
> userfaultfd and we expect to get page faults for the parts of the region
> that were not yet populated. However, khugepaged collapses the pages and
> the page faults we would expect do not occur.
I am not sure I undestand the problem. Do I get it right that the
khugepaged will effectivelly corrupt the memory by collapsing a range
which is not yet fully populated? If yes shouldn't that be fixed in
khugepaged rather than adding yet another madvise command? Also how do
you prevent on races? (say you VM_NOHUGEPAGE, khugepaged would be in the
middle of the operation and sees a collapsable vma and you get the same
result)
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE
From: Kirill A. Shutemov @ 2017-05-22 13:44 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Arnd Bergmann, Kirill A. Shutemov,
Andrea Arcangeli, Pavel Emelyanov, linux-mm, lkml
In-Reply-To: <20170522133559.GE27382@rapoport-lnx>
On Mon, May 22, 2017 at 04:36:00PM +0300, Mike Rapoport wrote:
> On Mon, May 22, 2017 at 02:42:43PM +0300, Kirill A. Shutemov wrote:
> > On Mon, May 22, 2017 at 09:12:42AM +0300, Mike Rapoport wrote:
> > > Currently applications can explicitly enable or disable THP for a memory
> > > region using MADV_HUGEPAGE or MADV_NOHUGEPAGE. However, once either of
> > > these advises is used, the region will always have
> > > VM_HUGEPAGE/VM_NOHUGEPAGE flag set in vma->vm_flags.
> > > The MADV_CLR_HUGEPAGE resets both these flags and allows managing THP in
> > > the region according to system-wide settings.
> >
> > Seems reasonable. But could you describe an use-case when it's useful in
> > real world.
>
> My use-case was combination of pre- and post-copy migration of containers
> with CRIU.
> In this case we populate a part of a memory region with data that was saved
> during the pre-copy stage. Afterwards, the region is registered with
> userfaultfd and we expect to get page faults for the parts of the region
> that were not yet populated. However, khugepaged collapses the pages and
> the page faults we would expect do not occur.
>
> We could have used MADV_NOHUGEPAGE before populating the region with the
> pre-copy data, but then, in the end, the restored application will be resumed
> with vma->vm_flags different from the ones it had when it was frozen.
>
> Another possibility I've considered was to register the region with
> userfaultfd before populating it with data, but in that case we get the
> overhead of UFFD_EVENT_PAGEFAULT + UFFDIO_{COPY,ZEROPAGE} for nothing :(
Okay. Makes sense. Feel free to use my Acked-by (with change to RESET).
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [v4 1/1] mm: Adaptive hash table scaling
From: Pasha Tatashin @ 2017-05-22 13:41 UTC (permalink / raw)
To: Michal Hocko; +Cc: akpm, linux-kernel, linux-mm, Michael Ellerman
In-Reply-To: <20170522133834.GL8509@dhcp22.suse.cz>
>
> This is just too ugly to live, really. If we do not need adaptive
> scaling then just make it #if __BITS_PER_LONG around the code. I would
> be fine with this. A big fat warning explaining why this is 64b only
> would be appropriate.
>
OK, let me prettify it somehow, and I will send a new patch out.
Pasha
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [v4 1/1] mm: Adaptive hash table scaling
From: Michal Hocko @ 2017-05-22 13:38 UTC (permalink / raw)
To: Pasha Tatashin; +Cc: akpm, linux-kernel, linux-mm, Michael Ellerman
In-Reply-To: <f6585e67-1640-daa3-370c-f37562cb5245@oracle.com>
On Mon 22-05-17 09:18:58, Pasha Tatashin wrote:
> >
> >I have only noticed this email today because my incoming emails stopped
> >syncing since Friday. But this is _definitely_ not the right approachh.
> >64G for 32b systems is _way_ off. We have only ~1G for the kernel. I've
> >already proposed scaling up to 32M for 32b systems and Andi seems to be
> >suggesting the same. So can we fold or apply the following instead?
>
> Hi Michal,
>
> Thank you for your suggestion. I will update the patch.
>
> 64G base for 32bit systems is not meant to be ever used, as the adaptive
> scaling for 32bit system is just not needed. 32M and 64G are going to be
> exactly the same on such systems.
>
> Here is theoretical limit for the max hash size of entries (dentry cache
> example):
>
> size of bucket: sizeof(struct hlist_bl_head) = 4 bytes
> numentries: (1 << 32) / PAGE_SIZE = 1048576 (for 4K pages)
> hash size: 4b * 1048576 = 4M
>
> In practice it is going to be an order smaller, as number of kernel pages is
> less then (1<<32).
I haven't double check your math but if the above is correct then I
would just go and disable the adaptive scaling for 32b altogether. More
on that below.
> However, I will apply your suggestions as there seems to be a problem of
> overflowing in comparing ul vs. ull as reported by Michael Ellerman, and
> having a large base on 32bit systems will solve this issue. I will revert
> back to "ul" all the quantities.
Yeah, that is just calling for troubles.
> Another approach is to make it a 64 bit only macro like this:
>
> #if __BITS_PER_LONG > 32
>
> #define ADAPT_SCALE_BASE (64ull << 30)
> #define ADAPT_SCALE_SHIFT 2
> #define ADAPT_SCALE_NPAGES (ADAPT_SCALE_BASE >> PAGE_SHIFT)
>
> #define adapt_scale(high_limit, numentries, scalep)
> if (!(high_limit)) { \
> unsigned long adapt; \
> for (adapt = ADAPT_SCALE_NPAGES; adapt < \
> (numentries); adapt <<= ADAPT_SCALE_SHIFT) \
> (*(scalep))++; \
> }
> #else
> #define adapt_scale(high_limit, numentries scalep)
> #endif
This is just too ugly to live, really. If we do not need adaptive
scaling then just make it #if __BITS_PER_LONG around the code. I would
be fine with this. A big fat warning explaining why this is 64b only
would be appropriate.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH v3 6/6] mm: rmap: Use correct helper when poisoning hugepages
From: Punit Agrawal @ 2017-05-22 13:36 UTC (permalink / raw)
To: akpm
Cc: Punit Agrawal, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
aneesh.kumar
In-Reply-To: <20170522133604.11392-1-punit.agrawal@arm.com>
Using set_pte_at() does not do the right thing when putting down
HWPOISON swap entries for hugepages on architectures that support
contiguous ptes.
Fix this problem by using set_huge_swap_pte_at() which was introduced to
fix exactly this problem.
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Steve Capper <steve.capper@arm.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
---
mm/rmap.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/mm/rmap.c b/mm/rmap.c
index d405f0e0ee96..feb2352aa95f 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1379,15 +1379,18 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
update_hiwater_rss(mm);
if (PageHWPoison(page) && !(flags & TTU_IGNORE_HWPOISON)) {
+ pteval = swp_entry_to_pte(make_hwpoison_entry(subpage));
if (PageHuge(page)) {
int nr = 1 << compound_order(page);
hugetlb_count_sub(nr, mm);
+ set_huge_swap_pte_at(mm, address,
+ pvmw.pte, pteval,
+ vma_mmu_pagesize(vma));
} else {
dec_mm_counter(mm, mm_counter(page));
+ set_pte_at(mm, address, pvmw.pte, pteval);
}
- pteval = swp_entry_to_pte(make_hwpoison_entry(subpage));
- set_pte_at(mm, address, pvmw.pte, pteval);
} else if (pte_unused(pteval)) {
/*
* The guest indicated that the page content is of no
--
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v3 5/6] mm/hugetlb: Introduce set_huge_swap_pte_at() helper
From: Punit Agrawal @ 2017-05-22 13:36 UTC (permalink / raw)
To: akpm
Cc: Punit Agrawal, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
aneesh.kumar
In-Reply-To: <20170522133604.11392-1-punit.agrawal@arm.com>
set_huge_pte_at(), an architecture callback to populate hugepage ptes,
does not provide the range of virtual memory that is targeted. This
leads to ambiguity when dealing with swap entries on architectures that
support hugepages consisting of contiguous ptes.
Fix the problem by introducing an overridable helper that is called when
populating the page tables with swap entries. The size of the targeted
region is provided to the helper to help determine the number of entries
to be updated.
Provide a default implementation that maintains the current behaviour.
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Steve Capper <steve.capper@arm.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
---
include/linux/hugetlb.h | 2 ++
mm/hugetlb.c | 14 +++++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 23010a3b2047..fa65ad73a65f 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -127,6 +127,8 @@ int pud_huge(pud_t pud);
unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
unsigned long address, unsigned long end, pgprot_t newprot);
+void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte, unsigned long sz);
#else /* !CONFIG_HUGETLB_PAGE */
static inline void reset_vma_resv_huge_pages(struct vm_area_struct *vma)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2b0f6f96f2c1..a27e926913f4 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3211,6 +3211,12 @@ static int is_hugetlb_entry_hwpoisoned(pte_t pte)
return 0;
}
+void __weak set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte, unsigned long sz)
+{
+ set_huge_pte_at(mm, addr, ptep, pte);
+}
+
int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
struct vm_area_struct *vma)
{
@@ -3263,9 +3269,10 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
*/
make_migration_entry_read(&swp_entry);
entry = swp_entry_to_pte(swp_entry);
- set_huge_pte_at(src, addr, src_pte, entry);
+ set_huge_swap_pte_at(src, addr, src_pte,
+ entry, sz);
}
- set_huge_pte_at(dst, addr, dst_pte, entry);
+ set_huge_swap_pte_at(dst, addr, dst_pte, entry, sz);
} else {
if (cow) {
huge_ptep_set_wrprotect(src, addr, src_pte);
@@ -4283,7 +4290,8 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
make_migration_entry_read(&entry);
newpte = swp_entry_to_pte(entry);
- set_huge_pte_at(mm, address, ptep, newpte);
+ set_huge_swap_pte_at(mm, address, ptep,
+ newpte, huge_page_size(h));
pages++;
}
spin_unlock(ptl);
--
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v3 4/6] mm/hugetlb: Allow architectures to override huge_pte_clear()
From: Punit Agrawal @ 2017-05-22 13:36 UTC (permalink / raw)
To: akpm
Cc: Punit Agrawal, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
aneesh.kumar, Martin Schwidefsky, Heiko Carstens, Arnd Bergmann
In-Reply-To: <20170522133604.11392-1-punit.agrawal@arm.com>
When unmapping a hugepage range, huge_pte_clear() is used to clear the
page table entries that are marked as not present. huge_pte_clear()
internally just ends up calling pte_clear() which does not correctly
deal with hugepages consisting of contiguous page table entries.
Add a size argument to address this issue and allow architectures to
override huge_pte_clear() in subsequent patches by making it a weak
function.
Update the s390 to use the new mechanism to override huge_pte_clear().
Note that the change only affects huge_pte_clear() - the other generic
hugetlb functions don't need any change.
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
---
arch/s390/include/asm/hugetlb.h | 10 ++--------
arch/s390/mm/hugetlbpage.c | 9 +++++++++
include/asm-generic/hugetlb.h | 7 ++-----
mm/hugetlb.c | 8 +++++++-
4 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h
index cd546a245c68..aa8489c07f24 100644
--- a/arch/s390/include/asm/hugetlb.h
+++ b/arch/s390/include/asm/hugetlb.h
@@ -38,14 +38,8 @@ static inline int prepare_hugepage_range(struct file *file,
#define arch_clear_hugepage_flags(page) do { } while (0)
-static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
- pte_t *ptep)
-{
- if ((pte_val(*ptep) & _REGION_ENTRY_TYPE_MASK) == _REGION_ENTRY_TYPE_R3)
- pte_val(*ptep) = _REGION3_ENTRY_EMPTY;
- else
- pte_val(*ptep) = _SEGMENT_ENTRY_EMPTY;
-}
+void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, unsigned long sz);
static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
unsigned long address, pte_t *ptep)
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index ae23afc18493..48e19b324017 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -144,6 +144,15 @@ pte_t huge_ptep_get(pte_t *ptep)
return __rste_to_pte(pte_val(*ptep));
}
+void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, unsigned long sz)
+{
+ if ((pte_val(*ptep) & _REGION_ENTRY_TYPE_MASK) == _REGION_ENTRY_TYPE_R3)
+ pte_val(*ptep) = _REGION3_ENTRY_EMPTY;
+ else
+ pte_val(*ptep) = _SEGMENT_ENTRY_EMPTY;
+}
+
pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
unsigned long addr, pte_t *ptep)
{
diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
index 99b490b4d05a..3138e126f43b 100644
--- a/include/asm-generic/hugetlb.h
+++ b/include/asm-generic/hugetlb.h
@@ -31,10 +31,7 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot)
return pte_modify(pte, newprot);
}
-static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
- pte_t *ptep)
-{
- pte_clear(mm, addr, ptep);
-}
+void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, unsigned long sz);
#endif /* _ASM_GENERIC_HUGETLB_H */
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 0e4d1fb3122f..2b0f6f96f2c1 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3289,6 +3289,12 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
return ret;
}
+void __weak huge_pte_clear(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, unsigned long sz)
+{
+ pte_clear(mm, addr, ptep);
+}
+
void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
unsigned long start, unsigned long end,
struct page *ref_page)
@@ -3338,7 +3344,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
* unmapped and its refcount is dropped, so just clear pte here.
*/
if (unlikely(!pte_present(pte))) {
- huge_pte_clear(mm, address, ptep);
+ huge_pte_clear(mm, address, ptep, sz);
spin_unlock(ptl);
continue;
}
--
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v3 3/6] mm/hugetlb: add size parameter to huge_pte_offset()
From: Punit Agrawal @ 2017-05-22 13:36 UTC (permalink / raw)
To: akpm
Cc: Punit Agrawal, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
aneesh.kumar, Tony Luck, Fenghua Yu, James Hogan, Ralf Baechle,
James E.J. Bottomley, Helge Deller, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Martin Schwidefsky,
Heiko Carstens, Yoshinori Sato, Rich Felker, David S. Miller,
Chris Metcalf, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Alexander Viro, Michal Hocko
In-Reply-To: <20170522133604.11392-1-punit.agrawal@arm.com>
A poisoned or migrated hugepage is stored as a swap entry in the page
tables. On architectures that support hugepages consisting of contiguous
page table entries (such as on arm64) this leads to ambiguity in
determining the page table entry to return in huge_pte_offset() when a
poisoned entry is encountered.
Let's remove the ambiguity by adding a size parameter to convey
additional information about the requested address. Also fixup the
definition/usage of huge_pte_offset() throughout the tree.
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Steve Capper <steve.capper@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: James Hogan <james.hogan@imgtec.com> (odd fixer:METAG ARCHITECTURE)
Cc: Ralf Baechle <ralf@linux-mips.org> (supporter:MIPS)
Cc: "James E.J. Bottomley" <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Hillf Danton <hillf.zj@alibaba-inc.com>
---
arch/arm64/mm/hugetlbpage.c | 3 ++-
arch/ia64/mm/hugetlbpage.c | 4 ++--
arch/metag/mm/hugetlbpage.c | 3 ++-
arch/mips/mm/hugetlbpage.c | 3 ++-
arch/parisc/mm/hugetlbpage.c | 3 ++-
arch/powerpc/mm/hugetlbpage.c | 2 +-
arch/s390/mm/hugetlbpage.c | 3 ++-
arch/sh/mm/hugetlbpage.c | 3 ++-
arch/sparc/mm/hugetlbpage.c | 3 ++-
arch/tile/mm/hugetlbpage.c | 3 ++-
arch/x86/mm/hugetlbpage.c | 2 +-
fs/userfaultfd.c | 7 +++++--
include/linux/hugetlb.h | 5 +++--
mm/hugetlb.c | 23 ++++++++++++++---------
mm/page_vma_mapped.c | 3 ++-
mm/pagewalk.c | 3 ++-
16 files changed, 46 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 69b8200b1cfd..425a8fcd3d38 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -132,7 +132,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
return pte;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgd;
pud_t *pud;
diff --git a/arch/ia64/mm/hugetlbpage.c b/arch/ia64/mm/hugetlbpage.c
index 85de86d36fdf..ae35140332f7 100644
--- a/arch/ia64/mm/hugetlbpage.c
+++ b/arch/ia64/mm/hugetlbpage.c
@@ -44,7 +44,7 @@ huge_pte_alloc(struct mm_struct *mm, unsigned long addr, unsigned long sz)
}
pte_t *
-huge_pte_offset (struct mm_struct *mm, unsigned long addr)
+huge_pte_offset (struct mm_struct *mm, unsigned long addr, unsigned long sz)
{
unsigned long taddr = htlbpage_to_page(addr);
pgd_t *pgd;
@@ -92,7 +92,7 @@ struct page *follow_huge_addr(struct mm_struct *mm, unsigned long addr, int writ
if (REGION_NUMBER(addr) != RGN_HPAGE)
return ERR_PTR(-EINVAL);
- ptep = huge_pte_offset(mm, addr);
+ ptep = huge_pte_offset(mm, addr, HPAGE_SIZE);
if (!ptep || pte_none(*ptep))
return NULL;
page = pte_page(*ptep);
diff --git a/arch/metag/mm/hugetlbpage.c b/arch/metag/mm/hugetlbpage.c
index db1b7da91e4f..67fd53e2935a 100644
--- a/arch/metag/mm/hugetlbpage.c
+++ b/arch/metag/mm/hugetlbpage.c
@@ -74,7 +74,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
return pte;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgd;
pud_t *pud;
diff --git a/arch/mips/mm/hugetlbpage.c b/arch/mips/mm/hugetlbpage.c
index 74aa6f62468f..cef152234312 100644
--- a/arch/mips/mm/hugetlbpage.c
+++ b/arch/mips/mm/hugetlbpage.c
@@ -36,7 +36,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, unsigned long addr,
return pte;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr,
+ unsigned long sz)
{
pgd_t *pgd;
pud_t *pud;
diff --git a/arch/parisc/mm/hugetlbpage.c b/arch/parisc/mm/hugetlbpage.c
index aa50ac090e9b..5eb8f633b282 100644
--- a/arch/parisc/mm/hugetlbpage.c
+++ b/arch/parisc/mm/hugetlbpage.c
@@ -69,7 +69,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
return pte;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgd;
pud_t *pud;
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index a4f33de4008e..e46744d3b4ae 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -55,7 +55,7 @@ static unsigned nr_gpages;
#define hugepd_none(hpd) (hpd_val(hpd) == 0)
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr, unsigned long sz)
{
/* Only called for hugetlbfs pages, hence can ignore THP */
return __find_linux_pte_or_hugepte(mm->pgd, addr, NULL, NULL);
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index 9b4050caa4e9..ae23afc18493 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -176,7 +176,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
return (pte_t *) pmdp;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgdp;
pud_t *pudp;
diff --git a/arch/sh/mm/hugetlbpage.c b/arch/sh/mm/hugetlbpage.c
index cc948db74878..d2412d2d6462 100644
--- a/arch/sh/mm/hugetlbpage.c
+++ b/arch/sh/mm/hugetlbpage.c
@@ -42,7 +42,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
return pte;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgd;
pud_t *pud;
diff --git a/arch/sparc/mm/hugetlbpage.c b/arch/sparc/mm/hugetlbpage.c
index 7c29d38e6b99..8989c5e155b3 100644
--- a/arch/sparc/mm/hugetlbpage.c
+++ b/arch/sparc/mm/hugetlbpage.c
@@ -277,7 +277,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
return pte;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgd;
pud_t *pud;
diff --git a/arch/tile/mm/hugetlbpage.c b/arch/tile/mm/hugetlbpage.c
index cb10153b5c9f..1f0993945521 100644
--- a/arch/tile/mm/hugetlbpage.c
+++ b/arch/tile/mm/hugetlbpage.c
@@ -102,7 +102,8 @@ static pte_t *get_pte(pte_t *base, int index, int level)
return ptep;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgd;
pud_t *pud;
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 302f43fd9c28..ccf509063dfd 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -33,7 +33,7 @@ follow_huge_addr(struct mm_struct *mm, unsigned long address, int write)
if (!vma || !is_vm_hugetlb_page(vma))
return ERR_PTR(-EINVAL);
- pte = huge_pte_offset(mm, address);
+ pte = huge_pte_offset(mm, address, vma_mmu_pagesize(vma));
/* hugetlb should be locked, and hence, prefaulted */
WARN_ON(!pte || pte_none(*pte));
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index f7555fc25877..7b9c94837895 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -214,6 +214,7 @@ static inline struct uffd_msg userfault_msg(unsigned long address,
* hugepmd ranges.
*/
static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
+ struct vm_area_struct *vma,
unsigned long address,
unsigned long flags,
unsigned long reason)
@@ -224,7 +225,7 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
VM_BUG_ON(!rwsem_is_locked(&mm->mmap_sem));
- pte = huge_pte_offset(mm, address);
+ pte = huge_pte_offset(mm, address, vma_mmu_pagesize(vma));
if (!pte)
goto out;
@@ -243,6 +244,7 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
}
#else
static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
+ struct vm_area_struct *vma,
unsigned long address,
unsigned long flags,
unsigned long reason)
@@ -435,7 +437,8 @@ int handle_userfault(struct vm_fault *vmf, unsigned long reason)
must_wait = userfaultfd_must_wait(ctx, vmf->address, vmf->flags,
reason);
else
- must_wait = userfaultfd_huge_must_wait(ctx, vmf->address,
+ must_wait = userfaultfd_huge_must_wait(ctx, vmf->vma,
+ vmf->address,
vmf->flags, reason);
up_read(&mm->mmap_sem);
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index b857fc8cc2ec..23010a3b2047 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -113,7 +113,8 @@ extern struct list_head huge_boot_pages;
pte_t *huge_pte_alloc(struct mm_struct *mm,
unsigned long addr, unsigned long sz);
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr);
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz);
int huge_pmd_unshare(struct mm_struct *mm, unsigned long *addr, pte_t *ptep);
struct page *follow_huge_addr(struct mm_struct *mm, unsigned long address,
int write);
@@ -157,7 +158,7 @@ static inline void hugetlb_show_meminfo(void)
#define hugetlb_fault(mm, vma, addr, flags) ({ BUG(); 0; })
#define hugetlb_mcopy_atomic_pte(dst_mm, dst_pte, dst_vma, dst_addr, \
src_addr, pagep) ({ BUG(); 0; })
-#define huge_pte_offset(mm, address) 0
+#define huge_pte_offset(mm, address, sz) 0
static inline int dequeue_hwpoisoned_huge_page(struct page *page)
{
return 0;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index e5828875f7bb..0e4d1fb3122f 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3233,7 +3233,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
for (addr = vma->vm_start; addr < vma->vm_end; addr += sz) {
spinlock_t *src_ptl, *dst_ptl;
- src_pte = huge_pte_offset(src, addr);
+ src_pte = huge_pte_offset(src, addr, sz);
if (!src_pte)
continue;
dst_pte = huge_pte_alloc(dst, addr, sz);
@@ -3317,7 +3317,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end);
address = start;
for (; address < end; address += sz) {
- ptep = huge_pte_offset(mm, address);
+ ptep = huge_pte_offset(mm, address, sz);
if (!ptep)
continue;
@@ -3535,7 +3535,8 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
unmap_ref_private(mm, vma, old_page, address);
BUG_ON(huge_pte_none(pte));
spin_lock(ptl);
- ptep = huge_pte_offset(mm, address & huge_page_mask(h));
+ ptep = huge_pte_offset(mm, address & huge_page_mask(h),
+ huge_page_size(h));
if (likely(ptep &&
pte_same(huge_ptep_get(ptep), pte)))
goto retry_avoidcopy;
@@ -3574,7 +3575,8 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
* before the page tables are altered
*/
spin_lock(ptl);
- ptep = huge_pte_offset(mm, address & huge_page_mask(h));
+ ptep = huge_pte_offset(mm, address & huge_page_mask(h),
+ huge_page_size(h));
if (likely(ptep && pte_same(huge_ptep_get(ptep), pte))) {
ClearPagePrivate(new_page);
@@ -3861,7 +3863,7 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
address &= huge_page_mask(h);
- ptep = huge_pte_offset(mm, address);
+ ptep = huge_pte_offset(mm, address, huge_page_size(h));
if (ptep) {
entry = huge_ptep_get(ptep);
if (unlikely(is_hugetlb_entry_migration(entry))) {
@@ -4118,7 +4120,8 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
*
* Note that page table lock is not held when pte is null.
*/
- pte = huge_pte_offset(mm, vaddr & huge_page_mask(h));
+ pte = huge_pte_offset(mm, vaddr & huge_page_mask(h),
+ huge_page_size(h));
if (pte)
ptl = huge_pte_lock(h, mm, pte);
absent = !pte || huge_pte_none(huge_ptep_get(pte));
@@ -4252,7 +4255,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
i_mmap_lock_write(vma->vm_file->f_mapping);
for (; address < end; address += huge_page_size(h)) {
spinlock_t *ptl;
- ptep = huge_pte_offset(mm, address);
+ ptep = huge_pte_offset(mm, address, huge_page_size(h));
if (!ptep)
continue;
ptl = huge_pte_lock(h, mm, ptep);
@@ -4516,7 +4519,8 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
saddr = page_table_shareable(svma, vma, addr, idx);
if (saddr) {
- spte = huge_pte_offset(svma->vm_mm, saddr);
+ spte = huge_pte_offset(svma->vm_mm, saddr,
+ vma_mmu_pagesize(svma));
if (spte) {
get_page(virt_to_page(spte));
break;
@@ -4612,7 +4616,8 @@ pte_t *huge_pte_alloc(struct mm_struct *mm,
return pte;
}
-pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+pte_t *huge_pte_offset(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
{
pgd_t *pgd;
p4d_t *p4d;
diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index de9c40d7304a..8ec6ba230bb9 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -116,7 +116,8 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
if (unlikely(PageHuge(pvmw->page))) {
/* when pud is not present, pte will be NULL */
- pvmw->pte = huge_pte_offset(mm, pvmw->address);
+ pvmw->pte = huge_pte_offset(mm, pvmw->address,
+ PAGE_SIZE << compound_order(page));
if (!pvmw->pte)
return false;
diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index 60f7856e508f..1a4197965415 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -180,12 +180,13 @@ static int walk_hugetlb_range(unsigned long addr, unsigned long end,
struct hstate *h = hstate_vma(vma);
unsigned long next;
unsigned long hmask = huge_page_mask(h);
+ unsigned long sz = huge_page_size(h);
pte_t *pte;
int err = 0;
do {
next = hugetlb_entry_end(h, addr, end);
- pte = huge_pte_offset(walk->mm, addr & hmask);
+ pte = huge_pte_offset(walk->mm, addr & hmask, sz);
if (pte && walk->hugetlb_entry)
err = walk->hugetlb_entry(pte, hmask, addr, next, walk);
if (err)
--
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v3 2/6] mm, gup: Ensure real head page is ref-counted when using hugepages
From: Punit Agrawal @ 2017-05-22 13:36 UTC (permalink / raw)
To: akpm
Cc: Punit Agrawal, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
aneesh.kumar, Michal Hocko
In-Reply-To: <20170522133604.11392-1-punit.agrawal@arm.com>
When speculatively taking references to a hugepage using
page_cache_add_speculative() in gup_huge_pmd(), it is assumed that the
page returned by pmd_page() is the head page. Although normally true,
this assumption doesn't hold when the hugepage comprises of successive
page table entries such as when using contiguous bit on arm64 at PTE or
PMD levels.
This can be addressed by ensuring that the page passed to
page_cache_add_speculative() is the real head or by de-referencing the
head page within the function.
We take the first approach to keep the usage pattern aligned with
page_cache_get_speculative() where users already pass the appropriate
page, i.e., the de-referenced head.
Apply the same logic to fix gup_huge_[pud|pgd]() as well.
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Steve Capper <steve.capper@arm.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
mm/gup.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index ccf8cb38234f..be67996513be 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1358,8 +1358,7 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
return __gup_device_huge_pmd(orig, addr, end, pages, nr);
refs = 0;
- head = pmd_page(orig);
- page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
+ page = pmd_page(orig) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
do {
pages[*nr] = page;
(*nr)++;
@@ -1367,6 +1366,7 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
refs++;
} while (addr += PAGE_SIZE, addr != end);
+ head = compound_head(page);
if (!page_cache_add_speculative(head, refs)) {
*nr -= refs;
return 0;
@@ -1396,8 +1396,7 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
return __gup_device_huge_pud(orig, addr, end, pages, nr);
refs = 0;
- head = pud_page(orig);
- page = head + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
+ page = pud_page(orig) + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
do {
pages[*nr] = page;
(*nr)++;
@@ -1405,6 +1404,7 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
refs++;
} while (addr += PAGE_SIZE, addr != end);
+ head = compound_head(page);
if (!page_cache_add_speculative(head, refs)) {
*nr -= refs;
return 0;
@@ -1433,8 +1433,7 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
BUILD_BUG_ON(pgd_devmap(orig));
refs = 0;
- head = pgd_page(orig);
- page = head + ((addr & ~PGDIR_MASK) >> PAGE_SHIFT);
+ page = pgd_page(orig) + ((addr & ~PGDIR_MASK) >> PAGE_SHIFT);
do {
pages[*nr] = page;
(*nr)++;
@@ -1442,6 +1441,7 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
refs++;
} while (addr += PAGE_SIZE, addr != end);
+ head = compound_head(page);
if (!page_cache_add_speculative(head, refs)) {
*nr -= refs;
return 0;
--
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v3 1/6] mm, gup: Remove broken VM_BUG_ON_PAGE compound check for hugepages
From: Punit Agrawal @ 2017-05-22 13:35 UTC (permalink / raw)
To: akpm
Cc: Will Deacon, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, n-horiguchi, kirill.shutemov, mike.kravetz,
steve.capper, mark.rutland, hillf.zj, linux-arch, aneesh.kumar,
Punit Agrawal
In-Reply-To: <20170522133604.11392-1-punit.agrawal@arm.com>
From: Will Deacon <will.deacon@arm.com>
When operating on hugepages with DEBUG_VM enabled, the GUP code checks the
compound head for each tail page prior to calling page_cache_add_speculative.
This is broken, because on the fast-GUP path (where we don't hold any page
table locks) we can be racing with a concurrent invocation of
split_huge_page_to_list.
split_huge_page_to_list deals with this race by using page_ref_freeze to
freeze the page and force concurrent GUPs to fail whilst the component
pages are modified. This modification includes clearing the compound_head
field for the tail pages, so checking this prior to a successful call
to page_cache_add_speculative can lead to false positives: In fact,
page_cache_add_speculative *already* has this check once the page refcount
has been successfully updated, so we can simply remove the broken calls
to VM_BUG_ON_PAGE.
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Acked-by: Steve Capper <steve.capper@arm.com>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
mm/gup.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index d9e6fddcc51f..ccf8cb38234f 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1361,7 +1361,6 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
head = pmd_page(orig);
page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
do {
- VM_BUG_ON_PAGE(compound_head(page) != head, page);
pages[*nr] = page;
(*nr)++;
page++;
@@ -1400,7 +1399,6 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
head = pud_page(orig);
page = head + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
do {
- VM_BUG_ON_PAGE(compound_head(page) != head, page);
pages[*nr] = page;
(*nr)++;
page++;
@@ -1438,7 +1436,6 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
head = pgd_page(orig);
page = head + ((addr & ~PGDIR_MASK) >> PAGE_SHIFT);
do {
- VM_BUG_ON_PAGE(compound_head(page) != head, page);
pages[*nr] = page;
(*nr)++;
page++;
--
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v3 0/6] Support for contiguous pte hugepages
From: Punit Agrawal @ 2017-05-22 13:35 UTC (permalink / raw)
To: akpm
Cc: Punit Agrawal, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
aneesh.kumar
Hi,
This patchset updates the hugetlb code to fix issues arising from
contiguous pte hugepages (such as on arm64). These are the generic
code changes and the arm64 support based on these patches will be
posted separately. The patches are based on v4.12-rc2. Previous
related postings can be found at [0], [1] and [2].
The patches fall into two categories -
* Patches 1-2 address issues with gup
* Patches 3-6 relate to passing a size argument to hugepage helpers to
disambiguate the size of the referred page. These changes are
required to enable arch code to properly handle swap entries for
contiguous pte hugepages.
The changes to huge_pte_offset() (patch 3) touch multiple
architectures but I've managed to minimise these changes for the
other affected functions - huge_pte_clear() and set_huge_pte_at().
These patches gate the enabling of contiguous hugepages support on
arm64 which has been requested for systems using !4k page granule.
Feedback welcome.
Thanks,
Punit
v2 -> v3
* Added gup fixes
v1 -> v2
* switch huge_pte_offset() to use size instead of hstate for
consistency with the rest of the api
* Expand the series to address huge_pte_clear() and set_huge_pte_at()
RFC -> v1
* Fixed a missing conversion of huge_pte_offset() prototype to add
hstate parameter. Reported by 0-day.
[0] https://lkml.org/lkml/2017/3/23/293
[1] https://lkml.org/lkml/2017/3/30/770
[2] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1370686.html
Punit Agrawal (5):
mm, gup: Ensure real head page is ref-counted when using hugepages
mm/hugetlb: add size parameter to huge_pte_offset()
mm/hugetlb: Allow architectures to override huge_pte_clear()
mm/hugetlb: Introduce set_huge_swap_pte_at() helper
mm: rmap: Use correct helper when poisoning hugepages
Will Deacon (1):
mm, gup: Remove broken VM_BUG_ON_PAGE compound check for hugepages
arch/arm64/mm/hugetlbpage.c | 3 ++-
arch/ia64/mm/hugetlbpage.c | 4 ++--
arch/metag/mm/hugetlbpage.c | 3 ++-
arch/mips/mm/hugetlbpage.c | 3 ++-
arch/parisc/mm/hugetlbpage.c | 3 ++-
arch/powerpc/mm/hugetlbpage.c | 2 +-
arch/s390/include/asm/hugetlb.h | 10 ++-------
arch/s390/mm/hugetlbpage.c | 12 ++++++++++-
arch/sh/mm/hugetlbpage.c | 3 ++-
arch/sparc/mm/hugetlbpage.c | 3 ++-
arch/tile/mm/hugetlbpage.c | 3 ++-
arch/x86/mm/hugetlbpage.c | 2 +-
fs/userfaultfd.c | 7 +++++--
include/asm-generic/hugetlb.h | 7 ++-----
include/linux/hugetlb.h | 7 +++++--
mm/gup.c | 15 ++++++--------
mm/hugetlb.c | 45 +++++++++++++++++++++++++++++------------
mm/page_vma_mapped.c | 3 ++-
mm/pagewalk.c | 3 ++-
mm/rmap.c | 7 +++++--
20 files changed, 90 insertions(+), 55 deletions(-)
--
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE
From: Mike Rapoport @ 2017-05-22 13:36 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, Arnd Bergmann, Kirill A. Shutemov,
Andrea Arcangeli, Pavel Emelyanov, linux-mm, lkml
In-Reply-To: <20170522114243.2wrdbncilozygbpl@node.shutemov.name>
On Mon, May 22, 2017 at 02:42:43PM +0300, Kirill A. Shutemov wrote:
> On Mon, May 22, 2017 at 09:12:42AM +0300, Mike Rapoport wrote:
> > Currently applications can explicitly enable or disable THP for a memory
> > region using MADV_HUGEPAGE or MADV_NOHUGEPAGE. However, once either of
> > these advises is used, the region will always have
> > VM_HUGEPAGE/VM_NOHUGEPAGE flag set in vma->vm_flags.
> > The MADV_CLR_HUGEPAGE resets both these flags and allows managing THP in
> > the region according to system-wide settings.
>
> Seems reasonable. But could you describe an use-case when it's useful in
> real world.
My use-case was combination of pre- and post-copy migration of containers
with CRIU.
In this case we populate a part of a memory region with data that was saved
during the pre-copy stage. Afterwards, the region is registered with
userfaultfd and we expect to get page faults for the parts of the region
that were not yet populated. However, khugepaged collapses the pages and
the page faults we would expect do not occur.
We could have used MADV_NOHUGEPAGE before populating the region with the
pre-copy data, but then, in the end, the restored application will be resumed
with vma->vm_flags different from the ones it had when it was frozen.
Another possibility I've considered was to register the region with
userfaultfd before populating it with data, but in that case we get the
overhead of UFFD_EVENT_PAGEFAULT + UFFDIO_{COPY,ZEROPAGE} for nothing :(
> And the name is bad. But I don't have better suggestion. At least do not
> abbreviate CLEAR. Saving two letters doesn't worth it.
>
> Maybe RESET instead?
I hesitated between CLR and RST, and CLR was chosen pretty much with coin
toss :)
I'm ok with RESET, which might be a bit more descriptive than CLEAR.
> --
> Kirill A. Shutemov
>
--
Sincerely yours,
Mike.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] x86/mm: pgds getting out of sync after memory hot remove
From: Kirill A. Shutemov @ 2017-05-22 13:12 UTC (permalink / raw)
To: Jérôme Glisse
Cc: linux-kernel, linux-mm, Kirill A. Shutemov, Andrew Morton,
Ingo Molnar, Michal Hocko, Mel Gorman
In-Reply-To: <1495216887-3175-1-git-send-email-jglisse@redhat.com>
On Fri, May 19, 2017 at 02:01:26PM -0400, Jerome Glisse wrote:
> After memory hot remove it seems we do not synchronize pgds for kernel
> virtual memory range (on vmemmap_free()). This seems bogus to me as it
> means we are left with stall entry for process with mm != mm_init
>
> Yet i am puzzle by the fact that i am only now hitting this issue. It
> never was an issue with 4.12 or before ie HMM never triggered following
> BUG_ON inside sync_global_pgds():
>
> if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
> BUG_ON(p4d_page_vaddr(*p4d) != p4d_page_vaddr(*p4d_ref));
>
>
> It seems that Kirill 5 level page table changes play a role in this
> behavior change. I could not bisect because HMM is painfull to rebase
> for each bisection step so that is just my best guess.
>
>
> Am i missing something here ? Am i wrong in assuming that should sync
> pgd on vmemmap_free() ? If so anyone have a good guess on why i am now
> seeing the above BUG_ON ?
What would we gain by syncing pgd on free? Stale pgds are fine as long as
they are not referenced (use-after-free case). Syncing is addtional work.
See af2cf278ef4f ("x86/mm/hotplug: Don't remove PGD entries in remove_pagetable()")
and 5372e155a28f ("x86/mm: Drop unused argument 'removed' from sync_global_pgds()").
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [v4 1/1] mm: Adaptive hash table scaling
From: Pasha Tatashin @ 2017-05-22 13:18 UTC (permalink / raw)
To: Michal Hocko; +Cc: akpm, linux-kernel, linux-mm, Michael Ellerman
In-Reply-To: <20170522092910.GD8509@dhcp22.suse.cz>
>
> I have only noticed this email today because my incoming emails stopped
> syncing since Friday. But this is _definitely_ not the right approachh.
> 64G for 32b systems is _way_ off. We have only ~1G for the kernel. I've
> already proposed scaling up to 32M for 32b systems and Andi seems to be
> suggesting the same. So can we fold or apply the following instead?
Hi Michal,
Thank you for your suggestion. I will update the patch.
64G base for 32bit systems is not meant to be ever used, as the adaptive
scaling for 32bit system is just not needed. 32M and 64G are going to be
exactly the same on such systems.
Here is theoretical limit for the max hash size of entries (dentry cache
example):
size of bucket: sizeof(struct hlist_bl_head) = 4 bytes
numentries: (1 << 32) / PAGE_SIZE = 1048576 (for 4K pages)
hash size: 4b * 1048576 = 4M
In practice it is going to be an order smaller, as number of kernel
pages is less then (1<<32).
However, I will apply your suggestions as there seems to be a problem of
overflowing in comparing ul vs. ull as reported by Michael Ellerman, and
having a large base on 32bit systems will solve this issue. I will
revert back to "ul" all the quantities.
Another approach is to make it a 64 bit only macro like this:
#if __BITS_PER_LONG > 32
#define ADAPT_SCALE_BASE (64ull << 30)
#define ADAPT_SCALE_SHIFT 2
#define ADAPT_SCALE_NPAGES (ADAPT_SCALE_BASE >> PAGE_SHIFT)
#define adapt_scale(high_limit, numentries, scalep)
if (!(high_limit)) { \
unsigned long adapt; \
for (adapt = ADAPT_SCALE_NPAGES; adapt < \
(numentries); adapt <<= ADAPT_SCALE_SHIFT) \
(*(scalep))++; \
}
#else
#define adapt_scale(high_limit, numentries scalep)
#endif
Pasha
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] dm ioctl: Restore __GFP_HIGH in copy_params()
From: Michal Hocko @ 2017-05-22 12:09 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Junaid Shahid, David Rientjes, Alasdair Kergon, Mike Snitzer,
Andrew Morton, linux-mm, andreslc, gthelen, vbabka, linux-kernel
In-Reply-To: <alpine.LRH.2.02.1705220759001.27401@file01.intranet.prod.int.rdu2.redhat.com>
On Mon 22-05-17 08:00:11, Mikulas Patocka wrote:
>
>
> On Mon, 22 May 2017, Michal Hocko wrote:
>
> > On Fri 19-05-17 19:43:23, Mikulas Patocka wrote:
> > >
> > >
> > > On Fri, 19 May 2017, Michal Hocko wrote:
> > >
> > > > On Thu 18-05-17 19:50:46, Junaid Shahid wrote:
> > > > > (Adding back the correct linux-mm email address and also adding linux-kernel.)
> > > > >
> > > > > On Thursday, May 18, 2017 01:41:33 PM David Rientjes wrote:
> > > > [...]
> > > > > > Let's ask Mikulas, who changed this from PF_MEMALLOC to __GFP_HIGH,
> > > > > > assuming there was a reason to do it in the first place in two different
> > > > > > ways.
> > > >
> > > > Hmm, the old PF_MEMALLOC used to have the following comment
> > > > /*
> > > > * Trying to avoid low memory issues when a device is
> > > > * suspended.
> > > > */
> > > >
> > > > I am not really sure what that means but __GFP_HIGH certainly have a
> > > > different semantic than PF_MEMALLOC. The later grants the full access to
> > > > the memory reserves while the prior on partial access. If this is _really_
> > > > needed then it deserves a comment explaining why.
> > > > --
> > > > Michal Hocko
> > > > SUSE Labs
> > >
> > > Sometimes, I/O to a device mapper device is blocked until the userspace
> > > daemon dmeventd does some action (for example, when dm-mirror leg fails,
> > > dmeventd needs to mark the leg as failed in the lvm metadata and then
> > > reload the device).
> > >
> > > The dmeventd daemon mlocks itself in memory so that it doesn't generate
> > > any I/O. But it must be able to call ioctls. __GFP_HIGH is there so that
> > > the ioctls issued by dmeventd have higher chance of succeeding if some I/O
> > > is blocked, waiting for dmeventd action. It reduces the possibility of
> > > low-memory-deadlock, though it doesn't eliminate it entirely.
> >
> > So what happens if the memory reserves are depleted. Do we deadlock?
>
> Yes, it will deadlock.
That would be more than unfortunate and begs for a different solution.
The thing is that __GFP_HIGH is not propagated to all allocations in the
vmalloc proper. E.g. page table allocations are hardcoded GFP_KERNEL.
> > Why is OOM killer insufficient to allow the further progress?
>
> I don't know if the OOM killer will or won't be triggered in this
> situation, it depends on the people who wrote the OOM killer.
I am not sure I understand. OOM killer is invoked for _all_ allocations
<= PAGE_ALLOC_COSTLY_ORDER that do not have __GFP_NORETRY as long as the
OOM killer is not disabled (oom_killer_disable) and that only happens
from the PM suspend path which makes sure that no userspace is active at
the time. AFAIU this is a userspace triggered path and so the later
shouldn't apply to it and GFP_KERNEL should be therefore sufficient.
Relying to a portion of memory reserves to prevent from deadlock seems
fundamentaly broken to me.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] dm ioctl: Restore __GFP_HIGH in copy_params()
From: Mikulas Patocka @ 2017-05-22 12:00 UTC (permalink / raw)
To: Michal Hocko
Cc: Junaid Shahid, David Rientjes, Alasdair Kergon, Mike Snitzer,
Andrew Morton, linux-mm, andreslc, gthelen, vbabka, linux-kernel
In-Reply-To: <20170522093725.GF8509@dhcp22.suse.cz>
On Mon, 22 May 2017, Michal Hocko wrote:
> On Fri 19-05-17 19:43:23, Mikulas Patocka wrote:
> >
> >
> > On Fri, 19 May 2017, Michal Hocko wrote:
> >
> > > On Thu 18-05-17 19:50:46, Junaid Shahid wrote:
> > > > (Adding back the correct linux-mm email address and also adding linux-kernel.)
> > > >
> > > > On Thursday, May 18, 2017 01:41:33 PM David Rientjes wrote:
> > > [...]
> > > > > Let's ask Mikulas, who changed this from PF_MEMALLOC to __GFP_HIGH,
> > > > > assuming there was a reason to do it in the first place in two different
> > > > > ways.
> > >
> > > Hmm, the old PF_MEMALLOC used to have the following comment
> > > /*
> > > * Trying to avoid low memory issues when a device is
> > > * suspended.
> > > */
> > >
> > > I am not really sure what that means but __GFP_HIGH certainly have a
> > > different semantic than PF_MEMALLOC. The later grants the full access to
> > > the memory reserves while the prior on partial access. If this is _really_
> > > needed then it deserves a comment explaining why.
> > > --
> > > Michal Hocko
> > > SUSE Labs
> >
> > Sometimes, I/O to a device mapper device is blocked until the userspace
> > daemon dmeventd does some action (for example, when dm-mirror leg fails,
> > dmeventd needs to mark the leg as failed in the lvm metadata and then
> > reload the device).
> >
> > The dmeventd daemon mlocks itself in memory so that it doesn't generate
> > any I/O. But it must be able to call ioctls. __GFP_HIGH is there so that
> > the ioctls issued by dmeventd have higher chance of succeeding if some I/O
> > is blocked, waiting for dmeventd action. It reduces the possibility of
> > low-memory-deadlock, though it doesn't eliminate it entirely.
>
> So what happens if the memory reserves are depleted. Do we deadlock?
Yes, it will deadlock.
> Why is OOM killer insufficient to allow the further progress?
I don't know if the OOM killer will or won't be triggered in this
situation, it depends on the people who wrote the OOM killer.
> --
> Michal Hocko
> SUSE Labs
Mikulas
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox