Skip to contentSkip to conversation

kexec: use mmap instead of read for slurp_file()

RFC PATCH In review Michael Holzheu 3 participants 9 messages Aug 18, 2015 – Oct 15, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 kexec: use mmap instead of read for slurp_file()

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Therefore use mmap() for non-character devices to reduce the memory consumption of the kexec tool.

---
 kexec/kexec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
kexec/kexec.c +5 -3
index 8ce6885..fecf061 100644
@@ -26,6 +26,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <limits.h>
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/reboot.h>
@@ -552,11 +553,12 @@ char *slurp_file(const char *filename, off_t *r_size)
 		if (err < 0)
 			die("Can not seek to the begin of file %s: %s\n",
 					filename, strerror(errno));
+		buf = slurp_fd(fd, filename, size, &nread);
 	} else {
-		size = stats.st_size;
+		size = nread = stats.st_size;
+		buf = mmap(NULL, size,
+			   PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 	}
-
-	buf = slurp_fd(fd, filename, size, &nread);
 	if (!buf)
 		die("Cannot read %s", filename);
 
Signature · 7 lines
2.3.0


_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
#1 lore raw

PING: [RFC PATCH] kexec: use mmap instead of read for slurp_file()

Hello Simon,

Did you have time to look at my patch?

Regards,
Michael

Quoted 32 lines · On Tue, 18 Aug 2015 18:17:23 +0200 Michael Holzheu…

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Therefore use mmap() for non-character devices to reduce the memory consumption of the kexec tool.

---
 kexec/kexec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
kexec/kexec.c +2 -0
index 8ce6885..fecf061 100644
@@ -26,6 +26,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <limits.h>
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/reboot.h>
@@ -552,11 +553,12 @@ char *slurp_file(const char *filename, off_t *r_size)
 		if (err < 0)
 			die("Can not seek to the begin of file %s: %s\n",
 					filename, strerror(errno));
+		buf = slurp_fd(fd, filename, size, &nread);
 	} else {

kexec/kexec.c char *slurp_file(const char *filename, off_t *r_size) · line 564

Quoted 10 lines
kexec/kexec.c +3 -3
-		size = stats.st_size;
+		size = nread = stats.st_size;
+		buf = mmap(NULL, size,
+			   PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 	}
-
-	buf = slurp_fd(fd, filename, size, &nread);
 	if (!buf)
 		die("Cannot read %s", filename);
 

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#2 lore raw
In reply to Michael Holzheu

Hi Michael,

sorry for the delay in responding to your patch.

Quoted 8 lines · Michael Holzheu

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Therefore use mmap() for non-character devices to reduce the memory consumption of the kexec tool.

I'm not opposed to this change but I also don't see a strong motivation for it. I would imagine that the memory saving is not that large. And that the memory consumption disappears when the presumably short-lived kexec process exits.

Quoted 37 lines
---
 kexec/kexec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
kexec/kexec.c +5 -3
index 8ce6885..fecf061 100644
@@ -26,6 +26,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <limits.h>
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/reboot.h>
@@ -552,11 +553,12 @@ char *slurp_file(const char *filename, off_t *r_size)
 		if (err < 0)
 			die("Can not seek to the begin of file %s: %s\n",
 					filename, strerror(errno));
+		buf = slurp_fd(fd, filename, size, &nread);
 	} else {
-		size = stats.st_size;
+		size = nread = stats.st_size;
+		buf = mmap(NULL, size,
+			   PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 	}
-
-	buf = slurp_fd(fd, filename, size, &nread);
 	if (!buf)
 		die("Cannot read %s", filename);
 
Signature · 1 line
2.3.0

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#3 lore raw

On Wed, 2 Sep 2015 10:07:21 +0900
Simon Horman <[email protected]> wrote:
[snip]

Quoted 6 lines
Quoted 6 lines

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Quoted 7 lines
Quoted 2 lines

Therefore use mmap() for non-character devices to reduce the memory consumption of the kexec tool.

I'm not opposed to this change but I also don't see a strong motivation for it. I would imagine that the memory saving is not that large. And that the memory consumption disappears when the presumably short-lived kexec process exits.

Correct it will disappear.

The reason for the the patch is that we have the following scanario:

1) Boot a 4 GB Linux system
2) Read kernel and 1,5 GB ramdisk from external source into local tmpfs (ram)
3) kexec the kernel and ramdisk

So without the mmap patch for the kexec runtime we need:

1,5 GB (tmpfs) + 1,5 GB (kexec malloc) + 1,5 GB (kernel memory) = 4,5 GB

If we use mmap we only need 3 GB memory.

Regards,
Michael

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#4 lore raw
Quoted 25 lines · Michael Holzheu

On Wed, 2 Sep 2015 10:07:21 +0900
Simon Horman <[email protected]> wrote:
[snip]

Quoted 6 lines
Quoted 6 lines

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Quoted 7 lines
Quoted 2 lines

Therefore use mmap() for non-character devices to reduce the memory consumption of the kexec tool.

I'm not opposed to this change but I also don't see a strong motivation for it. I would imagine that the memory saving is not that large. And that the memory consumption disappears when the presumably short-lived kexec process exits.

Correct it will disappear.

The reason for the the patch is that we have the following scanario:

1) Boot a 4 GB Linux system
2) Read kernel and 1,5 GB ramdisk from external source into local tmpfs (ram)
3) kexec the kernel and ramdisk

Quoted 5 lines

So without the mmap patch for the kexec runtime we need:

1,5 GB (tmpfs) + 1,5 GB (kexec malloc) + 1,5 GB (kernel memory) = 4,5 GB

If we use mmap we only need 3 GB memory.

Thanks, I am convinced.

Could you repost the patch as a non-RFC?

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#5 lore raw

PATCH 1/1 kexec: use mmap instead of read for slurp_file()

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Therefore use mmap() for non-character devices to reduce the runtime memory consumption of the kexec tool.

The following use case illustrates the usefulness of this patch a bit more:

1) Boot a 4 GB Linux system
2) Read kernel and 1,5 GB ramdisk from external source into local tmpfs (ram)
3) kexec the kernel and ramdisk

Without this patch for the kexec runtime we need:

1,5 GB (tmpfs) + 1,5 GB (kexec malloc) + 1,5 GB (kernel memory) = 4,5 GB

---
 kexec/kexec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
kexec/kexec.c +5 -3
index 8ce6885..fecf061 100644
@@ -26,6 +26,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <limits.h>
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/reboot.h>
@@ -552,11 +553,12 @@ char *slurp_file(const char *filename, off_t *r_size)
 		if (err < 0)
 			die("Can not seek to the begin of file %s: %s\n",
 					filename, strerror(errno));
+		buf = slurp_fd(fd, filename, size, &nread);
 	} else {
-		size = stats.st_size;
+		size = nread = stats.st_size;
+		buf = mmap(NULL, size,
+			   PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 	}
-
-	buf = slurp_fd(fd, filename, size, &nread);
 	if (!buf)
 		die("Cannot read %s", filename);
 
Signature · 7 lines
2.3.0


_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Applied
#6 lore raw
Quoted 15 lines · Michael Holzheu

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Therefore use mmap() for non-character devices to reduce the runtime memory consumption of the kexec tool.

The following use case illustrates the usefulness of this patch a bit more:

1) Boot a 4 GB Linux system
2) Read kernel and 1,5 GB ramdisk from external source into local tmpfs (ram)
3) kexec the kernel and ramdisk

Quoted 5 lines

Without this patch for the kexec runtime we need:

1,5 GB (tmpfs) + 1,5 GB (kexec malloc) + 1,5 GB (kernel memory) = 4,5 GB

Thanks, applied.

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#7 lore raw
In reply to Michael Holzheu

Hi,

Quoted 24 lines · Michael Holzheu

The slurp_fd() function allocates memory and uses the read() system call. This results in double memory consumption for image and initrd:

1) Memory allocated in user space by the kexec tool
2) Memory allocated in kernel by the kexec() system call

Therefore use mmap() for non-character devices to reduce the runtime memory consumption of the kexec tool.

The following use case illustrates the usefulness of this patch a bit more:

1) Boot a 4 GB Linux system
2) Read kernel and 1,5 GB ramdisk from external source into local tmpfs (ram)
3) kexec the kernel and ramdisk

Without this patch for the kexec runtime we need:

1,5 GB (tmpfs) + 1,5 GB (kexec malloc) + 1,5 GB (kernel memory) = 4,5 GB

---
 kexec/kexec.c | 8 +++++---
Quoted 8 lines

if (err < 0)

Quoted 2 lines
	> 	> 	> die("Can not seek to the begin of file %s: %s\n",
	> 	> 	> 	> 	> filename, strerror(errno));

+> > > buf = slurp_fd(fd, filename, size, &nread);

Quoted 1 line
	> } else {

-> > > size = stats.st_size;
+> > > size = nread = stats.st_size;
+> > > buf = mmap(NULL, size,

With this change the caller can't tell if buf was malloc'ed or mmaped. The only safe thing it can do is to not call free() on the returned buf, this will lead to memory leakage for malloc'ed buffers.

-Geoff

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#8 lore raw

On Wed, 14 Oct 2015 17:05:52 -0700
Geoff Levand <[email protected]> wrote:

[snip]

Quoted 9 lines
Quoted 8 lines

if (err < 0)

Quoted 2 lines
	> 	> 	> die("Can not seek to the begin of file %s: %s\n",
	> 	> 	> 	> 	> filename, strerror(errno));

+> > > buf = slurp_fd(fd, filename, size, &nread);

Quoted 1 line
	> } else {

-> > > size = stats.st_size;
+> > > size = nread = stats.st_size;
+> > > buf = mmap(NULL, size,

Quoted 3 lines

With this change the caller can't tell if buf was malloc'ed or mmaped. The only safe thing it can do is to not call free() on the returned buf, this will lead to memory leakage for malloc'ed buffers.

I have read the code and have not found any free call. Therefore I assumed that the kexec approach is to not free the buffer *explicitly* and leave to the kernel to free it *automatically* at process exit.

@Simon: Was this assumption wrong?

Michael

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec