On 02/07/2018 10:07, Chris Wilson wrote:
sysinfo() doesn't include all reclaimable memory. In particular it
excludes the majority of global_node_page_state(NR_FILE_PAGES),
reclaimable pages that are a copy of on-disk files It seems the only way
to obtain this counter is by parsing /proc/meminfo. For comparison,
check vm_enough_memory() which includes NR_FILE_PAGES as available
(sadly there's no way to call vm_enough_memory() directly either!)

v2: Pay attention to what one writes.

Signed-off-by: Chris Wilson <[email protected]>
---
  lib/intel_os.c | 36 +++++++++++++++++++++++++++++++-----
  1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/lib/intel_os.c b/lib/intel_os.c
index 885ffdcec..5bd18fc52 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -84,6 +84,18 @@ intel_get_total_ram_mb(void)
        return retval / (1024*1024);
  }
+static uint64_t get_meminfo(const char *info, const char *tag)
+{
+       const char *str;
+       unsigned long val;
+
+       str = strstr(info, tag);
+       if (str && sscanf(str + strlen(tag), " %lu", &val) == 1)
+               return (uint64_t)val << 10;
+
+       return 0;

Might be safer to assert format was as expected and keywords we expect are there, rather than silently return zero.

+}
+
  /**
   * intel_get_avail_ram_mb:
   *
@@ -96,17 +108,31 @@ intel_get_avail_ram_mb(void)
        uint64_t retval;
#ifdef HAVE_STRUCT_SYSINFO_TOTALRAM /* Linux */
-       struct sysinfo sysinf;
+       char *info;
        int fd;
fd = drm_open_driver(DRIVER_INTEL);
        intel_purge_vm_caches(fd);
        close(fd);
- igt_assert(sysinfo(&sysinf) == 0);
-       retval = sysinf.freeram;
-       retval += min(sysinf.freeswap, sysinf.bufferram);
-       retval *= sysinf.mem_unit;
+       fd = open("/proc", O_RDONLY);
+       info = igt_sysfs_get(fd, "meminfo");
+       close(fd);
+
+       if (info) {
+               retval  = get_meminfo(info, "MemAvailable: ");
+               retval += get_meminfo(info, "Buffers: ");
+               retval += get_meminfo(info, "Cached: ");
+               retval += get_meminfo(info, "SwapCached: ");

I think it would be more robust to have no trailing space in tag strings.

+               free(info);
+       } else {
+               struct sysinfo sysinf;
+
+               igt_assert(sysinfo(&sysinf) == 0);
+               retval = sysinf.freeram;
+               retval += min(sysinf.freeswap, sysinf.bufferram);
+               retval *= sysinf.mem_unit;
+       }

Not sure it is worth keeping this path - will we ever not have /proc/meminfo?

  #elif defined(_SC_PAGESIZE) && defined(_SC_AVPHYS_PAGES) /* Solaris */
        long pagesize, npages;

Google agrees with you that sysinfo indeed has this limitation. So in general no complaints.

One tiny detail might be that this would now return a too large value - doesn't count that it should not swap out itself when thinking about free memory. But I don't think that is for this level of API to concern with - it is definitely way more correct to report page cache.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to