* [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled @ 2012-04-28 6:33 Alex Shi 2012-04-30 23:05 ` Andrew Morton 0 siblings, 1 reply; 8+ messages in thread From: Alex Shi @ 2012-04-28 6:33 UTC (permalink / raw) To: aarcange, riel, mgorman; +Cc: akpm, linux-kernel, alex.shi When the transparent_hugepage_enabled() used out of mm/, is_vma_temporary_stack() need be referenced. Otherwise, it has compile error. Signed-off-by: Alex Shi <alex.shi@intel.com> --- include/linux/huge_mm.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index c8af7a2..10254ac 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -59,6 +59,7 @@ extern pmd_t *page_check_address_pmd(struct page *page, #define HPAGE_PMD_MASK HPAGE_MASK #define HPAGE_PMD_SIZE HPAGE_SIZE +extern bool is_vma_temporary_stack(struct vm_area_struct *vma); #define transparent_hugepage_enabled(__vma) \ ((transparent_hugepage_flags & \ (1<<TRANSPARENT_HUGEPAGE_FLAG) || \ -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled 2012-04-28 6:33 [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled Alex Shi @ 2012-04-30 23:05 ` Andrew Morton 2012-05-02 3:17 ` Alex Shi 0 siblings, 1 reply; 8+ messages in thread From: Andrew Morton @ 2012-04-30 23:05 UTC (permalink / raw) To: Alex Shi; +Cc: aarcange, riel, mgorman, linux-kernel On Sat, 28 Apr 2012 14:33:15 +0800 Alex Shi <alex.shi@intel.com> wrote: > When the transparent_hugepage_enabled() used out of mm/, > is_vma_temporary_stack() need be referenced. Otherwise, it has compile > error. This is a poor changelog - it doesn't tell us how this compilation error comes about. Is there some known build error in the mainline kernel, or did you discover this when altering the kernel, or what? One of the several reasons for this information is to permit others to work out which kernel version(s) should be fixed. > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h > index c8af7a2..10254ac 100644 > --- a/include/linux/huge_mm.h > +++ b/include/linux/huge_mm.h > @@ -59,6 +59,7 @@ extern pmd_t *page_check_address_pmd(struct page *page, > #define HPAGE_PMD_MASK HPAGE_MASK > #define HPAGE_PMD_SIZE HPAGE_SIZE > > +extern bool is_vma_temporary_stack(struct vm_area_struct *vma); > #define transparent_hugepage_enabled(__vma) \ > ((transparent_hugepage_flags & \ > (1<<TRANSPARENT_HUGEPAGE_FLAG) || \ is_vma_temporary_stack() is already declared in rmap.h. We should not declare it in two places. include/linux/huge_mm.h doesn't include any headers at all. It is one of those files which require its user to set up the preconditions. So, lacking any additional infomation I'd say that your mystery build breakage was caused by a failure to include rmap.h. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled 2012-04-30 23:05 ` Andrew Morton @ 2012-05-02 3:17 ` Alex Shi 2012-05-02 3:31 ` Alex Shi 2012-05-02 17:55 ` Andrea Arcangeli 0 siblings, 2 replies; 8+ messages in thread From: Alex Shi @ 2012-05-02 3:17 UTC (permalink / raw) To: Andrew Morton; +Cc: aarcange, riel, mgorman, linux-kernel On 05/01/2012 07:05 AM, Andrew Morton wrote: > On Sat, 28 Apr 2012 14:33:15 +0800 > Alex Shi <alex.shi@intel.com> wrote: > >> When the transparent_hugepage_enabled() used out of mm/, >> is_vma_temporary_stack() need be referenced. Otherwise, it has compile >> error. > > This is a poor changelog - it doesn't tell us how this compilation > error comes about. Is there some known build error in the mainline > kernel, or did you discover this when altering the kernel, or what? > > One of the several reasons for this information is to permit others to > work out which kernel version(s) should be fixed. > I am sorry for the unclear log! When I try to transparent_hugepage_enabled() in arch/x86/mm/tlb.c with huge_mm.h include: +#include <linux/huge_mm.h>. make give me the following error: ... CC arch/x86/mm/srat.o arch/x86/mm/tlb.c: In function ‘flush_tlb_range’: arch/x86/mm/tlb.c:324:4: error: implicit declaration of function ‘is_vma_temporary_stack’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors ... Since it is not convenitant for user to include 2 head files just for one target function, I send this patch. > is_vma_temporary_stack() is already declared in rmap.h. We should not > declare it in two places. Oh, yes. Since is_vma_temporay_stack is just used in rmap.c and huge_memory.c . is it better to move it to huge_mm.h? --- >From 7c208e10b3f4fc2f4f9c41068ea4d65a1119970e Mon Sep 17 00:00:00 2001 From: Alex Shi <alex.shi@intel.com> Date: Wed, 2 May 2012 11:04:04 +0800 Subject: [PATCH] mm/THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the transparent_hugepage_enabled() using out of mm/, like in a altering arch/x86/xx/tlb.c: + if (!cpu_has_invlpg || vma->vm_flags & VM_HUGETLB + || transparent_hugepage_enabled(vma)) { + flush_tlb_mm(vma->vm_mm); is_vma_temporary_stack() isn't referenced in huge_mm.h, so it has compile errors: arch/x86/mm/tlb.c: In function ‘flush_tlb_range’: arch/x86/mm/tlb.c:324:4: error: implicit declaration of function ‘is_vma_temporary_stack’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors Since is_vma_temporay_stack() is just used in rmap.c and huge_memory.c. It is better to move it to huge_mm.h from rmap.h to avoid such error. Signed-off-by: Alex Shi <alex.shi@intel.com> --- include/linux/huge_mm.h | 2 ++ include/linux/rmap.h | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index 1b92129..acf3ab1 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -56,6 +56,8 @@ extern pmd_t *page_check_address_pmd(struct page *page, #define HPAGE_PMD_MASK HPAGE_MASK #define HPAGE_PMD_SIZE HPAGE_SIZE +extern bool is_vma_temporary_stack(struct vm_area_struct *vma); + #define transparent_hugepage_enabled(__vma) \ ((transparent_hugepage_flags & \ (1<<TRANSPARENT_HUGEPAGE_FLAG) || \ diff --git a/include/linux/rmap.h b/include/linux/rmap.h index 1cdd62a..267fb6b 100644 --- a/include/linux/rmap.h +++ b/include/linux/rmap.h @@ -174,8 +174,6 @@ enum ttu_flags { }; #define TTU_ACTION(x) ((x) & TTU_ACTION_MASK) -bool is_vma_temporary_stack(struct vm_area_struct *vma); - int try_to_unmap(struct page *, enum ttu_flags flags); int try_to_unmap_one(struct page *, struct vm_area_struct *, unsigned long address, enum ttu_flags flags); -- 1.7.5.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled 2012-05-02 3:17 ` Alex Shi @ 2012-05-02 3:31 ` Alex Shi 2012-05-02 17:55 ` Andrea Arcangeli 1 sibling, 0 replies; 8+ messages in thread From: Alex Shi @ 2012-05-02 3:31 UTC (permalink / raw) To: Andrew Morton; +Cc: aarcange, riel, mgorman, linux-kernel > > Oh, yes. Since is_vma_temporay_stack is just used in rmap.c and huge_memory.c > . is it better to move it to huge_mm.h? If you still concern someone just want to use is_vma_temporay_stack function. I have no better idea. and please drop this patch if so. :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled 2012-05-02 3:17 ` Alex Shi 2012-05-02 3:31 ` Alex Shi @ 2012-05-02 17:55 ` Andrea Arcangeli 2012-05-03 0:56 ` Alex Shi 1 sibling, 1 reply; 8+ messages in thread From: Andrea Arcangeli @ 2012-05-02 17:55 UTC (permalink / raw) To: Alex Shi; +Cc: Andrew Morton, riel, mgorman, linux-kernel On Wed, May 02, 2012 at 11:17:23AM +0800, Alex Shi wrote: > On 05/01/2012 07:05 AM, Andrew Morton wrote: > > > On Sat, 28 Apr 2012 14:33:15 +0800 > > Alex Shi <alex.shi@intel.com> wrote: > > > >> When the transparent_hugepage_enabled() used out of mm/, > >> is_vma_temporary_stack() need be referenced. Otherwise, it has compile > >> error. > > > > This is a poor changelog - it doesn't tell us how this compilation > > error comes about. Is there some known build error in the mainline > > kernel, or did you discover this when altering the kernel, or what? > > > > > > One of the several reasons for this information is to permit others to > > work out which kernel version(s) should be fixed. > > I wanted to ask the same question. > I am sorry for the unclear log! > When I try to transparent_hugepage_enabled() in arch/x86/mm/tlb.c with > huge_mm.h include: +#include <linux/huge_mm.h>. make give me the following > error: I already guessed it was for development code out of tree code but I wasn't sure, thanks for the clarification. > ... > CC arch/x86/mm/srat.o > arch/x86/mm/tlb.c: In function ‘flush_tlb_range’: > arch/x86/mm/tlb.c:324:4: error: implicit declaration of function ‘is_vma_temporary_stack’ [-Werror=implicit-function-declaration] > cc1: some warnings being treated as errors > ... > Since it is not convenitant for user to include 2 head files just for one > target function, I send this patch. > > > is_vma_temporary_stack() is already declared in rmap.h. We should not > > declare it in two places. My preference would still be to remove the is_vma_temporary_stack and use two vmas during mremap of execve, that would remove the "vma" parameter from transparent_hugepage_enabled() but others prefers to skip a vma allocation in execve and stick to is_vma_temporary_stack, which is fair enough argument. > Oh, yes. Since is_vma_temporay_stack is just used in rmap.c and huge_memory.c > . is it better to move it to huge_mm.h? I guess it was cleaner on rmap.h as this is not related to THP, but clearly it works better in huge_mm.h (and rmap.h->mm.h->huge_mm.h is included automatically) so the patch looks fine to me. Thanks, Andrea ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled 2012-05-02 17:55 ` Andrea Arcangeli @ 2012-05-03 0:56 ` Alex Shi 2012-05-03 11:25 ` Andrea Arcangeli 0 siblings, 1 reply; 8+ messages in thread From: Alex Shi @ 2012-05-03 0:56 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: Andrew Morton, riel, mgorman, linux-kernel > My preference would still be to remove the is_vma_temporary_stack and > use two vmas during mremap of execve, that would remove the "vma" > parameter from transparent_hugepage_enabled() but others prefers to > skip a vma allocation in execve and stick to is_vma_temporary_stack, > which is fair enough argument. Actually, current transparent_hugepage_enabled just means the vma is in THP enable ENV, the vma is just possibly has some large page, no grantee really has. But in lots situations, user wants to know if a vma or a part of memory really include a large page. not the possibility. So, it will be great to see a real large page checking function appearing. > >> Oh, yes. Since is_vma_temporay_stack is just used in rmap.c and huge_memory.c >> . is it better to move it to huge_mm.h? > > I guess it was cleaner on rmap.h as this is not related to THP, but > clearly it works better in huge_mm.h (and rmap.h->mm.h->huge_mm.h is > included automatically) so the patch looks fine to me. Thanks for point out. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled 2012-05-03 0:56 ` Alex Shi @ 2012-05-03 11:25 ` Andrea Arcangeli 2012-05-04 7:26 ` Alex Shi 0 siblings, 1 reply; 8+ messages in thread From: Andrea Arcangeli @ 2012-05-03 11:25 UTC (permalink / raw) To: Alex Shi; +Cc: Andrew Morton, riel, mgorman, linux-kernel Hi, On Thu, May 03, 2012 at 08:56:57AM +0800, Alex Shi wrote: > > > My preference would still be to remove the is_vma_temporary_stack and > > use two vmas during mremap of execve, that would remove the "vma" > > parameter from transparent_hugepage_enabled() but others prefers to > > skip a vma allocation in execve and stick to is_vma_temporary_stack, > > which is fair enough argument. > > > Actually, current transparent_hugepage_enabled just means the vma is in > THP enable ENV, the vma is just possibly has some large page, no grantee > really has. But in lots situations, user wants to know if a vma or a > part of memory really include a large page. not the possibility. > > So, it will be great to see a real large page checking function appearing. Well, to know if a VMA (or a memory range) really includes a THP, it'd require to hold the page_table_lock and a loop on all pmds in the range, but by the time you relase the lock things may have already changed as split_huge_page can run at any time, madvise(MADV_DONTNEED) too if you only hold the mmap_sem in read mode and the THP page fault. In fact while holding the mmap_sem in read mode (the usual read lock you need to take to lookup and stabilize the vma) a THP can be freed and reallocated under it, and that's what pmd_trans_unstable is about. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled 2012-05-03 11:25 ` Andrea Arcangeli @ 2012-05-04 7:26 ` Alex Shi 0 siblings, 0 replies; 8+ messages in thread From: Alex Shi @ 2012-05-04 7:26 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: Andrew Morton, riel, mgorman, linux-kernel On 05/03/2012 07:25 PM, Andrea Arcangeli wrote: > Hi, > > On Thu, May 03, 2012 at 08:56:57AM +0800, Alex Shi wrote: >> >>> My preference would still be to remove the is_vma_temporary_stack and >>> use two vmas during mremap of execve, that would remove the "vma" >>> parameter from transparent_hugepage_enabled() but others prefers to >>> skip a vma allocation in execve and stick to is_vma_temporary_stack, >>> which is fair enough argument. >> >> >> Actually, current transparent_hugepage_enabled just means the vma is in >> THP enable ENV, the vma is just possibly has some large page, no grantee >> really has. But in lots situations, user wants to know if a vma or a >> part of memory really include a large page. not the possibility. >> >> So, it will be great to see a real large page checking function appearing. > > Well, to know if a VMA (or a memory range) really includes a THP, it'd > require to hold the page_table_lock and a loop on all pmds in the > range, but by the time you relase the lock things may have already > changed as split_huge_page can run at any time, madvise(MADV_DONTNEED) > too if you only hold the mmap_sem in read mode and the THP page > fault. In fact while holding the mmap_sem in read mode (the usual read > lock you need to take to lookup and stabilize the vma) a THP can be > freed and reallocated under it, and that's what pmd_trans_unstable is > about. Appreciate for explanation! ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-05-04 7:27 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-04-28 6:33 [PATCH] THP: need is_vma_temporary_stack() when reference transparent_hugepage_enabled Alex Shi 2012-04-30 23:05 ` Andrew Morton 2012-05-02 3:17 ` Alex Shi 2012-05-02 3:31 ` Alex Shi 2012-05-02 17:55 ` Andrea Arcangeli 2012-05-03 0:56 ` Alex Shi 2012-05-03 11:25 ` Andrea Arcangeli 2012-05-04 7:26 ` Alex Shi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).