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 <frolov(a)swemel.ru>
> ---
> 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