All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxl_set_memory_target: only remove videoram from absolute targets
@ 2015-01-26 16:47 Stefano Stabellini
  2015-01-28 13:22 ` Ian Campbell
  0 siblings, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2015-01-26 16:47 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian.Jackson, wei.liu2, Ian.Campbell, dslutz, stefano.stabellini

If the new target is relative to the current target, do not remove
videoram again: it has already been removed from the current target.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

Changes in v2:
- add videoram when setting maxmem.
---
 tools/libxl/libxl.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 82227e8..cd6f42c 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4764,13 +4764,17 @@ retry_transaction:
         goto out;
     }
 
+    videoram_s = libxl__xs_read(gc, t, libxl__sprintf(gc,
+                "%s/memory/videoram", dompath));
+    videoram = videoram_s ? atoi(videoram_s) : 0;
+
     if (relative) {
         if (target_memkb < 0 && abs(target_memkb) > current_target_memkb)
             new_target_memkb = 0;
         else
             new_target_memkb = current_target_memkb + target_memkb;
     } else
-        new_target_memkb = target_memkb;
+        new_target_memkb = target_memkb - videoram;
     if (new_target_memkb > memorykb) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                 "memory_dynamic_max must be less than or equal to"
@@ -4786,12 +4790,9 @@ retry_transaction:
         abort_transaction = 1;
         goto out;
     }
-    videoram_s = libxl__xs_read(gc, t, libxl__sprintf(gc,
-                "%s/memory/videoram", dompath));
-    videoram = videoram_s ? atoi(videoram_s) : 0;
 
     if (enforce) {
-        memorykb = new_target_memkb;
+        memorykb = new_target_memkb + videoram;
         rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
                 LIBXL_MAXMEM_CONSTANT);
         if (rc != 0) {
@@ -4803,7 +4804,6 @@ retry_transaction:
         }
     }
 
-    new_target_memkb -= videoram;
     rc = xc_domain_set_pod_target(ctx->xch, domid,
             new_target_memkb / 4, NULL, NULL, NULL);
     if (rc != 0) {
-- 
1.7.10.4

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

* Re: [PATCH] libxl_set_memory_target: only remove videoram from absolute targets
  2015-01-26 16:47 [PATCH] libxl_set_memory_target: only remove videoram from absolute targets Stefano Stabellini
@ 2015-01-28 13:22 ` Ian Campbell
  2015-01-28 14:24   ` Stefano Stabellini
  0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2015-01-28 13:22 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Ian.Jackson, xen-devel, wei.liu2, dslutz

On Mon, 2015-01-26 at 16:47 +0000, Stefano Stabellini wrote:
> If the new target is relative to the current target, do not remove
> videoram again: it has already been removed from the current target.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

How does this relate to "libxl_set_memory_target: retain the same maxmem
offset on top of the current target"? Doesn't that also achieve the same
aim, since maxmem accounts for vram but target doesn't?

Ian.

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

* Re: [PATCH] libxl_set_memory_target: only remove videoram from absolute targets
  2015-01-28 13:22 ` Ian Campbell
@ 2015-01-28 14:24   ` Stefano Stabellini
  2015-01-28 14:44     ` Ian Campbell
  0 siblings, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2015-01-28 14:24 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Ian.Jackson, xen-devel, wei.liu2, dslutz, Stefano Stabellini

On Wed, 28 Jan 2015, Ian Campbell wrote:
> On Mon, 2015-01-26 at 16:47 +0000, Stefano Stabellini wrote:
> > If the new target is relative to the current target, do not remove
> > videoram again: it has already been removed from the current target.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> How does this relate to "libxl_set_memory_target: retain the same maxmem
> offset on top of the current target"? Doesn't that also achieve the same
> aim, since maxmem accounts for vram but target doesn't?

The two patches are different: this one affects the memory target while
the other one affects maxmem. The only relationship is that this issue
was found out by looking at the other patch.

This patch didn't make it into 4.5 because although it is a bug fix, it
doesn't fix a regression compared to 4.4.

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

* Re: [PATCH] libxl_set_memory_target: only remove videoram from absolute targets
  2015-01-28 14:24   ` Stefano Stabellini
@ 2015-01-28 14:44     ` Ian Campbell
  2015-01-28 14:47       ` Stefano Stabellini
  0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2015-01-28 14:44 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Ian.Jackson, xen-devel, wei.liu2, dslutz

On Wed, 2015-01-28 at 14:24 +0000, Stefano Stabellini wrote:
> On Wed, 28 Jan 2015, Ian Campbell wrote:
> > On Mon, 2015-01-26 at 16:47 +0000, Stefano Stabellini wrote:
> > > If the new target is relative to the current target, do not remove
> > > videoram again: it has already been removed from the current target.
> > > 
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > How does this relate to "libxl_set_memory_target: retain the same maxmem
> > offset on top of the current target"? Doesn't that also achieve the same
> > aim, since maxmem accounts for vram but target doesn't?
> 
> The two patches are different: this one affects the memory target while
> the other one affects maxmem. The only relationship is that this issue
> was found out by looking at the other patch.

Not even a textual relationship? (i.e. one needs to go first to avoid
conflicts)

> This patch didn't make it into 4.5 because although it is a bug fix, it
> doesn't fix a regression compared to 4.4.

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

* Re: [PATCH] libxl_set_memory_target: only remove videoram from absolute targets
  2015-01-28 14:44     ` Ian Campbell
@ 2015-01-28 14:47       ` Stefano Stabellini
  2015-02-02 15:25         ` Ian Campbell
  0 siblings, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2015-01-28 14:47 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Ian.Jackson, xen-devel, wei.liu2, dslutz, Stefano Stabellini

On Wed, 28 Jan 2015, Ian Campbell wrote:
> On Wed, 2015-01-28 at 14:24 +0000, Stefano Stabellini wrote:
> > On Wed, 28 Jan 2015, Ian Campbell wrote:
> > > On Mon, 2015-01-26 at 16:47 +0000, Stefano Stabellini wrote:
> > > > If the new target is relative to the current target, do not remove
> > > > videoram again: it has already been removed from the current target.
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > How does this relate to "libxl_set_memory_target: retain the same maxmem
> > > offset on top of the current target"? Doesn't that also achieve the same
> > > aim, since maxmem accounts for vram but target doesn't?
> > 
> > The two patches are different: this one affects the memory target while
> > the other one affects maxmem. The only relationship is that this issue
> > was found out by looking at the other patch.
> 
> Not even a textual relationship? (i.e. one needs to go first to avoid
> conflicts)

Yep, there is a textual relationship. Sorry I failed to mention it.

This one needs to go first.


> > This patch didn't make it into 4.5 because although it is a bug fix, it
> > doesn't fix a regression compared to 4.4.
> 
> 

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

* Re: [PATCH] libxl_set_memory_target: only remove videoram from absolute targets
  2015-01-28 14:47       ` Stefano Stabellini
@ 2015-02-02 15:25         ` Ian Campbell
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2015-02-02 15:25 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Ian.Jackson, xen-devel, wei.liu2, dslutz

On Wed, 2015-01-28 at 14:47 +0000, Stefano Stabellini wrote:
> On Wed, 28 Jan 2015, Ian Campbell wrote:
> > On Wed, 2015-01-28 at 14:24 +0000, Stefano Stabellini wrote:
> > > On Wed, 28 Jan 2015, Ian Campbell wrote:
> > > > On Mon, 2015-01-26 at 16:47 +0000, Stefano Stabellini wrote:
> > > > > If the new target is relative to the current target, do not remove
> > > > > videoram again: it has already been removed from the current target.
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > 
> > > > How does this relate to "libxl_set_memory_target: retain the same maxmem
> > > > offset on top of the current target"? Doesn't that also achieve the same
> > > > aim, since maxmem accounts for vram but target doesn't?
> > > 
> > > The two patches are different: this one affects the memory target while
> > > the other one affects maxmem. The only relationship is that this issue
> > > was found out by looking at the other patch.
> > 
> > Not even a textual relationship? (i.e. one needs to go first to avoid
> > conflicts)
> 
> Yep, there is a textual relationship. Sorry I failed to mention it.
> 
> This one needs to go first.

Acked + applied then, thanks.

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

end of thread, other threads:[~2015-02-02 15:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-26 16:47 [PATCH] libxl_set_memory_target: only remove videoram from absolute targets Stefano Stabellini
2015-01-28 13:22 ` Ian Campbell
2015-01-28 14:24   ` Stefano Stabellini
2015-01-28 14:44     ` Ian Campbell
2015-01-28 14:47       ` Stefano Stabellini
2015-02-02 15:25         ` Ian Campbell

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.