All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Fengwei Yin <yfw.kernel@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org
Cc: fengguang.wu@intel.com,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH] smaps should deal with huge zero page exactly same as normal zero page
Date: Thu, 09 Oct 2014 09:36:30 -0700	[thread overview]
Message-ID: <5436B98E.1070407@intel.com> (raw)
In-Reply-To: <CADUXgx7QTWBMxesxgCet5rjpGu-V-xK_-5f2rX9R+v-ggi902A@mail.gmail.com>

On 10/09/2014 02:19 AM, Fengwei Yin wrote:
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 80ca4fb..8550b27 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -476,7 +476,7 @@ static void smaps_pte_entry(pte_t ptent, unsigned long addr,
>  			mss->nonlinear += ptent_size;
>  	}
>  
> -	if (!page)
> +	if (!page || is_huge_zero_page(page))
>  		return;

This really seems like a bit of a hack.  A normal (small) zero page
won't make it to this point because of the vm_normal_page() check in
smaps_pte_entry() which hits the _PAGE_SPECIAL bit in the pte.

Is there a reason we can't set _PAGE_SPECIAL on the huge_zero_page ptes?
 If we did that, we wouldn't need a special case here.

If we can't do that for some reason, can we at least teach
vm_normal_page() about the huge_zero_page in some other way?

>  	if (PageAnon(page))
> @@ -516,7 +516,8 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  	if (pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
>  		smaps_pte_entry(*(pte_t *)pmd, addr, HPAGE_PMD_SIZE, walk);
>  		spin_unlock(ptl);
> -		mss->anonymous_thp += HPAGE_PMD_SIZE;
> +		if (!is_huge_zero_pmd(*pmd))
> +			mss->anonymous_thp += HPAGE_PMD_SIZE;
>  		return 0;
>  	}

How about we just move this hunk in to smaps_pte_entry()?  Something
along these lines:

...
        if (PageAnon(page)) {
                mss->anonymous += ptent_size;
+		if (PageTransHuge(page))
+			mss->anonymous_thp += ptent_size;
	}

If we do that, plus teaching vm_normal_page() about huge_zero_pages, it
will help keep the hacks and the extra code due to huge pages to a miniumum.

> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 63579cb..758f569 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -34,6 +34,10 @@ extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
>  			unsigned long addr, pgprot_t newprot,
>  			int prot_numa);
>  
> +extern bool is_huge_zero_page(struct page *page);
> +
> +extern bool is_huge_zero_pmd(pmd_t pmd);
> +
>  enum transparent_hugepage_flag {
>  	TRANSPARENT_HUGEPAGE_FLAG,
>  	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index d9a21d06..bedc3ae 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -173,12 +173,12 @@ static int start_khugepaged(void)
>  static atomic_t huge_zero_refcount;
>  static struct page *huge_zero_page __read_mostly;
>  
> -static inline bool is_huge_zero_page(struct page *page)
> +bool is_huge_zero_page(struct page *page)
>  {
>  	return ACCESS_ONCE(huge_zero_page) == page;
>  }
>  
> -static inline bool is_huge_zero_pmd(pmd_t pmd)
> +bool is_huge_zero_pmd(pmd_t pmd)
>  {
>  	return is_huge_zero_page(pmd_page(pmd));
>  }

^^^ And all these exports.

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

WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave.hansen@intel.com>
To: Fengwei Yin <yfw.kernel@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org
Cc: fengguang.wu@intel.com,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH] smaps should deal with huge zero page exactly same as normal zero page
Date: Thu, 09 Oct 2014 09:36:30 -0700	[thread overview]
Message-ID: <5436B98E.1070407@intel.com> (raw)
In-Reply-To: <CADUXgx7QTWBMxesxgCet5rjpGu-V-xK_-5f2rX9R+v-ggi902A@mail.gmail.com>

On 10/09/2014 02:19 AM, Fengwei Yin wrote:
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 80ca4fb..8550b27 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -476,7 +476,7 @@ static void smaps_pte_entry(pte_t ptent, unsigned long addr,
>  			mss->nonlinear += ptent_size;
>  	}
>  
> -	if (!page)
> +	if (!page || is_huge_zero_page(page))
>  		return;

This really seems like a bit of a hack.  A normal (small) zero page
won't make it to this point because of the vm_normal_page() check in
smaps_pte_entry() which hits the _PAGE_SPECIAL bit in the pte.

Is there a reason we can't set _PAGE_SPECIAL on the huge_zero_page ptes?
 If we did that, we wouldn't need a special case here.

If we can't do that for some reason, can we at least teach
vm_normal_page() about the huge_zero_page in some other way?

>  	if (PageAnon(page))
> @@ -516,7 +516,8 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  	if (pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
>  		smaps_pte_entry(*(pte_t *)pmd, addr, HPAGE_PMD_SIZE, walk);
>  		spin_unlock(ptl);
> -		mss->anonymous_thp += HPAGE_PMD_SIZE;
> +		if (!is_huge_zero_pmd(*pmd))
> +			mss->anonymous_thp += HPAGE_PMD_SIZE;
>  		return 0;
>  	}

How about we just move this hunk in to smaps_pte_entry()?  Something
along these lines:

...
        if (PageAnon(page)) {
                mss->anonymous += ptent_size;
+		if (PageTransHuge(page))
+			mss->anonymous_thp += ptent_size;
	}

If we do that, plus teaching vm_normal_page() about huge_zero_pages, it
will help keep the hacks and the extra code due to huge pages to a miniumum.

> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 63579cb..758f569 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -34,6 +34,10 @@ extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
>  			unsigned long addr, pgprot_t newprot,
>  			int prot_numa);
>  
> +extern bool is_huge_zero_page(struct page *page);
> +
> +extern bool is_huge_zero_pmd(pmd_t pmd);
> +
>  enum transparent_hugepage_flag {
>  	TRANSPARENT_HUGEPAGE_FLAG,
>  	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index d9a21d06..bedc3ae 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -173,12 +173,12 @@ static int start_khugepaged(void)
>  static atomic_t huge_zero_refcount;
>  static struct page *huge_zero_page __read_mostly;
>  
> -static inline bool is_huge_zero_page(struct page *page)
> +bool is_huge_zero_page(struct page *page)
>  {
>  	return ACCESS_ONCE(huge_zero_page) == page;
>  }
>  
> -static inline bool is_huge_zero_pmd(pmd_t pmd)
> +bool is_huge_zero_pmd(pmd_t pmd)
>  {
>  	return is_huge_zero_page(pmd_page(pmd));
>  }

^^^ And all these exports.

  reply	other threads:[~2014-10-09 16:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-09  9:19 [PATCH] smaps should deal with huge zero page exactly same as normal zero page Fengwei Yin
2014-10-09 16:36 ` Dave Hansen [this message]
2014-10-09 16:36   ` Dave Hansen
2014-10-10  9:33   ` Fengwei Yin
2014-10-10  9:33     ` Fengwei Yin
2014-10-10 13:21   ` Fengwei Yin
2014-10-10 13:21     ` Fengwei Yin
2014-10-10 14:35     ` Dave Hansen
2014-10-10 14:35       ` Dave Hansen
2014-10-11 10:11       ` Fengwei Yin
2014-10-11 10:11         ` Fengwei Yin
2014-10-17 10:46       ` Fengwei Yin
2014-10-17 10:46         ` Fengwei Yin
2014-10-14 11:57     ` Kirill A. Shutemov
2014-10-14 11:57       ` Kirill A. Shutemov
2014-10-15 10:30       ` Fengwei Yin
2014-10-15 10:30         ` Fengwei Yin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5436B98E.1070407@intel.com \
    --to=dave.hansen@intel.com \
    --cc=fengguang.wu@intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=yfw.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.