On Sun, Apr 06, 2025 at 08:38:36PM +0100, Ramsay Jones wrote:
> The man page for sysinfo(2) on Linux states that (from v2.3.48) the
> sizes of the memory and swap fields, of the returned structure, are
> given as multiples of 'mem_unit' bytes. In earlier versions (prior to
> v2.3.23 on i386 in particular), the 'mem_unit' field was not part of
> the structure, and all sizes were measured in bytes. The man page does
> not discuss the motivation for this change, but it is possible that the
> change was intended for the, relatively rare, 32-bit platform with more
> than 4GB of memory.
>
> The total_ram() function makes the assumption that the 'totalram' field
> of the 'struct sysinfo' is measured in bytes, or alternatively that the
> 'mem_unit' field is always equal to one. Having writen a program to call
> the sysinfo() function and print the structure fields, it seems that, on
> Linux x84_64 and i686 anyway, the 'mem_unit' field is indeed set to one
> (note that the 32-bit system had only 2GB ram). However, cygwin also has
> an sysinfo() implementation, which gives the following values:
>
> $ ./sysinfo
> uptime: 21381
> loads: 0, 0, 0
> total ram: 2074637
> free ram: 843237
> shared ram: 0
> buffer ram: 0
> total swap: 327680
> free swap: 306932
> procs: 15
> total high: 0
> free high: 0
> mem_unit: 4096
>
> total ram: 8497713152
> $
>
> [This laptop has 8GB ram, so a little bit seems to be missing. ;) ]
Interesting. I can confirm that `mem_unit` is 1 on my system, so this
does not make a difference here. But my tests on Cygwin show the same
behaviour as on your system, so the patch looks reasonable to me.
> Modify the total_ram() function to allow for the possibility that the
> memory size is not specified in bytes (ie 'mem_unit' is greater than
> one).
>
> Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
> ---
> builtin/gc.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 99431fd467..cdcf1dc6e7 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -373,8 +373,13 @@ static uint64_t total_ram(void)
> #if defined(HAVE_SYSINFO)
> struct sysinfo si;
>
> - if (!sysinfo(&si))
> - return si.totalram;
> + if (!sysinfo(&si)) {
> + uint64_t total = si.totalram;
> +
> + if (si.mem_unit > 1)
> + total *= (uint64_t)si.mem_unit;
> + return total;
> + }
I expect that all systems have a proper value for `si.mem_unit` set so
that we could unconditionally multiplicate the fields with one another.
But it doesn't hurt either, so I don't mind the guarding clause.
Patrick
