All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxc: lzma build fix
@ 2011-01-26 15:22 Christoph Egger
  2011-01-26 16:16 ` Ian Jackson
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Egger @ 2011-01-26 15:22 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]


Hi!

Attached patch lets libxc build with lzma support on NetBSD.
NetBSD doesn't have sysconf(_SC_PHYS_PAGES).

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_libxc_lzma.diff --]
[-- Type: text/x-diff, Size: 451 bytes --]

diff -r fb8a14647f4d -r 48852e88d506 tools/libxc/xc_dom_bzimageloader.c
--- a/tools/libxc/xc_dom_bzimageloader.c
+++ b/tools/libxc/xc_dom_bzimageloader.c
@@ -144,7 +144,11 @@ static uint64_t physmem(void)
 {
     uint64_t ret = 0;
     const long pagesize = sysconf(_SC_PAGESIZE);
+#ifdef _SC_PHYS_PAGES
     const long pages = sysconf(_SC_PHYS_PAGES);
+#else
+    const long pages = -1;
+#endif
 
     if ( (pagesize != -1) || (pages != -1) )
     {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-26 15:22 [PATCH] libxc: lzma build fix Christoph Egger
@ 2011-01-26 16:16 ` Ian Jackson
  2011-01-26 17:27   ` Christoph Egger
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2011-01-26 16:16 UTC (permalink / raw)
  To: Christoph Egger; +Cc: xen-devel

Christoph Egger writes ("[Xen-devel] [PATCH] libxc: lzma build fix"):
> Attached patch lets libxc build with lzma support on NetBSD.
> NetBSD doesn't have sysconf(_SC_PHYS_PAGES).

Can you please break this out into separate os-dependent code in an
appropriate different file ?  These kind of #ifdefs break up the code
and make it hard to see what's going on.

Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-26 16:16 ` Ian Jackson
@ 2011-01-26 17:27   ` Christoph Egger
  2011-01-27 19:04     ` Ian Jackson
  2011-01-27 19:54     ` Ian Campbell
  0 siblings, 2 replies; 12+ messages in thread
From: Christoph Egger @ 2011-01-26 17:27 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Wednesday 26 January 2011 17:16:11 Ian Jackson wrote:
> Christoph Egger writes ("[Xen-devel] [PATCH] libxc: lzma build fix"):
> > Attached patch lets libxc build with lzma support on NetBSD.
> > NetBSD doesn't have sysconf(_SC_PHYS_PAGES).
>
> Can you please break this out into separate os-dependent code in an
> appropriate different file ?  These kind of #ifdefs break up the code
> and make it hard to see what's going on.

Yes. Attached patch factors physmem() out into os-dependent files
and renamed it to xc_get_physmem() to not pollute the namespace.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_libxc_lzma.diff --]
[-- Type: text/x-diff, Size: 3698 bytes --]

diff -r c332e775ba53 tools/libxc/xc_dom_bzimageloader.c
--- a/tools/libxc/xc_dom_bzimageloader.c	Wed Jan 26 17:44:59 2011 +0100
+++ b/tools/libxc/xc_dom_bzimageloader.c	Wed Jan 26 18:23:17 2011 +0100
@@ -140,27 +140,6 @@ static int xc_try_bzip2_decode(
 
 #include <lzma.h>
 
-static uint64_t physmem(void)
-{
-    uint64_t ret = 0;
-    const long pagesize = sysconf(_SC_PAGESIZE);
-    const long pages = sysconf(_SC_PHYS_PAGES);
-
-    if ( (pagesize != -1) || (pages != -1) )
-    {
-        /*
-         * According to docs, pagesize * pages can overflow.
-         * Simple case is 32-bit box with 4 GiB or more RAM,
-         * which may report exactly 4 GiB of RAM, and "long"
-         * being 32-bit will overflow. Casting to uint64_t
-         * hopefully avoids overflows in the near future.
-         */
-        ret = (uint64_t)(pagesize) * (uint64_t)(pages);
-    }
-
-    return ret;
-}
-
 static int xc_try_lzma_decode(
     struct xc_dom_image *dom, void **blob, size_t *size)
 {
@@ -173,7 +152,7 @@ static int xc_try_lzma_decode(
     int outsize;
     const char *msg;
 
-    ret = lzma_alone_decoder(&stream, physmem() / 3);
+    ret = lzma_alone_decoder(&stream, xc_get_physmem() / 3);
     if ( ret != LZMA_OK )
     {
         DOMPRINTF("LZMA: Failed to init stream decoder");
diff -r c332e775ba53 tools/libxc/xc_linux.c
--- a/tools/libxc/xc_linux.c	Wed Jan 26 17:44:59 2011 +0100
+++ b/tools/libxc/xc_linux.c	Wed Jan 26 18:23:17 2011 +0100
@@ -55,6 +55,27 @@ void discard_file_cache(xc_interface *xc
     errno = saved_errno;
 }
 
+uint64_t xc_get_physmem(void)
+{
+    uint64_t ret = 0;
+    const long pagesize = sysconf(_SC_PAGESIZE);
+    const long pages = sysconf(_SC_PHYS_PAGES);
+
+    if ( (pagesize != -1) || (pages != -1) )
+    {
+        /*
+         * According to docs, pagesize * pages can overflow.
+         * Simple case is 32-bit box with 4 GiB or more RAM,
+         * which may report exactly 4 GiB of RAM, and "long"
+         * being 32-bit will overflow. Casting to uint64_t
+         * hopefully avoids overflows in the near future.
+         */
+        ret = (uint64_t)(pagesize) * (uint64_t)(pages);
+    }
+
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r c332e775ba53 tools/libxc/xc_netbsd.c
--- a/tools/libxc/xc_netbsd.c	Wed Jan 26 17:44:59 2011 +0100
+++ b/tools/libxc/xc_netbsd.c	Wed Jan 26 18:23:17 2011 +0100
@@ -23,6 +23,9 @@
 #include <xen/sys/evtchn.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <stdio.h>
+#include <errno.h>
+#include <sys/sysctl.h>
 
 static xc_osdep_handle netbsd_privcmd_open(xc_interface *xch)
 {
@@ -351,6 +354,24 @@ void discard_file_cache(xc_interface *xc
     errno = saved_errno;
 }
 
+uint64_t xc_get_physmem(void)
+{
+    int mib[2], rc;
+    size_t len;
+    uint64_t physmem;
+
+    mib[0] = CTL_HW;
+    mib[1] = HW_PHYSMEM64;
+    rc = sysctl(mib, 2, &physmem, &len, NULL, 0);
+
+    if (rc == -1) {
+        /* PERROR("%s: Failed to get hw.physmem64: %s\n", strerror(errno)); */
+        return 0;
+    }
+
+    return physmem;
+}
+
 static struct xc_osdep_ops *netbsd_osdep_init(xc_interface *xch, enum xc_osdep_type type)
 {
     switch ( type )
diff -r c332e775ba53 tools/libxc/xc_private.h
--- a/tools/libxc/xc_private.h	Wed Jan 26 17:44:59 2011 +0100
+++ b/tools/libxc/xc_private.h	Wed Jan 26 18:23:17 2011 +0100
@@ -275,6 +275,9 @@ void bitmap_byte_to_64(uint64_t *lp, con
 /* Optionally flush file to disk and discard page cache */
 void discard_file_cache(xc_interface *xch, int fd, int flush);
 
+/* How much physical RAM is available? */
+uint64_t xc_get_physmem(void);
+
 #define MAX_MMU_UPDATES 1024
 struct xc_mmu {
     mmu_update_t updates[MAX_MMU_UPDATES];

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-26 17:27   ` Christoph Egger
@ 2011-01-27 19:04     ` Ian Jackson
  2011-01-27 19:54     ` Ian Campbell
  1 sibling, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2011-01-27 19:04 UTC (permalink / raw)
  To: Christoph Egger; +Cc: xen-devel

Christoph Egger writes ("Re: [Xen-devel] [PATCH] libxc: lzma build fix"):
> Yes. Attached patch factors physmem() out into os-dependent files
> and renamed it to xc_get_physmem() to not pollute the namespace.

Great, thanks.

I fixed up your commit message, so it looked like this:

   libxc: break xc_get_physmem out into os-dependent files

   NetBSD doesn't have sysconf(_SC_PHYS_PAGES).

   Factor physmem() out into os-dependent files and rename it to
   xc_get_physmem() so as not to pollute the namespace.

   Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
   Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
   Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-26 17:27   ` Christoph Egger
  2011-01-27 19:04     ` Ian Jackson
@ 2011-01-27 19:54     ` Ian Campbell
  2011-01-28 11:45       ` Ian Jackson
  1 sibling, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2011-01-27 19:54 UTC (permalink / raw)
  To: Christoph Egger; +Cc: xen-devel, Ian Jackson


On Wed, 2011-01-26 at 17:27 +0000, Christoph Egger wrote: 
> On Wednesday 26 January 2011 17:16:11 Ian Jackson wrote:
> > Christoph Egger writes ("[Xen-devel] [PATCH] libxc: lzma build fix"):
> > > Attached patch lets libxc build with lzma support on NetBSD.
> > > NetBSD doesn't have sysconf(_SC_PHYS_PAGES).
> >
> > Can you please break this out into separate os-dependent code in an
> > appropriate different file ?  These kind of #ifdefs break up the code
> > and make it hard to see what's going on.
> 
> Yes. Attached patch factors physmem() out into os-dependent files
> and renamed it to xc_get_physmem() to not pollute the namespace.

I see this has now been applied, which is my fault for putting this
aside to reply to later and then forgetting...

The physmem value calculated by this function is only used as an
argument to lzma_alone_decoder, it is divided by 3 to get the memory
limit for the decoder.

It's not clear to me why a userspace lzma decode would want to use that
particular value, what bearing it has on anything or why it would assume
it could use 1/3 of the total RAM in the system (potentially quite a
large amount of RAM) as opposed to any other limit number.

Why not just hardcode 32M or something? A quick scan through the
rdepends on Debian shows at least a couple of users doing so.

Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-27 19:54     ` Ian Campbell
@ 2011-01-28 11:45       ` Ian Jackson
  2011-01-28 18:51         ` Ian Jackson
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2011-01-28 11:45 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Christoph Egger, xen-devel

Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxc: lzma build fix"):
> The physmem value calculated by this function is only used as an
> argument to lzma_alone_decoder, it is divided by 3 to get the memory
> limit for the decoder.

I hadn't spotted that.  I should have looked more closely.

> It's not clear to me why a userspace lzma decode would want to use that
> particular value, what bearing it has on anything or why it would assume
> it could use 1/3 of the total RAM in the system (potentially quite a
> large amount of RAM) as opposed to any other limit number.

It's dom0's "physmem", so not the whole system, but yes.

> Why not just hardcode 32M or something? A quick scan through the
> rdepends on Debian shows at least a couple of users doing so.

Yes, we could revert this patch and hardcode a value.  32M seems
plausible.

Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-28 11:45       ` Ian Jackson
@ 2011-01-28 18:51         ` Ian Jackson
  2011-01-28 19:14           ` Ian Campbell
  2011-02-11 13:41           ` Jan Beulich
  0 siblings, 2 replies; 12+ messages in thread
From: Ian Jackson @ 2011-01-28 18:51 UTC (permalink / raw)
  To: Ian Campbell, Christoph Egger, xen-devel

Ian Jackson writes ("Re: [Xen-devel] [PATCH] libxc: lzma build fix"):
> Yes, we could revert this patch and hardcode a value.  32M seems
> plausible.

How about this.

Ian.


libxc: Do not use host physmem as parameter to lzma decoder

It's not clear why a userspace lzma decode would want to use that
particular value, what bearing it has on anything or why it would
assume it could use 1/3 of the total RAM in the system (potentially
quite a large amount of RAM) as opposed to any other limit number.

Instead, hardcode 32Mby.

This reverts 22830:c80960244942, removes the xc_get_physmem/physmem
function entirely, and replaces the expression at the call site with a
fixed constant.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>

diff -r 88cf07fed7d2 tools/libxc/xc_dom_bzimageloader.c
--- a/tools/libxc/xc_dom_bzimageloader.c	Fri Jan 28 18:39:09 2011 +0000
+++ b/tools/libxc/xc_dom_bzimageloader.c	Fri Jan 28 18:49:08 2011 +0000
@@ -152,7 +152,7 @@ static int xc_try_lzma_decode(
     int outsize;
     const char *msg;
 
-    ret = lzma_alone_decoder(&stream, xc_get_physmem() / 3);
+    ret = lzma_alone_decoder(&stream, 32*1024*1024);
     if ( ret != LZMA_OK )
     {
         DOMPRINTF("LZMA: Failed to init stream decoder");
diff -r 88cf07fed7d2 tools/libxc/xc_linux.c
--- a/tools/libxc/xc_linux.c	Fri Jan 28 18:39:09 2011 +0000
+++ b/tools/libxc/xc_linux.c	Fri Jan 28 18:49:08 2011 +0000
@@ -55,27 +55,6 @@ void discard_file_cache(xc_interface *xc
     errno = saved_errno;
 }
 
-uint64_t xc_get_physmem(void)
-{
-    uint64_t ret = 0;
-    const long pagesize = sysconf(_SC_PAGESIZE);
-    const long pages = sysconf(_SC_PHYS_PAGES);
-
-    if ( (pagesize != -1) || (pages != -1) )
-    {
-        /*
-         * According to docs, pagesize * pages can overflow.
-         * Simple case is 32-bit box with 4 GiB or more RAM,
-         * which may report exactly 4 GiB of RAM, and "long"
-         * being 32-bit will overflow. Casting to uint64_t
-         * hopefully avoids overflows in the near future.
-         */
-        ret = (uint64_t)(pagesize) * (uint64_t)(pages);
-    }
-
-    return ret;
-}
-
 /*
  * Local variables:
  * mode: C
diff -r 88cf07fed7d2 tools/libxc/xc_netbsd.c
--- a/tools/libxc/xc_netbsd.c	Fri Jan 28 18:39:09 2011 +0000
+++ b/tools/libxc/xc_netbsd.c	Fri Jan 28 18:49:08 2011 +0000
@@ -23,9 +23,6 @@
 #include <xen/sys/evtchn.h>
 #include <unistd.h>
 #include <fcntl.h>
-#include <stdio.h>
-#include <errno.h>
-#include <sys/sysctl.h>
 
 static xc_osdep_handle netbsd_privcmd_open(xc_interface *xch)
 {
@@ -354,24 +351,6 @@ void discard_file_cache(xc_interface *xc
     errno = saved_errno;
 }
 
-uint64_t xc_get_physmem(void)
-{
-    int mib[2], rc;
-    size_t len;
-    uint64_t physmem;
-
-    mib[0] = CTL_HW;
-    mib[1] = HW_PHYSMEM64;
-    rc = sysctl(mib, 2, &physmem, &len, NULL, 0);
-
-    if (rc == -1) {
-        /* PERROR("%s: Failed to get hw.physmem64: %s\n", strerror(errno)); */
-        return 0;
-    }
-
-    return physmem;
-}
-
 static struct xc_osdep_ops *netbsd_osdep_init(xc_interface *xch, enum xc_osdep_type type)
 {
     switch ( type )
diff -r 88cf07fed7d2 tools/libxc/xc_private.h
--- a/tools/libxc/xc_private.h	Fri Jan 28 18:39:09 2011 +0000
+++ b/tools/libxc/xc_private.h	Fri Jan 28 18:49:08 2011 +0000
@@ -275,9 +275,6 @@ void bitmap_byte_to_64(uint64_t *lp, con
 /* Optionally flush file to disk and discard page cache */
 void discard_file_cache(xc_interface *xch, int fd, int flush);
 
-/* How much physical RAM is available? */
-uint64_t xc_get_physmem(void);
-
 #define MAX_MMU_UPDATES 1024
 struct xc_mmu {
     mmu_update_t updates[MAX_MMU_UPDATES];

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-28 18:51         ` Ian Jackson
@ 2011-01-28 19:14           ` Ian Campbell
  2011-01-28 19:38             ` Ian Jackson
  2011-02-11 13:41           ` Jan Beulich
  1 sibling, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2011-01-28 19:14 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Christoph Egger, xen-devel

On Fri, 2011-01-28 at 18:51 +0000, Ian Jackson wrote: 
> Ian Jackson writes ("Re: [Xen-devel] [PATCH] libxc: lzma build fix"):
> > Yes, we could revert this patch and hardcode a value.  32M seems
> > plausible.
> 
> How about this.
> 
> Ian.
> 
> 
> libxc: Do not use host physmem as parameter to lzma decoder

Strictly speaking it's the dom0 physmem.

> It's not clear why a userspace lzma decode would want to use that
> particular value, what bearing it has on anything or why it would
> assume it could use 1/3 of the total RAM in the system (potentially
> quite a large amount of RAM) as opposed to any other limit number.

Sounds good to me ;-)

> Instead, hardcode 32Mby.
> 
> This reverts 22830:c80960244942, removes the xc_get_physmem/physmem
> function entirely, and replaces the expression at the call site with a
> fixed constant.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>

> Cc: Christoph Egger <Christoph.Egger@amd.com>
> 
> diff -r 88cf07fed7d2 tools/libxc/xc_dom_bzimageloader.c
> --- a/tools/libxc/xc_dom_bzimageloader.c	Fri Jan 28 18:39:09 2011 +0000
> +++ b/tools/libxc/xc_dom_bzimageloader.c	Fri Jan 28 18:49:08 2011 +0000
> @@ -152,7 +152,7 @@ static int xc_try_lzma_decode(
>      int outsize;
>      const char *msg;
>  
> -    ret = lzma_alone_decoder(&stream, xc_get_physmem() / 3);
> +    ret = lzma_alone_decoder(&stream, 32*1024*1024);
>      if ( ret != LZMA_OK )
>      {
>          DOMPRINTF("LZMA: Failed to init stream decoder");
> diff -r 88cf07fed7d2 tools/libxc/xc_linux.c
> --- a/tools/libxc/xc_linux.c	Fri Jan 28 18:39:09 2011 +0000
> +++ b/tools/libxc/xc_linux.c	Fri Jan 28 18:49:08 2011 +0000
> @@ -55,27 +55,6 @@ void discard_file_cache(xc_interface *xc
>      errno = saved_errno;
>  }
>  
> -uint64_t xc_get_physmem(void)
> -{
> -    uint64_t ret = 0;
> -    const long pagesize = sysconf(_SC_PAGESIZE);
> -    const long pages = sysconf(_SC_PHYS_PAGES);
> -
> -    if ( (pagesize != -1) || (pages != -1) )
> -    {
> -        /*
> -         * According to docs, pagesize * pages can overflow.
> -         * Simple case is 32-bit box with 4 GiB or more RAM,
> -         * which may report exactly 4 GiB of RAM, and "long"
> -         * being 32-bit will overflow. Casting to uint64_t
> -         * hopefully avoids overflows in the near future.
> -         */
> -        ret = (uint64_t)(pagesize) * (uint64_t)(pages);
> -    }
> -
> -    return ret;
> -}
> -
>  /*
>   * Local variables:
>   * mode: C
> diff -r 88cf07fed7d2 tools/libxc/xc_netbsd.c
> --- a/tools/libxc/xc_netbsd.c	Fri Jan 28 18:39:09 2011 +0000
> +++ b/tools/libxc/xc_netbsd.c	Fri Jan 28 18:49:08 2011 +0000
> @@ -23,9 +23,6 @@
>  #include <xen/sys/evtchn.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> -#include <stdio.h>
> -#include <errno.h>
> -#include <sys/sysctl.h>
>  
>  static xc_osdep_handle netbsd_privcmd_open(xc_interface *xch)
>  {
> @@ -354,24 +351,6 @@ void discard_file_cache(xc_interface *xc
>      errno = saved_errno;
>  }
>  
> -uint64_t xc_get_physmem(void)
> -{
> -    int mib[2], rc;
> -    size_t len;
> -    uint64_t physmem;
> -
> -    mib[0] = CTL_HW;
> -    mib[1] = HW_PHYSMEM64;
> -    rc = sysctl(mib, 2, &physmem, &len, NULL, 0);
> -
> -    if (rc == -1) {
> -        /* PERROR("%s: Failed to get hw.physmem64: %s\n", strerror(errno)); */
> -        return 0;
> -    }
> -
> -    return physmem;
> -}
> -
>  static struct xc_osdep_ops *netbsd_osdep_init(xc_interface *xch, enum xc_osdep_type type)
>  {
>      switch ( type )
> diff -r 88cf07fed7d2 tools/libxc/xc_private.h
> --- a/tools/libxc/xc_private.h	Fri Jan 28 18:39:09 2011 +0000
> +++ b/tools/libxc/xc_private.h	Fri Jan 28 18:49:08 2011 +0000
> @@ -275,9 +275,6 @@ void bitmap_byte_to_64(uint64_t *lp, con
>  /* Optionally flush file to disk and discard page cache */
>  void discard_file_cache(xc_interface *xch, int fd, int flush);
>  
> -/* How much physical RAM is available? */
> -uint64_t xc_get_physmem(void);
> -
>  #define MAX_MMU_UPDATES 1024
>  struct xc_mmu {
>      mmu_update_t updates[MAX_MMU_UPDATES];

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-28 19:14           ` Ian Campbell
@ 2011-01-28 19:38             ` Ian Jackson
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2011-01-28 19:38 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Christoph Egger, xen-devel

Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxc: lzma build fix"):
> Strictly speaking it's the dom0 physmem.

Ah, yes, I shouldn't speak so loosely.

> Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>

Applied, thanks.

Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-01-28 18:51         ` Ian Jackson
  2011-01-28 19:14           ` Ian Campbell
@ 2011-02-11 13:41           ` Jan Beulich
  2011-02-11 13:53             ` Ian Campbell
  1 sibling, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2011-02-11 13:41 UTC (permalink / raw)
  To: Christoph Egger, Ian Campbell, Ian Jackson; +Cc: xen-devel

>>> On 28.01.11 at 19:51, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> libxc: Do not use host physmem as parameter to lzma decoder
> 
> It's not clear why a userspace lzma decode would want to use that
> particular value, what bearing it has on anything or why it would
> assume it could use 1/3 of the total RAM in the system (potentially
> quite a large amount of RAM) as opposed to any other limit number.
> 
> Instead, hardcode 32Mby.

Has anyone of you actually tested this? I don't get lzma to work
with this set to less than 128Mb, and xz's configure.ac also sort
of states this to be a minimal requirement for arbitrary input.

> This reverts 22830:c80960244942, removes the xc_get_physmem/physmem
> function entirely, and replaces the expression at the call site with a
> fixed constant.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
> Cc: Christoph Egger <Christoph.Egger@amd.com>

Jan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-02-11 13:41           ` Jan Beulich
@ 2011-02-11 13:53             ` Ian Campbell
  2011-02-11 17:49               ` Ian Jackson
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2011-02-11 13:53 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Christoph Egger, xen-devel, Ian Jackson

On Fri, 2011-02-11 at 13:41 +0000, Jan Beulich wrote:
> >>> On 28.01.11 at 19:51, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> > libxc: Do not use host physmem as parameter to lzma decoder
> > 
> > It's not clear why a userspace lzma decode would want to use that
> > particular value, what bearing it has on anything or why it would
> > assume it could use 1/3 of the total RAM in the system (potentially
> > quite a large amount of RAM) as opposed to any other limit number.
> > 
> > Instead, hardcode 32Mby.
> 
> Has anyone of you actually tested this? I don't get lzma to work
> with this set to less than 128Mb, and xz's configure.ac also sort
> of states this to be a minimal requirement for arbitrary input.

I looked quite hard for any documentation on this and couldn't find any,
I could only find examples of people using host physmem like we were or
hardcoding ~30MB.

I should have looked in configure.ac, its obvious really ;-)

Putting aside the fact that 128MB is a quite obscene amount of memory as
a _minimum_ requirement for something like this I guess we should change
the constant.

Ian.

> 
> > This reverts 22830:c80960244942, removes the xc_get_physmem/physmem
> > function entirely, and replaces the expression at the call site with a
> > fixed constant.
> > 
> > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
> > Cc: Christoph Egger <Christoph.Egger@amd.com>
> 
> Jan
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] libxc: lzma build fix
  2011-02-11 13:53             ` Ian Campbell
@ 2011-02-11 17:49               ` Ian Jackson
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2011-02-11 17:49 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Christoph Egger, xen-devel, Jan Beulich

Ian Campbell writes ("Re: [Xen-devel] [PATCH] libxc: lzma build fix"):
> Putting aside the fact that 128MB is a quite obscene amount of memory as
> a _minimum_ requirement for something like this I guess we should change
> the constant.

I have done so.

Ian.

# HG changeset patch
# User Ian Jackson <Ian.Jackson@eu.citrix.com>
# Date 1297446553 0
# Node ID 4376c4f0196f0dc129904a0f8c54c74ae4bcdff2
# Parent  6c22ae0f654084a4d2811e51e22b7f8b20b49cec
libxc: increase lzma max memory constant to 128Mby

According to lzma's configure.ac (!) the minimum memory limit to cope
with arbitrary input is 128Mby (!)

This is obviously an unreasonable amount of memory for this kind of
task, but we need to increase the constant limit for it not to
randomly fail.  So do so.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r 6c22ae0f6540 -r 4376c4f0196f tools/libxc/xc_dom_bzimageloader.c
--- a/tools/libxc/xc_dom_bzimageloader.c	Fri Feb 11 16:51:44 2011 +0000
+++ b/tools/libxc/xc_dom_bzimageloader.c	Fri Feb 11 17:49:13 2011 +0000
@@ -152,7 +152,7 @@ static int xc_try_lzma_decode(
     int outsize;
     const char *msg;
 
-    ret = lzma_alone_decoder(&stream, 32*1024*1024);
+    ret = lzma_alone_decoder(&stream, 128*1024*1024);
     if ( ret != LZMA_OK )
     {
         DOMPRINTF("LZMA: Failed to init stream decoder");

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-02-11 17:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-26 15:22 [PATCH] libxc: lzma build fix Christoph Egger
2011-01-26 16:16 ` Ian Jackson
2011-01-26 17:27   ` Christoph Egger
2011-01-27 19:04     ` Ian Jackson
2011-01-27 19:54     ` Ian Campbell
2011-01-28 11:45       ` Ian Jackson
2011-01-28 18:51         ` Ian Jackson
2011-01-28 19:14           ` Ian Campbell
2011-01-28 19:38             ` Ian Jackson
2011-02-11 13:41           ` Jan Beulich
2011-02-11 13:53             ` Ian Campbell
2011-02-11 17:49               ` Ian Jackson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.