[PATCH] shmem: add shmem_size option for full filesystem

[PATCH] shmem: add shmem_size option for full filesystem

[Gary B. Genett]

Adds a kernel configuration option to specify the size of the shmem
filesystem.  It is currently hard-coded to 50% of memory.  Users should
have the option to set this value as they see fit.

A specific case where this would be necessary is if the initramfs were
larger than half of the memory, such as a 2.5GB "live" filesystem on
a system with 4GB of memory.  Without this option, this causes a kernel
panic.  With this option, the user may specify the number of pages of
memory they need for their root filesystem.

This patch creates the SHMEM_SIZE configuration option, which is
specified as the number of memory pages to use for the shmem
filesystem.  The default remains unchanged.  This patch has no impact
unless the values are changed.

The option is marked as expert, and the help text is clear that it
should only be set if the user knows what they are doing.

Signed-off-by: Gary B. Genett <me@garybgenett.net>
---
 init/Kconfig | 11 +++++++++++
 mm/shmem.c   | 20 ++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index b4daad2bac23..fcff0f1ea3e8 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1537,6 +1537,17 @@ config SHMEM
 	  option replaces shmem and tmpfs with the much simpler ramfs code,
 	  which may be appropriate on small systems without swap.
 
+config SHMEM_SIZE
+	int "Set the full shmem filesystem size" if EXPERT
+	default 0
+	depends on SHMEM
+	help
+	   Size of full shmem filesystem.  Defaults to 50% of memory,
+	   dynamically.  Must be defined as the number of 4096 byte memory
+	   pages, which will be static regardless of the amount of actual
+	   memory.  Unless you have a special use case, this value should not
+	   be changed.
+
 config AIO
 	bool "Enable AIO support" if EXPERT
 	default y
diff --git a/mm/shmem.c b/mm/shmem.c
index cd570cc79c76..d5baaf7ac9e8 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -95,6 +95,13 @@ static struct vfsmount *shm_mnt;
 /* Symlink up to this size is kmalloc'ed instead of using a swappable page */
 #define SHORT_SYMLINK_LEN 128
 
+/* Default size of shmem filesystem (defaults to 50% of memory) */
+#if defined(CONFIG_SHMEM_SIZE) && (CONFIG_SHMEM_SIZE > 0)
+#define SHMEM_SIZE_DEFAULT ((unsigned long)CONFIG_SHMEM_SIZE)
+#else
+#define SHMEM_SIZE_DEFAULT (totalram_pages() / 2)
+#endif
+
 /*
  * shmem_fallocate communicates with shmem_fault or shmem_writepage via
  * inode->i_private (with i_mutex making sure that it has only one user at
@@ -123,16 +130,22 @@ struct shmem_options {
 };
 
 #ifdef CONFIG_TMPFS
+static void shmem_size_info(void)
+{
+	pr_info("shmem: setting default size: %lu pages, %lu bytes\n",
+		SHMEM_SIZE_DEFAULT, (SHMEM_SIZE_DEFAULT * PAGE_SIZE));
+}
+
 static unsigned long shmem_default_max_blocks(void)
 {
-	return totalram_pages() / 2;
+	return SHMEM_SIZE_DEFAULT;
 }
 
 static unsigned long shmem_default_max_inodes(void)
 {
 	unsigned long nr_pages = totalram_pages();
 
-	return min(nr_pages - totalhigh_pages(), nr_pages / 2);
+	return min(nr_pages - totalhigh_pages(), SHMEM_SIZE_DEFAULT);
 }
 #endif
 
@@ -3887,6 +3900,9 @@ int __init shmem_init(void)
 {
 	int error;
 
+#ifdef CONFIG_TMPFS
+	shmem_size_info();
+#endif
 	shmem_init_inodecache();
 
 	error = register_filesystem(&shmem_fs_type);
-- 
2.15.2

Re: [PATCH] shmem: add shmem_size option for full filesystem

[Qian Cai]

> On Oct 8, 2019, at 4:17 PM, Gary B. Genett <me@garybgenett.net> wrote:
> 
> Adds a kernel configuration option to specify the size of the shmem
> filesystem.  It is currently hard-coded to 50% of memory.  Users should
> have the option to set this value as they see fit.
> 
> A specific case where this would be necessary is if the initramfs were
> larger than half of the memory, such as a 2.5GB "live" filesystem on
> a system with 4GB of memory.  Without this option, this causes a kernel
> panic.  With this option, the user may specify the number of pages of
> memory they need for their root filesystem.
> 
> This patch creates the SHMEM_SIZE configuration option, which is
> specified as the number of memory pages to use for the shmem
> filesystem.  The default remains unchanged.  This patch has no impact
> unless the values are changed.
> 
> The option is marked as expert, and the help text is clear that it
> should only be set if the user knows what they are doing.

Hide it under the EXPERT does not mean the bar is lower to justify when introduced a new config option. What’s the benefits of this vs squashfs/resize?

Re: [PATCH] shmem: add shmem_size option for full filesystem

[Vlastimil Babka]

On 10/8/19 10:17 PM, Gary B. Genett wrote:
> Adds a kernel configuration option to specify the size of the shmem
> filesystem.  It is currently hard-coded to 50% of memory.  Users should
> have the option to set this value as they see fit.

Users can override the default via a mount option, no?

> A specific case where this would be necessary is if the initramfs were
> larger than half of the memory, such as a 2.5GB "live" filesystem on
> a system with 4GB of memory.  Without this option, this causes a kernel
> panic.  With this option, the user may specify the number of pages of
> memory they need for their root filesystem.

Can the initramfs boot parameter specify the needed mount option
somehow? If not, could we detect that we are mounting an initramfs, and
size it accordingly over 50%? Do we know in advance how much we need?
I'd rather avoid a new config option for this, if possible.

> This patch creates the SHMEM_SIZE configuration option, which is
> specified as the number of memory pages to use for the shmem
> filesystem.  The default remains unchanged.  This patch has no impact
> unless the values are changed.
> 
> The option is marked as expert, and the help text is clear that it
> should only be set if the user knows what they are doing.
> 
> Signed-off-by: Gary B. Genett <me@garybgenett.net>
> ---
>  init/Kconfig | 11 +++++++++++
>  mm/shmem.c   | 20 ++++++++++++++++++--
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index b4daad2bac23..fcff0f1ea3e8 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1537,6 +1537,17 @@ config SHMEM
>  	  option replaces shmem and tmpfs with the much simpler ramfs code,
>  	  which may be appropriate on small systems without swap.
>  
> +config SHMEM_SIZE
> +	int "Set the full shmem filesystem size" if EXPERT
> +	default 0
> +	depends on SHMEM
> +	help
> +	   Size of full shmem filesystem.  Defaults to 50% of memory,
> +	   dynamically.  Must be defined as the number of 4096 byte memory
> +	   pages, which will be static regardless of the amount of actual
> +	   memory.  Unless you have a special use case, this value should not
> +	   be changed.
> +
>  config AIO
>  	bool "Enable AIO support" if EXPERT
>  	default y
> diff --git a/mm/shmem.c b/mm/shmem.c
> index cd570cc79c76..d5baaf7ac9e8 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -95,6 +95,13 @@ static struct vfsmount *shm_mnt;
>  /* Symlink up to this size is kmalloc'ed instead of using a swappable page */
>  #define SHORT_SYMLINK_LEN 128
>  
> +/* Default size of shmem filesystem (defaults to 50% of memory) */
> +#if defined(CONFIG_SHMEM_SIZE) && (CONFIG_SHMEM_SIZE > 0)
> +#define SHMEM_SIZE_DEFAULT ((unsigned long)CONFIG_SHMEM_SIZE)
> +#else
> +#define SHMEM_SIZE_DEFAULT (totalram_pages() / 2)
> +#endif
> +
>  /*
>   * shmem_fallocate communicates with shmem_fault or shmem_writepage via
>   * inode->i_private (with i_mutex making sure that it has only one user at
> @@ -123,16 +130,22 @@ struct shmem_options {
>  };
>  
>  #ifdef CONFIG_TMPFS
> +static void shmem_size_info(void)
> +{
> +	pr_info("shmem: setting default size: %lu pages, %lu bytes\n",
> +		SHMEM_SIZE_DEFAULT, (SHMEM_SIZE_DEFAULT * PAGE_SIZE));
> +}
> +
>  static unsigned long shmem_default_max_blocks(void)
>  {
> -	return totalram_pages() / 2;
> +	return SHMEM_SIZE_DEFAULT;
>  }
>  
>  static unsigned long shmem_default_max_inodes(void)
>  {
>  	unsigned long nr_pages = totalram_pages();
>  
> -	return min(nr_pages - totalhigh_pages(), nr_pages / 2);
> +	return min(nr_pages - totalhigh_pages(), SHMEM_SIZE_DEFAULT);
>  }
>  #endif
>  
> @@ -3887,6 +3900,9 @@ int __init shmem_init(void)
>  {
>  	int error;
>  
> +#ifdef CONFIG_TMPFS
> +	shmem_size_info();
> +#endif
>  	shmem_init_inodecache();
>  
>  	error = register_filesystem(&shmem_fs_type);
> 

Re: [PATCH] shmem: add shmem_size option for full filesystem

[-Gary-]

From Qian:

> Hide it under the EXPERT does not mean the bar is lower to justify when introduced a new config option. What’s the benefits of this vs squashfs/resize?

From Vlastimil:

> Users can override the default via a mount option, no?

> Can the initramfs boot parameter specify the needed mount option
> somehow? If not, could we detect that we are mounting an initramfs, and
> size it accordingly over 50%? Do we know in advance how much we need?
> I'd rather avoid a new config option for this, if possible.

Thank you for clarifying all of that, Qian and Vlastimil.  I have been making the assumption, apparently mistaken, that adding kernel configuration options is a trivial change.  I stand educated.

Thank you for your ongoing patience with my passion regarding this.  I am *not* a kernel developer, understand and appreciate the work involved, and know there are many other important things which need attention.

I concede, but am going to submit one final patch which simply makes the value a define.

My fundamental issue/fix here is that it is a magic token buried in the code, and it should at least be a clearly defined default, as a matter of principle.  This will also make pre-compile sed/awk hacking much more reliable, for those so inclined.

If even that change is unpalatable, I will graciously accept a simple "reject, based on previous discussion", with gratitude.  <^)

Thank you again for your consideration, and all of your hard work and contributions to the community.

Happy Hacking!

-- Gary