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

Groups > linux.kernel > #1516656 > unrolled thread

Re: [PATCHv4] shmem: avoid huge pages for small files

Started byHugh Dickins <hughd@google.com>
First post2016-11-08 00:20 +0100
Last post2016-11-10 19:10 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCHv4] shmem: avoid huge pages for small files Hugh Dickins <hughd@google.com> - 2016-11-08 00:20 +0100
    Re: [PATCHv4] shmem: avoid huge pages for small files "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-11-10 17:30 +0100
      Re: [PATCH] shmem: avoid huge pages for small files kbuild test robot <lkp@intel.com> - 2016-11-10 18:50 +0100
        Re: [PATCH] shmem: avoid huge pages for small files "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-11-10 19:10 +0100

#1516656 — Re: [PATCHv4] shmem: avoid huge pages for small files

FromHugh Dickins <hughd@google.com>
Date2016-11-08 00:20 +0100
SubjectRe: [PATCHv4] shmem: avoid huge pages for small files
Message-ID<sB1mN-1rD-11@gated-at.bofh.it>
On Sat, 22 Oct 2016, Kirill A. Shutemov wrote:
> 
> Huge pages are detrimental for small file: they causes noticible
> overhead on both allocation performance and memory footprint.
> 
> This patch aimed to address this issue by avoiding huge pages until file
> grown to size of huge page. This would cover most of the cases where huge
> pages causes regressions in performance.
> 
> Couple notes:
> 
>   - if shmem_enabled is set to 'force', the limit is ignored. We still
>     want to generate as many pages as possible for functional testing.
> 
>   - the limit doesn't affect khugepaged behaviour: it still can collapse
>     pages based on its settings;
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Sorry, but NAK.  I was expecting a patch to tune within_size behaviour.

> ---
>  Documentation/vm/transhuge.txt | 3 +++
>  mm/shmem.c                     | 5 +++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/Documentation/vm/transhuge.txt b/Documentation/vm/transhuge.txt
> index 2ec6adb5a4ce..d1889c7c8c46 100644
> --- a/Documentation/vm/transhuge.txt
> +++ b/Documentation/vm/transhuge.txt
> @@ -238,6 +238,9 @@ values:
>    - "force":
>      Force the huge option on for all - very useful for testing;
>  
> +To avoid overhead for small files, we don't allocate huge pages for a file
> +until it grows to size of huge pages.
> +
>  == Need of application restart ==
>  
>  The transparent_hugepage/enabled values and tmpfs mount option only affect
> diff --git a/mm/shmem.c b/mm/shmem.c
> index ad7813d73ea7..49618d2d6330 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1692,6 +1692,11 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
>  				goto alloc_huge;
>  			/* TODO: implement fadvise() hints */
>  			goto alloc_nohuge;
> +		case SHMEM_HUGE_ALWAYS:
> +			i_size = i_size_read(inode);
> +			if (index < HPAGE_PMD_NR && i_size < HPAGE_PMD_SIZE)
> +				goto alloc_nohuge;
> +			break;
>  		}
>  
>  alloc_huge:

So (eliding the SHMEM_HUGE_ADVISE case in between) you now have:

		case SHMEM_HUGE_WITHIN_SIZE:
			off = round_up(index, HPAGE_PMD_NR);
			i_size = round_up(i_size_read(inode), PAGE_SIZE);
			if (i_size >= HPAGE_PMD_SIZE &&
					i_size >> PAGE_SHIFT >= off)
				goto alloc_huge;
			goto alloc_nohuge;
		case SHMEM_HUGE_ALWAYS:
			i_size = i_size_read(inode);
			if (index < HPAGE_PMD_NR && i_size < HPAGE_PMD_SIZE)
				goto alloc_nohuge;
			goto alloc_huge;

I'll concede that those two conditions are not the same; but again you're
messing with huge=always to make it, not always, but conditional on size.

Please, keep huge=always as is: if I copy a 4MiB file into a huge tmpfs,
I got ShmemHugePages 4096 kB before, which is what I wanted.  Whereas
with this change I get only 2048 kB, just like with huge=within_size.

Treating the first extent differently is a hack, and does not respect
that this is a filesystem, on which size is likely to increase.

By all means refine the condition for huge=within_size, and by all means
warn in transhuge.txt that huge=always may tend to waste valuable huge
pages if the filesystem is used for small files without good reason
(but maybe the implementation needs to reclaim those more effectively).

Hugh

[toc] | [next] | [standalone]

#1519115

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2016-11-10 17:30 +0100
Message-ID<sC0oF-as-11@gated-at.bofh.it>
In reply to#1516656
On Mon, Nov 07, 2016 at 03:17:11PM -0800, Hugh Dickins wrote:
> On Sat, 22 Oct 2016, Kirill A. Shutemov wrote:
> > 
> > Huge pages are detrimental for small file: they causes noticible
> > overhead on both allocation performance and memory footprint.
> > 
> > This patch aimed to address this issue by avoiding huge pages until file
> > grown to size of huge page. This would cover most of the cases where huge
> > pages causes regressions in performance.
> > 
> > Couple notes:
> > 
> >   - if shmem_enabled is set to 'force', the limit is ignored. We still
> >     want to generate as many pages as possible for functional testing.
> > 
> >   - the limit doesn't affect khugepaged behaviour: it still can collapse
> >     pages based on its settings;
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> 
> Sorry, but NAK.  I was expecting a patch to tune within_size behaviour.
> 
> > ---
> >  Documentation/vm/transhuge.txt | 3 +++
> >  mm/shmem.c                     | 5 +++++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/vm/transhuge.txt b/Documentation/vm/transhuge.txt
> > index 2ec6adb5a4ce..d1889c7c8c46 100644
> > --- a/Documentation/vm/transhuge.txt
> > +++ b/Documentation/vm/transhuge.txt
> > @@ -238,6 +238,9 @@ values:
> >    - "force":
> >      Force the huge option on for all - very useful for testing;
> >  
> > +To avoid overhead for small files, we don't allocate huge pages for a file
> > +until it grows to size of huge pages.
> > +
> >  == Need of application restart ==
> >  
> >  The transparent_hugepage/enabled values and tmpfs mount option only affect
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index ad7813d73ea7..49618d2d6330 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -1692,6 +1692,11 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
> >  				goto alloc_huge;
> >  			/* TODO: implement fadvise() hints */
> >  			goto alloc_nohuge;
> > +		case SHMEM_HUGE_ALWAYS:
> > +			i_size = i_size_read(inode);
> > +			if (index < HPAGE_PMD_NR && i_size < HPAGE_PMD_SIZE)
> > +				goto alloc_nohuge;
> > +			break;
> >  		}
> >  
> >  alloc_huge:
> 
> So (eliding the SHMEM_HUGE_ADVISE case in between) you now have:
> 
> 		case SHMEM_HUGE_WITHIN_SIZE:
> 			off = round_up(index, HPAGE_PMD_NR);
> 			i_size = round_up(i_size_read(inode), PAGE_SIZE);
> 			if (i_size >= HPAGE_PMD_SIZE &&
> 					i_size >> PAGE_SHIFT >= off)
> 				goto alloc_huge;
> 			goto alloc_nohuge;
> 		case SHMEM_HUGE_ALWAYS:
> 			i_size = i_size_read(inode);
> 			if (index < HPAGE_PMD_NR && i_size < HPAGE_PMD_SIZE)
> 				goto alloc_nohuge;
> 			goto alloc_huge;
> 
> I'll concede that those two conditions are not the same; but again you're
> messing with huge=always to make it, not always, but conditional on size.
> 
> Please, keep huge=always as is: if I copy a 4MiB file into a huge tmpfs,
> I got ShmemHugePages 4096 kB before, which is what I wanted.  Whereas
> with this change I get only 2048 kB, just like with huge=within_size.

I don't think it's a problem really. We don't have guarantees anyway.
And we can collapse the page later.

But okay.

> Treating the first extent differently is a hack, and does not respect
> that this is a filesystem, on which size is likely to increase.
> 
> By all means refine the condition for huge=within_size, and by all means
> warn in transhuge.txt that huge=always may tend to waste valuable huge
> pages if the filesystem is used for small files without good reason

Would it be okay, if I just replace huge=within_size logic with what I
proposed here for huge=always?

That's not what I intended initially for this option, but...

> (but maybe the implementation needs to reclaim those more effectively).

It's more about cost of allocation than memory pressure.

-----8<-----

From 287ab05c09bfd49c7356ca74b6fea36d8131edaf Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Mon, 17 Oct 2016 14:44:47 +0300
Subject: [PATCH] shmem: avoid huge pages for small files

Huge pages are detrimental for small file: they causes noticible
overhead on both allocation performance and memory footprint.

This patch aimed to address this issue by avoiding huge pages until
file grown to size of huge page if the filesystem mounted with
huge=within_size option.

This would cover most of the cases where huge pages causes regressions
in performance.

The limit doesn't affect khugepaged behaviour: it still can collapse
pages based on its settings.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 Documentation/vm/transhuge.txt | 7 ++++++-
 mm/shmem.c                     | 6 ++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/vm/transhuge.txt b/Documentation/vm/transhuge.txt
index 2ec6adb5a4ce..14c911c56f4a 100644
--- a/Documentation/vm/transhuge.txt
+++ b/Documentation/vm/transhuge.txt
@@ -208,11 +208,16 @@ You can control hugepage allocation policy in tmpfs with mount option
   - "always":
     Attempt to allocate huge pages every time we need a new page;
 
+    This option can lead to significant overhead if filesystem is used to
+    store small files.
+
   - "never":
     Do not allocate huge pages;
 
   - "within_size":
-    Only allocate huge page if it will be fully within i_size.
+    Only allocate huge page if size of the file more than size of huge
+    page. This helps to avoid overhead for small files.
+
     Also respect fadvise()/madvise() hints;
 
   - "advise:
diff --git a/mm/shmem.c b/mm/shmem.c
index ad7813d73ea7..3589d36c7c63 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1681,10 +1681,8 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
 		case SHMEM_HUGE_NEVER:
 			goto alloc_nohuge;
 		case SHMEM_HUGE_WITHIN_SIZE:
-			off = round_up(index, HPAGE_PMD_NR);
-			i_size = round_up(i_size_read(inode), PAGE_SIZE);
-			if (i_size >= HPAGE_PMD_SIZE &&
-					i_size >> PAGE_SHIFT >= off)
+			i_size = i_size_read(inode);
+			if (index >= HPAGE_PMD_NR || i_size >= HPAGE_PMD_SIZE)
 				goto alloc_huge;
 			/* fallthrough */
 		case SHMEM_HUGE_ADVISE:
-- 
 Kirill A. Shutemov

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

#1519230 — Re: [PATCH] shmem: avoid huge pages for small files

Fromkbuild test robot <lkp@intel.com>
Date2016-11-10 18:50 +0100
SubjectRe: [PATCH] shmem: avoid huge pages for small files
Message-ID<sC1E5-QH-1@gated-at.bofh.it>
In reply to#1519115

[Multipart message — attachments visible in raw view] — view raw

Hi Kirill,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.9-rc4 next-20161110]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kirill-A-Shutemov/shmem-avoid-huge-pages-for-small-files/20161111-005428
config: i386-randconfig-s0-201645 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   mm/shmem.c: In function 'shmem_getpage_gfp':
>> mm/shmem.c:1680:12: warning: unused variable 'off' [-Wunused-variable]
       pgoff_t off;
               ^~~

vim +/off +1680 mm/shmem.c

66d2f4d2 Hugh Dickins       2014-07-02  1664  			mark_page_accessed(page);
66d2f4d2 Hugh Dickins       2014-07-02  1665  
54af6042 Hugh Dickins       2011-08-03  1666  		delete_from_swap_cache(page);
27ab7006 Hugh Dickins       2011-07-25  1667  		set_page_dirty(page);
27ab7006 Hugh Dickins       2011-07-25  1668  		swap_free(swap);
27ab7006 Hugh Dickins       2011-07-25  1669  
54af6042 Hugh Dickins       2011-08-03  1670  	} else {
800d8c63 Kirill A. Shutemov 2016-07-26  1671  		/* shmem_symlink() */
800d8c63 Kirill A. Shutemov 2016-07-26  1672  		if (mapping->a_ops != &shmem_aops)
800d8c63 Kirill A. Shutemov 2016-07-26  1673  			goto alloc_nohuge;
657e3038 Kirill A. Shutemov 2016-07-26  1674  		if (shmem_huge == SHMEM_HUGE_DENY || sgp_huge == SGP_NOHUGE)
800d8c63 Kirill A. Shutemov 2016-07-26  1675  			goto alloc_nohuge;
800d8c63 Kirill A. Shutemov 2016-07-26  1676  		if (shmem_huge == SHMEM_HUGE_FORCE)
800d8c63 Kirill A. Shutemov 2016-07-26  1677  			goto alloc_huge;
800d8c63 Kirill A. Shutemov 2016-07-26  1678  		switch (sbinfo->huge) {
800d8c63 Kirill A. Shutemov 2016-07-26  1679  			loff_t i_size;
800d8c63 Kirill A. Shutemov 2016-07-26 @1680  			pgoff_t off;
800d8c63 Kirill A. Shutemov 2016-07-26  1681  		case SHMEM_HUGE_NEVER:
800d8c63 Kirill A. Shutemov 2016-07-26  1682  			goto alloc_nohuge;
800d8c63 Kirill A. Shutemov 2016-07-26  1683  		case SHMEM_HUGE_WITHIN_SIZE:
bb89f249 Kirill A. Shutemov 2016-11-10  1684  			i_size = i_size_read(inode);
bb89f249 Kirill A. Shutemov 2016-11-10  1685  			if (index >= HPAGE_PMD_NR || i_size >= HPAGE_PMD_SIZE)
800d8c63 Kirill A. Shutemov 2016-07-26  1686  				goto alloc_huge;
800d8c63 Kirill A. Shutemov 2016-07-26  1687  			/* fallthrough */
800d8c63 Kirill A. Shutemov 2016-07-26  1688  		case SHMEM_HUGE_ADVISE:

:::::: The code at line 1680 was first introduced by commit
:::::: 800d8c63b2e989c2e349632d1648119bf5862f01 shmem: add huge pages support

:::::: TO: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

#1519266 — Re: [PATCH] shmem: avoid huge pages for small files

From"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date2016-11-10 19:10 +0100
SubjectRe: [PATCH] shmem: avoid huge pages for small files
Message-ID<sC1Xx-1gl-53@gated-at.bofh.it>
In reply to#1519230
On Fri, Nov 11, 2016 at 01:42:47AM +0800, kbuild test robot wrote:
> Hi Kirill,
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.9-rc4 next-20161110]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Kirill-A-Shutemov/shmem-avoid-huge-pages-for-small-files/20161111-005428
> config: i386-randconfig-s0-201645 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
>    mm/shmem.c: In function 'shmem_getpage_gfp':
> >> mm/shmem.c:1680:12: warning: unused variable 'off' [-Wunused-variable]
>        pgoff_t off;


From f0a582888ac6dcb56c6134611c83edfb091bbcb6 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Mon, 17 Oct 2016 14:44:47 +0300
Subject: [PATCH] shmem: avoid huge pages for small files

Huge pages are detrimental for small file: they causes noticible
overhead on both allocation performance and memory footprint.

This patch aimed to address this issue by avoiding huge pages until
file grown to size of huge page if the filesystem mounted with
huge=within_size option.

This would cover most of the cases where huge pages causes regressions
in performance.

The limit doesn't affect khugepaged behaviour: it still can collapse
pages based on its settings.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 Documentation/vm/transhuge.txt | 7 ++++++-
 mm/shmem.c                     | 7 ++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/vm/transhuge.txt b/Documentation/vm/transhuge.txt
index 2ec6adb5a4ce..14c911c56f4a 100644
--- a/Documentation/vm/transhuge.txt
+++ b/Documentation/vm/transhuge.txt
@@ -208,11 +208,16 @@ You can control hugepage allocation policy in tmpfs with mount option
   - "always":
     Attempt to allocate huge pages every time we need a new page;
 
+    This option can lead to significant overhead if filesystem is used to
+    store small files.
+
   - "never":
     Do not allocate huge pages;
 
   - "within_size":
-    Only allocate huge page if it will be fully within i_size.
+    Only allocate huge page if size of the file more than size of huge
+    page. This helps to avoid overhead for small files.
+
     Also respect fadvise()/madvise() hints;
 
   - "advise:
diff --git a/mm/shmem.c b/mm/shmem.c
index ad7813d73ea7..3e2c0912c587 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1677,14 +1677,11 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
 			goto alloc_huge;
 		switch (sbinfo->huge) {
 			loff_t i_size;
-			pgoff_t off;
 		case SHMEM_HUGE_NEVER:
 			goto alloc_nohuge;
 		case SHMEM_HUGE_WITHIN_SIZE:
-			off = round_up(index, HPAGE_PMD_NR);
-			i_size = round_up(i_size_read(inode), PAGE_SIZE);
-			if (i_size >= HPAGE_PMD_SIZE &&
-					i_size >> PAGE_SHIFT >= off)
+			i_size = i_size_read(inode);
+			if (index >= HPAGE_PMD_NR || i_size >= HPAGE_PMD_SIZE)
 				goto alloc_huge;
 			/* fallthrough */
 		case SHMEM_HUGE_ADVISE:
-- 
2.9.3

-- 
 Kirill A. Shutemov

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web