Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1646543 > unrolled thread

[PATCH] mm: introduce MADV_CLR_HUGEPAGE

Back to article view | Back to linux.kernel

Contents

  [PATCH] mm: introduce MADV_CLR_HUGEPAGE "Mike Rapoport" <rppt@linux.vnet.ibm.com> - 2017-05-22 08:20 +0200
    Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-05-22 09:30 +0200
      Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-22 10:20 +0200
    Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-05-22 13:50 +0200
      Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-22 15:40 +0200
        Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-05-22 15:50 +0200
        Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Michal Hocko <mhocko@kernel.org> - 2017-05-22 16:00 +0200
          Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-22 16:30 +0200
            Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Vlastimil Babka <vbabka@suse.cz> - 2017-05-22 18:00 +0200
              Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-22 20:00 +0200
              Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-24 10:00 +0200
                Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Vlastimil Babka <vbabka@suse.cz> - 2017-05-24 10:10 +0200
                  Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-24 12:50 +0200
                    Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Michal Hocko <mhocko@kernel.org> - 2017-05-24 13:20 +0200
                      Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-24 16:30 +0200
                        Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Andrea Arcangeli <aarcange@redhat.com> - 2017-05-24 17:30 +0200
                    Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Vlastimil Babka <vbabka@suse.cz> - 2017-05-24 13:40 +0200
                      Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Vlastimil Babka <vbabka@suse.cz> - 2017-05-24 17:00 +0200
                        Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE Mike Rapoport <rppt@linux.vnet.ibm.com> - 2017-05-24 17:20 +0200

#1646543 — [PATCH] mm: introduce MADV_CLR_HUGEPAGE

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.

Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
---
 include/uapi/asm-generic/mman-common.h | 3 +++
 mm/khugepaged.c                        | 7 +++++++
 mm/madvise.c                           | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index 8c27db0..3201712 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -58,6 +58,9 @@
 					   overrides the coredump filter bits */
 #define MADV_DODUMP	17		/* Clear the MADV_DONTDUMP flag */
 
+#define MADV_CLR_HUGEPAGE 18		/* Clear flags controlling backing with
+					   hugepages */
+
 /* compatibility flags */
 #define MAP_FILE	0
 
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 945fd1c..b9ee9bb 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -336,6 +336,13 @@ int hugepage_madvise(struct vm_area_struct *vma,
 		 * it got registered before VM_NOHUGEPAGE was set.
 		 */
 		break;
+	case MADV_CLR_HUGEPAGE:
+		*vm_flags &= ~(VM_HUGEPAGE | VM_NOHUGEPAGE);
+		/*
+		 * The vma will be treated according to the
+		 * system-wide settings in transparent_hugepage_flags
+		 */
+		break;
 	}
 
 	return 0;
diff --git a/mm/madvise.c b/mm/madvise.c
index 25b78ee..ae650a3 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -105,6 +105,7 @@ static long madvise_behavior(struct vm_area_struct *vma,
 		break;
 	case MADV_HUGEPAGE:
 	case MADV_NOHUGEPAGE:
+	case MADV_CLR_HUGEPAGE:
 		error = hugepage_madvise(vma, &new_flags, behavior);
 		if (error) {
 			/*
@@ -684,6 +685,7 @@ madvise_behavior_valid(int behavior)
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	case MADV_HUGEPAGE:
 	case MADV_NOHUGEPAGE:
+	case MADV_CLR_HUGEPAGE:
 #endif
 	case MADV_DONTDUMP:
 	case MADV_DODUMP:
@@ -739,6 +741,9 @@ madvise_behavior_valid(int behavior)
  *  MADV_NOHUGEPAGE - mark the given range as not worth being backed by
  *		transparent huge pages so the existing pages will not be
  *		coalesced into THP and new pages will not be allocated as THP.
+ *  MADV_CLR_HUGEPAGE - clear MADV_HUGEPAGE/MADV_NOHUGEPAGE marking;
+ *		the range will be treated by khugepaged according to the
+ *		system wide settings
  *  MADV_DONTDUMP - the application wants to prevent pages in the given range
  *		from being included in its core dump.
  *  MADV_DODUMP - cancel MADV_DONTDUMP: no longer exclude from core dump.
-- 
2.7.4

[toc] | [next] | [standalone]

#1646564

On 05/22/2017 11:42 AM, 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.

Invoking madvise() for the first time with either MADV_HUGEPAGE or
MADV_NOHUGEPAGE on the buffer will unsubscribe it from the system
wide behavior for good. I am not saying we should not have a way
to put it back into system wide mode but are there no other functions
through madvise() or any other interface which may have the same
situation.

[toc] | [prev] | [next] | [standalone]

#1646593

On Mon, May 22, 2017 at 12:56:45PM +0530, Anshuman Khandual wrote:
> On 05/22/2017 11:42 AM, 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.
> 
> Invoking madvise() for the first time with either MADV_HUGEPAGE or
> MADV_NOHUGEPAGE on the buffer will unsubscribe it from the system
> wide behavior for good. I am not saying we should not have a way
> to put it back into system wide mode but are there no other functions
> through madvise() or any other interface which may have the same
> situation.

There are madvise() interfaces that set or clear some of the vma->vm_flags,
e.g MADV_*FORK or MADV_*DUMP. The difference with MADV_*HUGEPAGE is that
it is using two flags and with current madvise() interface either of them
has to be set, but there is no interface to clear them both.

--
Sincerely yours,
Mike. 

[toc] | [prev] | [next] | [standalone]

#1646775

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.

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?

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1646908

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.

[toc] | [prev] | [next] | [standalone]

#1646916

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

[toc] | [prev] | [next] | [standalone]

#1646920

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

[toc] | [prev] | [next] | [standalone]

#1646973

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.

[toc] | [prev] | [next] | [standalone]

#1647081

On 05/22/2017 04:29 PM, Mike Rapoport wrote:
> 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.

Hmm, I wouldn't be that sure if this is indeed race-free. Check that
this scenario is indeed impossible?

- you do the mmap
- khugepaged will choose the process' mm to scan
- khugepaged will get to the vma in question, it doesn't have
MADV_NOHUGEPAGE yet
- you set MADV_NOHUGEPAGE on the vma
- you start populating the vma
- khugepaged sees the vma is non-empty, collapses

unless I'm wrong, the racers will have mmap_sem for reading only when
setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.

However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
nonzero value, the flag is set, otherwise it is cleared." says the
manpage. Do it before the mmap and you avoid the race as well?

> 
>> -- 
>> 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>
> 

[toc] | [prev] | [next] | [standalone]

#1647228

On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
> > 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.
> 
> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
> this scenario is indeed impossible?
> 
> - you do the mmap
> - khugepaged will choose the process' mm to scan
> - khugepaged will get to the vma in question, it doesn't have
> MADV_NOHUGEPAGE yet
> - you set MADV_NOHUGEPAGE on the vma
> - you start populating the vma
> - khugepaged sees the vma is non-empty, collapses
> 
> unless I'm wrong, the racers will have mmap_sem for reading only when
> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.

madvise(MADV_*HUGEPAGE) takes mmap_sem for writing, so it is safe.
 
> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
> nonzero value, the flag is set, otherwise it is cleared." says the
> manpage. Do it before the mmap and you avoid the race as well?

I've missed that one, thanks Vlastimil!

[toc] | [prev] | [next] | [standalone]

#1649209

On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
> > 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.
> 
> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
> this scenario is indeed impossible?
> 
> - you do the mmap
> - khugepaged will choose the process' mm to scan
> - khugepaged will get to the vma in question, it doesn't have
> MADV_NOHUGEPAGE yet
> - you set MADV_NOHUGEPAGE on the vma
> - you start populating the vma
> - khugepaged sees the vma is non-empty, collapses
> 
> unless I'm wrong, the racers will have mmap_sem for reading only when
> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.
> 
> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
> nonzero value, the flag is set, otherwise it is cleared." says the
> manpage. Do it before the mmap and you avoid the race as well?

Unfortunately, prctl(PR_SET_THP_DISABLE) didn't help :(
When I've tried to use it, I've ended up with VM_NOHUGEPAGE set on all VMAs
created after prctl(). This returns me to the state when checkpoint-restore
alters the application vma->vm_flags although it shouldn't and I do not see
a way to fix it using existing interfaces.

--
Sincerely yours,
Mike. 

[toc] | [prev] | [next] | [standalone]

#1649216

On 05/24/2017 09:50 AM, Mike Rapoport wrote:
> On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
>> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
>>> 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.
>>
>> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
>> this scenario is indeed impossible?
>>
>> - you do the mmap
>> - khugepaged will choose the process' mm to scan
>> - khugepaged will get to the vma in question, it doesn't have
>> MADV_NOHUGEPAGE yet
>> - you set MADV_NOHUGEPAGE on the vma
>> - you start populating the vma
>> - khugepaged sees the vma is non-empty, collapses
>>
>> unless I'm wrong, the racers will have mmap_sem for reading only when
>> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.
>>
>> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
>> nonzero value, the flag is set, otherwise it is cleared." says the
>> manpage. Do it before the mmap and you avoid the race as well?
> 
> Unfortunately, prctl(PR_SET_THP_DISABLE) didn't help :(
> When I've tried to use it, I've ended up with VM_NOHUGEPAGE set on all VMAs
> created after prctl(). This returns me to the state when checkpoint-restore
> alters the application vma->vm_flags although it shouldn't and I do not see
> a way to fix it using existing interfaces.

[CC linux-api, should have been done in the initial posting already]

Hm so the prctl does:

                if (arg2)
                        me->mm->def_flags |= VM_NOHUGEPAGE;
                else
                        me->mm->def_flags &= ~VM_NOHUGEPAGE;

That's rather lazy implementation IMHO. Could we change it so the flag
is stored elsewhere in the mm, and the code that decides to (not) use
THP will check both the per-vma flag and the per-mm flag?

> --
> Sincerely yours,
> Mike. 
> 

[toc] | [prev] | [next] | [standalone]

#1649455

On Wed, May 24, 2017 at 09:58:06AM +0200, Vlastimil Babka wrote:
> On 05/24/2017 09:50 AM, Mike Rapoport wrote:
> > On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
> >> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
> >>>
> >>> 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.
> >>
> >> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
> >> this scenario is indeed impossible?
> >>
> >> - you do the mmap
> >> - khugepaged will choose the process' mm to scan
> >> - khugepaged will get to the vma in question, it doesn't have
> >> MADV_NOHUGEPAGE yet
> >> - you set MADV_NOHUGEPAGE on the vma
> >> - you start populating the vma
> >> - khugepaged sees the vma is non-empty, collapses
> >>
> >> unless I'm wrong, the racers will have mmap_sem for reading only when
> >> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.
> >>
> >> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
> >> nonzero value, the flag is set, otherwise it is cleared." says the
> >> manpage. Do it before the mmap and you avoid the race as well?
> > 
> > Unfortunately, prctl(PR_SET_THP_DISABLE) didn't help :(
> > When I've tried to use it, I've ended up with VM_NOHUGEPAGE set on all VMAs
> > created after prctl(). This returns me to the state when checkpoint-restore
> > alters the application vma->vm_flags although it shouldn't and I do not see
> > a way to fix it using existing interfaces.
> 
> [CC linux-api, should have been done in the initial posting already]

Sorry, missed that.
 
> Hm so the prctl does:
> 
>                 if (arg2)
>                         me->mm->def_flags |= VM_NOHUGEPAGE;
>                 else
>                         me->mm->def_flags &= ~VM_NOHUGEPAGE;
> 
> That's rather lazy implementation IMHO. Could we change it so the flag
> is stored elsewhere in the mm, and the code that decides to (not) use
> THP will check both the per-vma flag and the per-mm flag?

I afraid I don't understand how that can help.
What we need is an ability to temporarily disable collapse of the pages in
VMAs that do not have VM_*HUGEPAGE flags set and that after we re-enable
THP, the vma->vm_flags for those VMAs will remain intact.

--
Sincerely yours,
Mike.

[toc] | [prev] | [next] | [standalone]

#1649479

On Wed 24-05-17 13:39:48, Mike Rapoport wrote:
> On Wed, May 24, 2017 at 09:58:06AM +0200, Vlastimil Babka wrote:
> > On 05/24/2017 09:50 AM, Mike Rapoport wrote:
> > > On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
> > >> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
> > >>>
> > >>> 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.
> > >>
> > >> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
> > >> this scenario is indeed impossible?
> > >>
> > >> - you do the mmap
> > >> - khugepaged will choose the process' mm to scan
> > >> - khugepaged will get to the vma in question, it doesn't have
> > >> MADV_NOHUGEPAGE yet
> > >> - you set MADV_NOHUGEPAGE on the vma
> > >> - you start populating the vma
> > >> - khugepaged sees the vma is non-empty, collapses
> > >>
> > >> unless I'm wrong, the racers will have mmap_sem for reading only when
> > >> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.
> > >>
> > >> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
> > >> nonzero value, the flag is set, otherwise it is cleared." says the
> > >> manpage. Do it before the mmap and you avoid the race as well?
> > > 
> > > Unfortunately, prctl(PR_SET_THP_DISABLE) didn't help :(
> > > When I've tried to use it, I've ended up with VM_NOHUGEPAGE set on all VMAs
> > > created after prctl(). This returns me to the state when checkpoint-restore
> > > alters the application vma->vm_flags although it shouldn't and I do not see
> > > a way to fix it using existing interfaces.
> > 
> > [CC linux-api, should have been done in the initial posting already]
> 
> Sorry, missed that.
>  
> > Hm so the prctl does:
> > 
> >                 if (arg2)
> >                         me->mm->def_flags |= VM_NOHUGEPAGE;
> >                 else
> >                         me->mm->def_flags &= ~VM_NOHUGEPAGE;
> > 
> > That's rather lazy implementation IMHO. Could we change it so the flag
> > is stored elsewhere in the mm, and the code that decides to (not) use
> > THP will check both the per-vma flag and the per-mm flag?
> 
> I afraid I don't understand how that can help.
> What we need is an ability to temporarily disable collapse of the pages in
> VMAs that do not have VM_*HUGEPAGE flags set and that after we re-enable
> THP, the vma->vm_flags for those VMAs will remain intact.

Why cannot khugepaged simply skip over all VMAs which have userfault
regions registered? This would sound like a less error prone approach to
me.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1649659

On Wed, May 24, 2017 at 01:18:00PM +0200, Michal Hocko wrote:
> On Wed 24-05-17 13:39:48, Mike Rapoport wrote:
> > On Wed, May 24, 2017 at 09:58:06AM +0200, Vlastimil Babka wrote:
> > > On 05/24/2017 09:50 AM, Mike Rapoport wrote:
> > > > On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
> > > >> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
> > > >>>
> > > >>> 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.
> > > >>
> > > >> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
> > > >> this scenario is indeed impossible?
> > > >>
> > > >> - you do the mmap
> > > >> - khugepaged will choose the process' mm to scan
> > > >> - khugepaged will get to the vma in question, it doesn't have
> > > >> MADV_NOHUGEPAGE yet
> > > >> - you set MADV_NOHUGEPAGE on the vma
> > > >> - you start populating the vma
> > > >> - khugepaged sees the vma is non-empty, collapses
> > > >>
> > > >> unless I'm wrong, the racers will have mmap_sem for reading only when
> > > >> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.
> > > >>
> > > >> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
> > > >> nonzero value, the flag is set, otherwise it is cleared." says the
> > > >> manpage. Do it before the mmap and you avoid the race as well?
> > > > 
> > > > Unfortunately, prctl(PR_SET_THP_DISABLE) didn't help :(
> > > > When I've tried to use it, I've ended up with VM_NOHUGEPAGE set on all VMAs
> > > > created after prctl(). This returns me to the state when checkpoint-restore
> > > > alters the application vma->vm_flags although it shouldn't and I do not see
> > > > a way to fix it using existing interfaces.
> > > 
> > > [CC linux-api, should have been done in the initial posting already]
> > 
> > Sorry, missed that.
> >  
> > > Hm so the prctl does:
> > > 
> > >                 if (arg2)
> > >                         me->mm->def_flags |= VM_NOHUGEPAGE;
> > >                 else
> > >                         me->mm->def_flags &= ~VM_NOHUGEPAGE;
> > > 
> > > That's rather lazy implementation IMHO. Could we change it so the flag
> > > is stored elsewhere in the mm, and the code that decides to (not) use
> > > THP will check both the per-vma flag and the per-mm flag?
> > 
> > I afraid I don't understand how that can help.
> > What we need is an ability to temporarily disable collapse of the pages in
> > VMAs that do not have VM_*HUGEPAGE flags set and that after we re-enable
> > THP, the vma->vm_flags for those VMAs will remain intact.
> 
> Why cannot khugepaged simply skip over all VMAs which have userfault
> regions registered? This would sound like a less error prone approach to
> me.

khugepaged does skip over VMAs which have userfault. We could register the
regions with userfault before populating them to avoid collapses in the
transition period. But then we'll have to populate these regions with
UFFDIO_COPY which adds quite an overhead.

> -- 
> Michal Hocko
> SUSE Labs
> 

[toc] | [prev] | [next] | [standalone]

#1649705

Hello,

On Wed, May 24, 2017 at 05:27:36PM +0300, Mike Rapoport wrote:
> khugepaged does skip over VMAs which have userfault. We could register the
> regions with userfault before populating them to avoid collapses in the
> transition period. But then we'll have to populate these regions with
> UFFDIO_COPY which adds quite an overhead.

Yes, in fact with postcopy-only mode, there's no issue because of the
above.

The case where THP has to be temporarily disabled by CRIU is before
postcopy/userfaults engages, i.e. during the precopy with a
precopy+postcopy mode.

QEMU preferred mode is to do one pass of precopy before starting
postcopy/userfaults. During QEMU precopy phase VM_HUGEPAGE is set for
maximum performance and to back with THP in the destination as many
readonly (i.e. no source-redirtied) pages as possible. The dirty
logging in the source happens at 4k granularity by forcing the KVM
shadow MMU to map all pages at 4k granularity and by tracking the
dirty bit in software for the updates happening through the primary
MMU (linux pagetables dirty bit are ignored because soft dirty would
be too slow with O(N) complexity where N is linear with the size of
the VM, not with the number of re-dirtied pages in a precopy
pass). After that we track which 4k pages aren't uptodate on
destination and we zap them at 4k granularity with MADV_DONTNEED (we
badly need madvisev in fact to reduce the totally unnecessary flood of
4k wide MADV_DONTNEED there). So before calling the MADV_DONTNEED
flood, QEMU sets VM_NOHUGEPAGE, and after calling UFFDIO_REGISTER QEMU
sets back VM_HUGEPAGE (as the UFFDIO registration will keep khugepaged
at bay until postcopy completes). QEMU then finally calls
UFFDIO_UNREGISTER and khugepaged starts compacting everything that was
migrated through 4k wide userfaults.

CRIU doesn't attempt to populate destination with THP at all to be
simpler, but the problem is similar. It still has to call
VM_NOHUGEPAGE somehow during precopy (i.e. during the whole precopy
phase, precisely to avoid having to call MADV_DONTNEED to zap
4k not-uptodate fragments).

QEMU gets away with setting VM_NOHUGEPAGE and then back to VM_HUGEPAGE
without any issue because it's cooperative. CRIU as opposed has to
restore the same vm_flags that the vma had in the source to avoid
changing the behavior of the app after precopy+postcopy
completes. This is where the need of clearing the VM_*HUGEPAGE bits
from vm_flags comes into play.

Thanks,
Andrea

[toc] | [prev] | [next] | [standalone]

#1649510

On 05/24/2017 12:39 PM, Mike Rapoport wrote:
>> Hm so the prctl does:
>>
>>                 if (arg2)
>>                         me->mm->def_flags |= VM_NOHUGEPAGE;
>>                 else
>>                         me->mm->def_flags &= ~VM_NOHUGEPAGE;
>>
>> That's rather lazy implementation IMHO. Could we change it so the flag
>> is stored elsewhere in the mm, and the code that decides to (not) use
>> THP will check both the per-vma flag and the per-mm flag?
> 
> I afraid I don't understand how that can help.
> What we need is an ability to temporarily disable collapse of the pages in
> VMAs that do not have VM_*HUGEPAGE flags set and that after we re-enable
> THP, the vma->vm_flags for those VMAs will remain intact.

That's what I'm saying - instead of implementing the prctl flag via
mm->def_flags (which gets permanently propagated to newly created vma's
but e.g. doesn't affect already existing ones), it would be setting a
flag somewhere in mm, which khugepaged (and page faults) would check in
addition to the per-vma flags.


> --
> 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>
> 

[toc] | [prev] | [next] | [standalone]

#1649691

On 05/24/2017 04:28 PM, Pavel Emelyanov wrote:
> On 05/24/2017 02:31 PM, Vlastimil Babka wrote:
>> On 05/24/2017 12:39 PM, Mike Rapoport wrote:
>>>> Hm so the prctl does:
>>>>
>>>>                 if (arg2)
>>>>                         me->mm->def_flags |= VM_NOHUGEPAGE;
>>>>                 else
>>>>                         me->mm->def_flags &= ~VM_NOHUGEPAGE;
>>>>
>>>> That's rather lazy implementation IMHO. Could we change it so the flag
>>>> is stored elsewhere in the mm, and the code that decides to (not) use
>>>> THP will check both the per-vma flag and the per-mm flag?
>>>
>>> I afraid I don't understand how that can help.
>>> What we need is an ability to temporarily disable collapse of the pages in
>>> VMAs that do not have VM_*HUGEPAGE flags set and that after we re-enable
>>> THP, the vma->vm_flags for those VMAs will remain intact.
>>
>> That's what I'm saying - instead of implementing the prctl flag via
>> mm->def_flags (which gets permanently propagated to newly created vma's
>> but e.g. doesn't affect already existing ones), it would be setting a
>> flag somewhere in mm, which khugepaged (and page faults) would check in
>> addition to the per-vma flags.
> 
> I do not insist, but this would make existing paths (checking for flags) be 
> 2 times slower -- from now on these would need to check two bits (vma flags
> and mm flags) which are 100% in different cache lines.

I'd expect you already have mm struct cached during a page fault. And
THP-eligible page fault is just one per pmd, the overhead should be
practically zero.

> What Mike is proposing is the way to fine-tune the existing vma flags. This
> would keep current paths as fast (or slow ;) ) as they are now. All the
> complexity would go to rare cases when someone needs to turn thp off for a
> while and then turn it back on.

Yeah but it's extending user-space API for a corner case. We should do
that only when there's no other option.

> -- Pavel
> 
> --
> 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>
> 

[toc] | [prev] | [next] | [standalone]

#1649702

On Wed, May 24, 2017 at 04:54:38PM +0200, Vlastimil Babka wrote:
> On 05/24/2017 04:28 PM, Pavel Emelyanov wrote:
> > On 05/24/2017 02:31 PM, Vlastimil Babka wrote:
> >> On 05/24/2017 12:39 PM, Mike Rapoport wrote:
> >>>> Hm so the prctl does:
> >>>>
> >>>>                 if (arg2)
> >>>>                         me->mm->def_flags |= VM_NOHUGEPAGE;
> >>>>                 else
> >>>>                         me->mm->def_flags &= ~VM_NOHUGEPAGE;
> >>>>
> >>>> That's rather lazy implementation IMHO. Could we change it so the flag
> >>>> is stored elsewhere in the mm, and the code that decides to (not) use
> >>>> THP will check both the per-vma flag and the per-mm flag?
> >>>
> >>> I afraid I don't understand how that can help.
> >>> What we need is an ability to temporarily disable collapse of the pages in
> >>> VMAs that do not have VM_*HUGEPAGE flags set and that after we re-enable
> >>> THP, the vma->vm_flags for those VMAs will remain intact.
> >>
> >> That's what I'm saying - instead of implementing the prctl flag via
> >> mm->def_flags (which gets permanently propagated to newly created vma's
> >> but e.g. doesn't affect already existing ones), it would be setting a
> >> flag somewhere in mm, which khugepaged (and page faults) would check in
> >> addition to the per-vma flags.
> > 
> > I do not insist, but this would make existing paths (checking for flags) be 
> > 2 times slower -- from now on these would need to check two bits (vma flags
> > and mm flags) which are 100% in different cache lines.
> 
> I'd expect you already have mm struct cached during a page fault. And
> THP-eligible page fault is just one per pmd, the overhead should be
> practically zero.
> 
> > What Mike is proposing is the way to fine-tune the existing vma flags. This
> > would keep current paths as fast (or slow ;) ) as they are now. All the
> > complexity would go to rare cases when someone needs to turn thp off for a
> > while and then turn it back on.
> 
> Yeah but it's extending user-space API for a corner case. We should do
> that only when there's no other option.

With madvise() I'm suggesting we rather add "completeness" to the existing
API, IMHO. We do have API that sets VM_HUGEPAGE and clears VM_NOHUGEPAGE or
vise versa, but we do not have an API that can clear both flags...

And if we would use prctl(), we either change user visible behaviour or we
still need to extend the API and use, say, arg2 to distinguish between the
current behaviour and the new one.

--
Sincerely yours,
Mike. 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web