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

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

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

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 kexec/kexec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 8ce6885..fecf061 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -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);
 
-- 
2.3.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Michael Holzheu]

Hello Simon,

Did you have time to look at my patch?

Regards,
Michael

On Tue, 18 Aug 2015 18:17:23 +0200
Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

> 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.
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  kexec/kexec.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index 8ce6885..fecf061 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -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);
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Simon Horman]

Hi Michael,

sorry for the delay in responding to your patch.

On Tue, Aug 18, 2015 at 06:17:23PM +0200, Michael Holzheu wrote:
> 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.

> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  kexec/kexec.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index 8ce6885..fecf061 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -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);
>  
> -- 
> 2.3.0
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Michael Holzheu]

On Wed, 2 Sep 2015 10:07:21 +0900
Simon Horman <horms@verge.net.au> wrote:
[snip]
> > 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.

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
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Simon Horman]

On Wed, Sep 02, 2015 at 10:48:39AM +0200, Michael Holzheu wrote:
> On Wed, 2 Sep 2015 10:07:21 +0900
> Simon Horman <horms@verge.net.au> wrote:
> [snip]
> > > 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.
> 
> 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.

Thanks, I am convinced.

Could you repost the patch as a non-RFC?

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[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

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 kexec/kexec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 8ce6885..fecf061 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -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);
 
-- 
2.3.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Simon Horman]

On Fri, Sep 04, 2015 at 02:11:59PM +0200, Michael Holzheu wrote:
> 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
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>

Thanks, applied.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Geoff Levand]

Hi,

On Fri, 2015-09-04 at 14:11 +0200, Michael Holzheu wrote:
> 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
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  kexec/kexec.c | 8 +++++---
> 

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

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
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Michael Holzheu]

On Wed, 14 Oct 2015 17:05:52 -0700
Geoff Levand <geoff@infradead.org> wrote:

[snip]

> >  		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,
> 
> 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
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec