Re: [PATCH] fix uint64 underflow
On 9/18/23 02:22, Дмитрий Фролов wrote:
Hi, Michal
Of course, this changes the semantics. To my opinion, it is obvious from
the source code, that we need some additional memory. I really doubt, that
we intended to allocate some Exabytes additionally (using dirty underflow
hack by the way).
libxlSetMemoryTargetWrapper would have at least prevented allocating > LLONG_MAX
:-).
I took a stab at fixing it, along with general brokeness of the dom0 autoballoon
logic
https://listman.redhat.com/archives/libvir-list/2023-September/242160.html
It's compile-tested only ATM. I'll check the functionality later today after
repurposing a test system.
Regards,
Jim
Dmitry
18.09.2023 10:59, Michal Prívozník пишет:
On 9/12/23 16:50, Dmitry Frolov wrote:
According to previous statement,
'free_mem' is less than 'needed_mem'.
So, the subtraction 'free_mem - needed_mem' is negative,
and will raise uint64 underflow.
Signed-off-by: Dmitry Frolov
---
src/libxl/libxl_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 6c167df63e..36be042971 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -940,7 +940,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config
*d_config)
if (free_mem >= needed_mem)
return 0;
- target_mem = free_mem - needed_mem;
+ target_mem = needed_mem - free_mem;
if (libxlSetMemoryTargetWrapper(ctx, 0, target_mem,
/* relative */ 1, 0) < 0)
goto error;
Yeah, this fixes the underflow, but I worry about the while semantics a
bit. What the code effectively does:
libxl_domain_need_memory(&needed_mem);
do {
libxl_get_free_memory(&free_mem);
if (free_mem >= needed_mem)
return 0;
target_mem = needed_mem - free_mem;
libxl_set_memory_target(target_mem);
} while(...)
Now, if libxl_domain_need_memory() returns how much memory a domain
really needs, then why undergo trouble of getting its free memory? Why
not pass it to set_memory_target() right away?
Or am I misunderstanding something?
Michal
Re: [PATCH] fix uint64 underflow
Hi, Michal
Of course, this changes the semantics. To my opinion, it is obvious from
the source code, that we need some additional memory. I really doubt, that
we intended to allocate some Exabytes additionally (using dirty underflow
hack by the way).
Dmitry
18.09.2023 10:59, Michal Prívozník пишет:
On 9/12/23 16:50, Dmitry Frolov wrote:
According to previous statement,
'free_mem' is less than 'needed_mem'.
So, the subtraction 'free_mem - needed_mem' is negative,
and will raise uint64 underflow.
Signed-off-by: Dmitry Frolov
---
src/libxl/libxl_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 6c167df63e..36be042971 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -940,7 +940,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config
*d_config)
if (free_mem >= needed_mem)
return 0;
-target_mem = free_mem - needed_mem;
+target_mem = needed_mem - free_mem;
if (libxlSetMemoryTargetWrapper(ctx, 0, target_mem,
/* relative */ 1, 0) < 0)
goto error;
Yeah, this fixes the underflow, but I worry about the while semantics a
bit. What the code effectively does:
libxl_domain_need_memory(&needed_mem);
do {
libxl_get_free_memory(&free_mem);
if (free_mem >= needed_mem)
return 0;
target_mem = needed_mem - free_mem;
libxl_set_memory_target(target_mem);
} while(...)
Now, if libxl_domain_need_memory() returns how much memory a domain
really needs, then why undergo trouble of getting its free memory? Why
not pass it to set_memory_target() right away?
Or am I misunderstanding something?
Michal
Re: [PATCH] fix uint64 underflow
On 9/12/23 16:50, Dmitry Frolov wrote:
> According to previous statement,
> 'free_mem' is less than 'needed_mem'.
> So, the subtraction 'free_mem - needed_mem' is negative,
> and will raise uint64 underflow.
>
> Signed-off-by: Dmitry Frolov
> ---
> src/libxl/libxl_domain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 6c167df63e..36be042971 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -940,7 +940,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config
> *d_config)
> if (free_mem >= needed_mem)
> return 0;
>
> -target_mem = free_mem - needed_mem;
> +target_mem = needed_mem - free_mem;
> if (libxlSetMemoryTargetWrapper(ctx, 0, target_mem,
> /* relative */ 1, 0) < 0)
> goto error;
Yeah, this fixes the underflow, but I worry about the while semantics a
bit. What the code effectively does:
libxl_domain_need_memory(&needed_mem);
do {
libxl_get_free_memory(&free_mem);
if (free_mem >= needed_mem)
return 0;
target_mem = needed_mem - free_mem;
libxl_set_memory_target(target_mem);
} while(...)
Now, if libxl_domain_need_memory() returns how much memory a domain
really needs, then why undergo trouble of getting its free memory? Why
not pass it to set_memory_target() right away?
Or am I misunderstanding something?
Michal
