diff mbox series

[v6,3/6] initramfs: add INITRAMFS_PRESERVE_MTIME Kconfig option

Message ID 20220107133814.32655-4-ddiss@suse.de (mailing list archive)
State New, archived
Headers show
Series initramfs: "crc" cpio format and INITRAMFS_PRESERVE_MTIME | expand

Commit Message

David Disseldorp Jan. 7, 2022, 1:38 p.m. UTC 
initramfs cpio mtime preservation, as implemented in commit 889d51a10712
("initramfs: add option to preserve mtime from initramfs cpio images"),
uses a linked list to defer directory mtime processing until after all
other items in the cpio archive have been processed. This is done to
ensure that parent directory mtimes aren't overwritten via subsequent
child creation. Contrary to the 889d51a10712 commit message, the mtime
preservation behaviour is unconditional.

This change adds a new INITRAMFS_PRESERVE_MTIME Kconfig option, which
can be used to disable on-by-default mtime retention and in turn
speed up initramfs extraction, particularly for cpio archives with large
directory counts.

Benchmarks with a one million directory cpio archive extracted 20 times
demonstrated:
				mean extraction time (s)	std dev
INITRAMFS_PRESERVE_MTIME=y		3.808			 0.006
INITRAMFS_PRESERVE_MTIME unset		3.056			 0.004

The above extraction times were measured using ftrace
(initcall_finish - initcall_start) values for populate_rootfs() with
initramfs_async disabled.

Signed-off-by: David Disseldorp <ddiss@suse.de>
Reviewed-by: Martin Wilck <mwilck@suse.com>
[ddiss: rebase atop dir_entry.name flexible array member]
---
 init/Kconfig           | 10 ++++++++
 init/initramfs.c       | 50 +++-------------------------------------
 init/initramfs_mtime.h | 52 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 47 deletions(-)
 create mode 100644 init/initramfs_mtime.h

Comments

David Disseldorp March 8, 2022, 1:09 p.m. UTC | #1 
Ping on this patchset again...

@Andrew: while looking through lkml archives from a lifetime ago at
https://lkml.org/lkml/2008/8/16/59 , it appears that your preference at
the time was to drop the scattered INITRAMFS_PRESERVE_MTIME ifdefs prior
to merge.
I'd much appreciate your thoughts on the reintroduction of the option
based on the microbenchmark results below.

On Fri,  7 Jan 2022 14:38:11 +0100, David Disseldorp wrote:

> initramfs cpio mtime preservation, as implemented in commit 889d51a10712
> ("initramfs: add option to preserve mtime from initramfs cpio images"),
> uses a linked list to defer directory mtime processing until after all
> other items in the cpio archive have been processed. This is done to
> ensure that parent directory mtimes aren't overwritten via subsequent
> child creation. Contrary to the 889d51a10712 commit message, the mtime
> preservation behaviour is unconditional.
> 
> This change adds a new INITRAMFS_PRESERVE_MTIME Kconfig option, which
> can be used to disable on-by-default mtime retention and in turn
> speed up initramfs extraction, particularly for cpio archives with large
> directory counts.
> 
> Benchmarks with a one million directory cpio archive extracted 20 times
> demonstrated:
> 				mean extraction time (s)	std dev
> INITRAMFS_PRESERVE_MTIME=y		3.808			 0.006
> INITRAMFS_PRESERVE_MTIME unset		3.056			 0.004
> 
> The above extraction times were measured using ftrace
> (initcall_finish - initcall_start) values for populate_rootfs() with
> initramfs_async disabled.
> 
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> Reviewed-by: Martin Wilck <mwilck@suse.com>
> [ddiss: rebase atop dir_entry.name flexible array member]
> ---
>  init/Kconfig           | 10 ++++++++
>  init/initramfs.c       | 50 +++-------------------------------------
>  init/initramfs_mtime.h | 52 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+), 47 deletions(-)
>  create mode 100644 init/initramfs_mtime.h
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 4b7bac10c72d..a98f63d3c366 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1357,6 +1357,16 @@ config BOOT_CONFIG
>  
>  	  If unsure, say Y.
>  
> +config INITRAMFS_PRESERVE_MTIME
> +	bool "Preserve cpio archive mtimes in initramfs"
> +	default y
> +	help
> +	  Each entry in an initramfs cpio archive carries an mtime value. When
> +	  enabled, extracted cpio items take this mtime, with directory mtime
> +	  setting deferred until after creation of any child entries.
> +
> +	  If unsure, say Y.
> +
>  choice
>  	prompt "Compiler optimization level"
>  	default CC_OPTIMIZE_FOR_PERFORMANCE
> diff --git a/init/initramfs.c b/init/initramfs.c
> index 656d2d71349f..5b4ca8ecadb5 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -17,6 +17,8 @@
>  #include <linux/init_syscalls.h>
>  #include <linux/umh.h>
>  
> +#include "initramfs_mtime.h"
> +
>  static ssize_t __init xwrite(struct file *file, const char *p, size_t count,
>  		loff_t *pos)
>  {
> @@ -116,48 +118,6 @@ static void __init free_hash(void)
>  	}
>  }
>  
> -static long __init do_utime(char *filename, time64_t mtime)
> -{
> -	struct timespec64 t[2];
> -
> -	t[0].tv_sec = mtime;
> -	t[0].tv_nsec = 0;
> -	t[1].tv_sec = mtime;
> -	t[1].tv_nsec = 0;
> -	return init_utimes(filename, t);
> -}
> -
> -static __initdata LIST_HEAD(dir_list);
> -struct dir_entry {
> -	struct list_head list;
> -	time64_t mtime;
> -	char name[];
> -};
> -
> -static void __init dir_add(const char *name, time64_t mtime)
> -{
> -	size_t nlen = strlen(name) + 1;
> -	struct dir_entry *de;
> -
> -	de = kmalloc(sizeof(struct dir_entry) + nlen, GFP_KERNEL);
> -	if (!de)
> -		panic_show_mem("can't allocate dir_entry buffer");
> -	INIT_LIST_HEAD(&de->list);
> -	strscpy(de->name, name, nlen);
> -	de->mtime = mtime;
> -	list_add(&de->list, &dir_list);
> -}
> -
> -static void __init dir_utime(void)
> -{
> -	struct dir_entry *de, *tmp;
> -	list_for_each_entry_safe(de, tmp, &dir_list, list) {
> -		list_del(&de->list);
> -		do_utime(de->name, de->mtime);
> -		kfree(de);
> -	}
> -}
> -
>  static __initdata time64_t mtime;
>  
>  /* cpio header parsing */
> @@ -381,14 +341,10 @@ static int __init do_name(void)
>  static int __init do_copy(void)
>  {
>  	if (byte_count >= body_len) {
> -		struct timespec64 t[2] = { };
>  		if (xwrite(wfile, victim, body_len, &wfile_pos) != body_len)
>  			error("write error");
>  
> -		t[0].tv_sec = mtime;
> -		t[1].tv_sec = mtime;
> -		vfs_utimes(&wfile->f_path, t);
> -
> +		do_utime_path(&wfile->f_path, mtime);
>  		fput(wfile);
>  		eat(body_len);
>  		state = SkipIt;
> diff --git a/init/initramfs_mtime.h b/init/initramfs_mtime.h
> new file mode 100644
> index 000000000000..688ed4b6f327
> --- /dev/null
> +++ b/init/initramfs_mtime.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifdef CONFIG_INITRAMFS_PRESERVE_MTIME
> +static void __init do_utime(char *filename, time64_t mtime)
> +{
> +	struct timespec64 t[2] = { { .tv_sec = mtime }, { .tv_sec = mtime } };
> +	init_utimes(filename, t);
> +}
> +
> +static void __init do_utime_path(const struct path *path, time64_t mtime)
> +{
> +	struct timespec64 t[2] = { { .tv_sec = mtime }, { .tv_sec = mtime } };
> +	vfs_utimes(path, t);
> +}
> +
> +static __initdata LIST_HEAD(dir_list);
> +struct dir_entry {
> +	struct list_head list;
> +	time64_t mtime;
> +	char name[];
> +};
> +
> +static void __init dir_add(const char *name, time64_t mtime)
> +{
> +	size_t nlen = strlen(name) + 1;
> +	struct dir_entry *de;
> +
> +	de = kmalloc(sizeof(struct dir_entry) + nlen, GFP_KERNEL);
> +	if (!de)
> +		panic("can't allocate dir_entry buffer");
> +	INIT_LIST_HEAD(&de->list);
> +	strscpy(de->name, name, nlen);
> +	de->mtime = mtime;
> +	list_add(&de->list, &dir_list);
> +}
> +
> +static void __init dir_utime(void)
> +{
> +	struct dir_entry *de, *tmp;
> +
> +	list_for_each_entry_safe(de, tmp, &dir_list, list) {
> +		list_del(&de->list);
> +		do_utime(de->name, de->mtime);
> +		kfree(de);
> +	}
> +}
> +#else
> +static void __init do_utime(char *filename, time64_t mtime) {}
> +static void __init do_utime_path(const struct path *path, time64_t mtime) {}
> +static void __init dir_add(const char *name, time64_t mtime) {}
> +static void __init dir_utime(void) {}
> +#endif
Andrew Morton March 10, 2022, 3:41 a.m. UTC | #2 
On Tue, 8 Mar 2022 14:09:42 +0100 David Disseldorp <ddiss@suse.de> wrote:

> @Andrew: while looking through lkml archives from a lifetime ago at
> https://lkml.org/lkml/2008/8/16/59 , it appears that your preference at
> the time was to drop the scattered INITRAMFS_PRESERVE_MTIME ifdefs prior
> to merge.

That was but moments ago.

> I'd much appreciate your thoughts on the reintroduction of the option
> based on the microbenchmark results below.

Thanks, I'll take a look after -rc1.  Which gives reviewers much time
to do their thing!
diff mbox series

Patch

diff --git a/init/Kconfig b/init/Kconfig
index 4b7bac10c72d..a98f63d3c366 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1357,6 +1357,16 @@  config BOOT_CONFIG
 
 	  If unsure, say Y.
 
+config INITRAMFS_PRESERVE_MTIME
+	bool "Preserve cpio archive mtimes in initramfs"
+	default y
+	help
+	  Each entry in an initramfs cpio archive carries an mtime value. When
+	  enabled, extracted cpio items take this mtime, with directory mtime
+	  setting deferred until after creation of any child entries.
+
+	  If unsure, say Y.
+
 choice
 	prompt "Compiler optimization level"
 	default CC_OPTIMIZE_FOR_PERFORMANCE
diff --git a/init/initramfs.c b/init/initramfs.c
index 656d2d71349f..5b4ca8ecadb5 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -17,6 +17,8 @@ 
 #include <linux/init_syscalls.h>
 #include <linux/umh.h>
 
+#include "initramfs_mtime.h"
+
 static ssize_t __init xwrite(struct file *file, const char *p, size_t count,
 		loff_t *pos)
 {
@@ -116,48 +118,6 @@  static void __init free_hash(void)
 	}
 }
 
-static long __init do_utime(char *filename, time64_t mtime)
-{
-	struct timespec64 t[2];
-
-	t[0].tv_sec = mtime;
-	t[0].tv_nsec = 0;
-	t[1].tv_sec = mtime;
-	t[1].tv_nsec = 0;
-	return init_utimes(filename, t);
-}
-
-static __initdata LIST_HEAD(dir_list);
-struct dir_entry {
-	struct list_head list;
-	time64_t mtime;
-	char name[];
-};
-
-static void __init dir_add(const char *name, time64_t mtime)
-{
-	size_t nlen = strlen(name) + 1;
-	struct dir_entry *de;
-
-	de = kmalloc(sizeof(struct dir_entry) + nlen, GFP_KERNEL);
-	if (!de)
-		panic_show_mem("can't allocate dir_entry buffer");
-	INIT_LIST_HEAD(&de->list);
-	strscpy(de->name, name, nlen);
-	de->mtime = mtime;
-	list_add(&de->list, &dir_list);
-}
-
-static void __init dir_utime(void)
-{
-	struct dir_entry *de, *tmp;
-	list_for_each_entry_safe(de, tmp, &dir_list, list) {
-		list_del(&de->list);
-		do_utime(de->name, de->mtime);
-		kfree(de);
-	}
-}
-
 static __initdata time64_t mtime;
 
 /* cpio header parsing */
@@ -381,14 +341,10 @@  static int __init do_name(void)
 static int __init do_copy(void)
 {
 	if (byte_count >= body_len) {
-		struct timespec64 t[2] = { };
 		if (xwrite(wfile, victim, body_len, &wfile_pos) != body_len)
 			error("write error");
 
-		t[0].tv_sec = mtime;
-		t[1].tv_sec = mtime;
-		vfs_utimes(&wfile->f_path, t);
-
+		do_utime_path(&wfile->f_path, mtime);
 		fput(wfile);
 		eat(body_len);
 		state = SkipIt;
diff --git a/init/initramfs_mtime.h b/init/initramfs_mtime.h
new file mode 100644
index 000000000000..688ed4b6f327
--- /dev/null
+++ b/init/initramfs_mtime.h
@@ -0,0 +1,52 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifdef CONFIG_INITRAMFS_PRESERVE_MTIME
+static void __init do_utime(char *filename, time64_t mtime)
+{
+	struct timespec64 t[2] = { { .tv_sec = mtime }, { .tv_sec = mtime } };
+	init_utimes(filename, t);
+}
+
+static void __init do_utime_path(const struct path *path, time64_t mtime)
+{
+	struct timespec64 t[2] = { { .tv_sec = mtime }, { .tv_sec = mtime } };
+	vfs_utimes(path, t);
+}
+
+static __initdata LIST_HEAD(dir_list);
+struct dir_entry {
+	struct list_head list;
+	time64_t mtime;
+	char name[];
+};
+
+static void __init dir_add(const char *name, time64_t mtime)
+{
+	size_t nlen = strlen(name) + 1;
+	struct dir_entry *de;
+
+	de = kmalloc(sizeof(struct dir_entry) + nlen, GFP_KERNEL);
+	if (!de)
+		panic("can't allocate dir_entry buffer");
+	INIT_LIST_HEAD(&de->list);
+	strscpy(de->name, name, nlen);
+	de->mtime = mtime;
+	list_add(&de->list, &dir_list);
+}
+
+static void __init dir_utime(void)
+{
+	struct dir_entry *de, *tmp;
+
+	list_for_each_entry_safe(de, tmp, &dir_list, list) {
+		list_del(&de->list);
+		do_utime(de->name, de->mtime);
+		kfree(de);
+	}
+}
+#else
+static void __init do_utime(char *filename, time64_t mtime) {}
+static void __init do_utime_path(const struct path *path, time64_t mtime) {}
+static void __init dir_add(const char *name, time64_t mtime) {}
+static void __init dir_utime(void) {}
+#endif