* [PATCH V2 0/1] libxl: set stub domain size based on VRAM size
@ 2015-07-10 22:14 Eric Shelton
2015-07-10 22:14 ` [PATCH V2 1/1] " Eric Shelton
0 siblings, 1 reply; 8+ messages in thread
From: Eric Shelton @ 2015-07-10 22:14 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, ian.jackson,
samuel.thibault, Eric Shelton
I ran into crashes with qemu-traditional stub domain when 16 MB was
assigned to the stdvga virtual video adapter. These were occurring due
to an out of memory condition arising from stub domains having a fixed
size of 32 MB, with half of that being demanded for video. Assuming the
the original value of 32 MB was based on having a 4 MB video adapter,
this increases the stub domain size by however much the VRAM exceeds 4 MB.
Revised per Ian Jackson's suggestion.
Eric Shelton (1):
libxl: set stub domain size based on VRAM size
tools/libxl/libxl_dm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
2.1.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V2 1/1] libxl: set stub domain size based on VRAM size
2015-07-10 22:14 [PATCH V2 0/1] libxl: set stub domain size based on VRAM size Eric Shelton
@ 2015-07-10 22:14 ` Eric Shelton
2015-07-10 22:27 ` Samuel Thibault
2015-07-13 10:16 ` Ian Jackson
0 siblings, 2 replies; 8+ messages in thread
From: Eric Shelton @ 2015-07-10 22:14 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, ian.jackson,
samuel.thibault, Eric Shelton
Allocate additional memory to the stub domain for qemu-traditional if
more than 4 MB is assigned to the video adapter to avoid out of memory
condition for QEMU.
Signed-off-by: Eric Shelton <eshelton@pobox.com>
---
tools/libxl/libxl_dm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 317a8eb..9a5a937 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1087,6 +1087,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
dm_config->b_info.max_vcpus = 1;
- dm_config->b_info.max_memkb = 32 * 1024;
+ dm_config->b_info.max_memkb = 28 * 1024 +
+ max(guest_config->b_info.video_memkb, 4096);
dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
dm_config->b_info.u.pv.features = "";
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size
2015-07-10 22:14 ` [PATCH V2 1/1] " Eric Shelton
@ 2015-07-10 22:27 ` Samuel Thibault
2015-07-13 10:16 ` Ian Jackson
1 sibling, 0 replies; 8+ messages in thread
From: Samuel Thibault @ 2015-07-10 22:27 UTC (permalink / raw)
To: Eric Shelton
Cc: xen-devel, wei.liu2, ian.jackson, ian.campbell, stefano.stabellini
Eric Shelton, le Fri 10 Jul 2015 18:14:32 -0400, a écrit :
> - dm_config->b_info.max_memkb = 32 * 1024;
> + dm_config->b_info.max_memkb = 28 * 1024 +
> + max(guest_config->b_info.video_memkb, 4096);
I'm actually wondering whether just using
> + dm_config->b_info.max_memkb = 28 * 1024 +
> + guest_config->b_info.video_memkb;
and I don't see why not.
Samuel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size
2015-07-10 22:14 ` [PATCH V2 1/1] " Eric Shelton
2015-07-10 22:27 ` Samuel Thibault
@ 2015-07-13 10:16 ` Ian Jackson
2015-07-13 10:22 ` Ian Jackson
1 sibling, 1 reply; 8+ messages in thread
From: Ian Jackson @ 2015-07-13 10:16 UTC (permalink / raw)
To: Eric Shelton
Cc: xen-devel, ian.campbell, wei.liu2, samuel.thibault, stefano.stabellini
Eric Shelton writes ("[PATCH V2 1/1] libxl: set stub domain size based on VRAM size"):
> Allocate additional memory to the stub domain for qemu-traditional if
> more than 4 MB is assigned to the video adapter to avoid out of memory
> condition for QEMU.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
This is IMO a bugfix so I am queueing it for 4.6.
Ian.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size
2015-07-13 10:16 ` Ian Jackson
@ 2015-07-13 10:22 ` Ian Jackson
2015-07-13 10:24 ` [PATCH for-4.6 v3] " Ian Jackson
2015-07-13 10:37 ` [PATCH V2 1/1] " Ian Campbell
0 siblings, 2 replies; 8+ messages in thread
From: Ian Jackson @ 2015-07-13 10:22 UTC (permalink / raw)
To: Eric Shelton, xen-devel, stefano.stabellini, samuel.thibault,
ian.campbell, wei.liu2
Ian Jackson writes ("Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size"):
> Eric Shelton writes ("[PATCH V2 1/1] libxl: set stub domain size based on VRAM size"):
> > Allocate additional memory to the stub domain for qemu-traditional if
> > more than 4 MB is assigned to the video adapter to avoid out of memory
> > condition for QEMU.
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> This is IMO a bugfix so I am queueing it for 4.6.
My build test failed. It turns out that max() is no good because the
types of `4096' and `guest_config->b_info.video_memkb' are not the
same.
In a moment I am going to send a v3 which uses max_t and uint64_t
(which is the type of the memkb fields and also obviously correct).
Ian.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH for-4.6 v3] libxl: set stub domain size based on VRAM size
2015-07-13 10:22 ` Ian Jackson
@ 2015-07-13 10:24 ` Ian Jackson
2015-07-13 10:37 ` [PATCH V2 1/1] " Ian Campbell
1 sibling, 0 replies; 8+ messages in thread
From: Ian Jackson @ 2015-07-13 10:24 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, ian.campbell, stefano.stabellini, Ian Jackson,
samuel.thibault, Eric Shelton
From: Eric Shelton <eshelton@pobox.com>
Allocate additional memory to the stub domain for qemu-traditional if
more than 4 MB is assigned to the video adapter to avoid out of memory
condition for QEMU.
Signed-off-by: Eric Shelton <eshelton@pobox.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v3: Use max_t() instead
v2: Use max()
---
tools/libxl/libxl_dm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index ad434f0..f700f9a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1095,7 +1095,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
libxl_domain_build_info_init_type(&dm_config->b_info, LIBXL_DOMAIN_TYPE_PV);
dm_config->b_info.max_vcpus = 1;
- dm_config->b_info.max_memkb = 32 * 1024;
+ dm_config->b_info.max_memkb = 28 * 1024 +
+ max_t(uint64_t, guest_config->b_info.video_memkb, 4096);
dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
dm_config->b_info.u.pv.features = "";
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size
2015-07-13 10:22 ` Ian Jackson
2015-07-13 10:24 ` [PATCH for-4.6 v3] " Ian Jackson
@ 2015-07-13 10:37 ` Ian Campbell
2015-07-13 11:14 ` Ian Jackson
1 sibling, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2015-07-13 10:37 UTC (permalink / raw)
To: Ian Jackson
Cc: xen-devel, stefano.stabellini, wei.liu2, samuel.thibault, Eric Shelton
On Mon, 2015-07-13 at 11:22 +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size"):
> > Eric Shelton writes ("[PATCH V2 1/1] libxl: set stub domain size based on VRAM size"):
> > > Allocate additional memory to the stub domain for qemu-traditional if
> > > more than 4 MB is assigned to the video adapter to avoid out of memory
> > > condition for QEMU.
> >
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> >
> > This is IMO a bugfix so I am queueing it for 4.6.
>
> My build test failed. It turns out that max() is no good because the
> types of `4096' and `guest_config->b_info.video_memkb' are not the
> same.
>
> In a moment I am going to send a v3 which uses max_t and uint64_t
> (which is the type of the memkb fields and also obviously correct).
Eric already sent a v3 in
<1436650242-1067-2-git-send-email-eshelton@pobox.com> which avoids the
use of max in a different way.
I think his approach looked fine.
Ian.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size
2015-07-13 10:37 ` [PATCH V2 1/1] " Ian Campbell
@ 2015-07-13 11:14 ` Ian Jackson
0 siblings, 0 replies; 8+ messages in thread
From: Ian Jackson @ 2015-07-13 11:14 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel, stefano.stabellini, wei.liu2, samuel.thibault, Eric Shelton
Ian Campbell writes ("Re: [PATCH V2 1/1] libxl: set stub domain size based on VRAM size"):
> On Mon, 2015-07-13 at 11:22 +0100, Ian Jackson wrote:
> > In a moment I am going to send a v3 which uses max_t and uint64_t
> > (which is the type of the memkb fields and also obviously correct).
>
> Eric already sent a v3 in
> <1436650242-1067-2-git-send-email-eshelton@pobox.com> which avoids the
> use of max in a different way.
>
> I think his approach looked fine.
Well, it does also reduce the stubdom memory when the video memory is
<4Mby. That sounds plausible to me but at the very least the commit
message needs changing, and maybe we should wait a bit to see if one
of the qemu experts objects.
Ian.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-07-13 11:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-10 22:14 [PATCH V2 0/1] libxl: set stub domain size based on VRAM size Eric Shelton
2015-07-10 22:14 ` [PATCH V2 1/1] " Eric Shelton
2015-07-10 22:27 ` Samuel Thibault
2015-07-13 10:16 ` Ian Jackson
2015-07-13 10:22 ` Ian Jackson
2015-07-13 10:24 ` [PATCH for-4.6 v3] " Ian Jackson
2015-07-13 10:37 ` [PATCH V2 1/1] " Ian Campbell
2015-07-13 11:14 ` 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.