lib: add tst_read_meminfo / tst_get_avail_mem

This patch adds 2 new functions that both parse data from /proc/meminfo.

---
 include/tst_mem.h | 36 ++++++++++++++++++++++++++++++++
 lib/tst_mem.c     | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)
 create mode 100644 include/tst_mem.h
 create mode 100644 lib/tst_mem.c

Julio Cruz reported couple issues when running LTP on systems with limited RAM. To avoid duplicating get_avail_mem() in all of them, I propose we add such function to lib.

new file mode 100644
index 000000000000..a37aaa088103
@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2016 Linux Test Project
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef TST_MEM_H__
+#define TST_MEM_H__
+
+/*
+ * Returns number of kB for /proc/meminfo "item" or -1
+ * if "item" can not be found.
+ *
+ * @item: name of /proc/meminfo item, e.g. MemFree
+ * @strict: if true and "item" can not be found,
+ *          function calls tst_brk(TBROK,..)
+ */
+long tst_read_meminfo(const char *item, int strict);
+
+/*
+ * Returns number of kB of available memory.
+ */
+long tst_get_avail_mem(void);
+
+#endif /* TST_MEM_H__ */

1.8.3.1

Hi Jan,

It is a good proposal, but the achieved function seems a little bit narrow for ltp-lib. Can we
rename the tst_mem.h to tst_proc.h, and makes the whole items of procfs could be readable via the same interfaces? And the strict parameter is not necessary for the function i think.

How about something like this?

-----------------------------------------
long read_proc(char *path, char *item)

{
    FILE *fp;
    char line[BUFSIZ], buf[BUFSIZ];
    long val;

    fp = fopen(path, "r");
    if (fp == NULL)
        tst_brkm(TBROK | TERRNO, cleanup, "fopen %s", path);

    while (fgets(line, BUFSIZ, fp) != NULL) {
        if (sscanf(line, "%64s %ld", buf, &val) == 2)
            if (strcmp(buf, item) == 0) {
                fclose(fp);
                return val;
            }
        continue;
    }
    fclose(fp);

    tst_res(TINFO, "cannot find \"%s\" in %s", item, path);

    return -1;
}

long tst_read_meminfo(char *item)
{

    return read_proc("/proc/meminfo", item);
}

long tst_read_self_status(char *item)

{
    return read_proc("/proc/self/status", item);
}

// we can extend this interface easily by uniform prototype long tst_read_blablabla(char *item)

{
    return read_proc("/proc/blablabla...", item);
}

...

long tst_get_avail_mem(void)

{
       long mem_available;

       mem_available = tst_read_meminfo("memavailable:");
       if (mem_available == -1) {
               mem_available = tst_read_meminfo("memfree:");
               mem_available += tst_read_meminfo("cached:");
       }

       return mem_available;
}

Regards,
Li Wang

----- Original Message -----

OK, let me try to come up with something more generic.

They don't all share same format, so there have to be some limitations.

We could leave that up to caller.

Regards,
Jan

How about something like this?

#include <stdio.h>
#include "tst_proc.h"

#define TST_NO_DEFAULT_MAIN
#include "tst_test.h"

static const char *path_meminfo = "/proc/meminfo";

typedef int walk_func(const char *line, void *priv);

/*
* Go through all lines in a text file, calling walk_func() on each
* until it returns 1 or file hits EOF. Returns last return value of
* walk_func().
*
* @filename: name of text file to walk
* @walk_func: function called for each line
* @func_priv: walk_func's specific struct representing in/out data
* */
static int walk_file_lines(const char *filename, walk_func func,
void *func_priv)

{
	FILE *fp;
	char line[BUFSIZ];

	fp = fopen(filename, "r");
	if (fp == NULL)
		tst_brk(TBROK | TERRNO, "fopen %s", path_meminfo);

	while (fgets(line, BUFSIZ, fp) != NULL) {
		if (func(line, func_priv) == 1) {
			fclose(fp);
			return 1;
		}
		continue;
	}
	fclose(fp);

	return 0;
}

struct colonpair_str_long

{
	const char *key;
	long value;
};

/*
* Returns 1 if "line" matches pattern: "Key: Value" and
* Key is equal to colonpair_str_long->key, otherwise returns 0. *
* @line: line searched for pattern
* @priv: pointer to colonpair_str_long struct where
* .key - Key to search for
* .value - stores "Value" if "Key" is found
*/
static int match_colonpair_str_long(const char *line, void *priv)

{
	struct colonpair_str_long *s = priv;
	char buf[BUFSIZ];
	long val;

	if (sscanf(line, "%64s %ld", buf, &val) == 2) {
		/* strip colon for comparison with key */
		buf[strlen(buf) - 1] = '\0';

		if (strcmp(buf, s->key) == 0) {
			s->value = val;
			return 1;
		}
	}

	return 0;
}

static long read_colonpair_str_long(const char *filename, const char *key,
long ret_for_missing)

{
	struct colonpair_str_long data = {
		.key = key,
	};

	if (walk_file_lines(filename, match_colonpair_str_long, &data))
		return data.value;

	return ret_for_missing;
}

/*
* Search for Key: Value pair in /proc/meminfo, if found
* return Value, otherwise return -1.
*/
long tst_get_meminfo(const char *key)

{
	return read_colonpair_str_long(path_meminfo, key, -1);
}

long tst_get_avail_mem(void)

{
	long mem_available;

	mem_available = tst_get_meminfo("MemAvailable");
	if (mem_available == -1) {
		mem_available = tst_get_meminfo("MemFree");
		mem_available += tst_get_meminfo("Cached");
	}

	return mem_available;
}

This one looks better, just have a little comments as below:

A typo here, the 'path_meminfo' should be replaced by 'filename'.

If manual set the last character as '\0', there will be limit this match function area.

e.g.

  # cat /proc/stat | grep processes
      processes 129493

A caller can not get correct value of the processes, since the string saved in buf[] as: 'p' 'r' 'o' 'c' 'e' 's' 's' 'e' '\0', it will disturb the mathing result and return 0.

My proposal is to have a simple choice here:

/* strip colon for comparison with key */

	if (buf[strlen(buf) -1] == ':')
		buf[strlen(buf) -1] = '\0';

I'd like to add new callers for reading '/proc/stat' and '/proc/self/status' to verify read_colonpair_str_long() function.

static const char *path_proc_stat = "/proc/stat"; static const char *path_self_status = "/proc/self/status";

long tst_get_stat(const char *key)

{
	return read_colonpair_str_long(path_proc_stat, key, -1);
}

long tst_get_self_status(const char *key)

{
	return read_colonpair_str_long(path_self_status, key, -1);
}

a simple test program:
---------------------

# cat newlib_tests/test14.c

#include "tst_test.h"
#include "tst_proc.h"

static void do_test(void)

{
	tst_res(TINFO, "MemTotal = %ld kB", tst_get_meminfo("MemTotal"));
	tst_res(TINFO, "MemAvailable = %ld kB", tst_get_meminfo("MemAvailable"));
	tst_res(TINFO, "MemAvailable = %ld kB", tst_get_avail_mem());
	tst_res(TINFO, "Shmem = %ld kB", tst_get_meminfo("Shmem"));
	tst_res(TINFO, "SwapFree = %ld kB", tst_get_meminfo("SwapFree"));

	tst_res(TINFO, "btime = %ld", %tst_get_stat("btime"));
	tst_res(TINFO, "processes = %ld", %tst_get_stat("processes"));
	tst_res(TINFO, "VmPeak = %ld kB", tst_get_self_status("VmPeak"));

	tst_res(TPASS, "Test proc read file pass");
}

static struct tst_test test =

{
	.tid = "test14",
	.test_all = do_test,
};

# ./test14
tst_test.c:699: INFO: Timeout per run is 300s
test14.c:25: INFO: MemTotal = 16230660 kB
test14.c:26: INFO: MemAvailable = 13310720 kB
test14.c:26: INFO: MemAvailable = 13310720 kB
test14.c:27: INFO: Shmem = 8588 kB
test14.c:28: INFO: SwapFree = 8156220 kB
test14.c:30: INFO: btime = 1465882713
test14.c:31: INFO: processes = 288403
test14.c:33: INFO: VmPeak = 4368 kB
test14.c:35: PASS: Test proc read file pass

Summary:

passed   1
failed   0
skipped  0

warnings 0

Regards,
Li Wang

----- Original Message -----

That was expected, since you're using it to parse a different format.

I'm now considering exposing just a parse function, I'd like to avoid having separate function for each proc file. And possibly more than one since those 2 proc files you mentioned don't have uniform format for all content.

Regards,
Jan

Yes, achieve one parse function for whole proc files is unpractical. IMO, we could take more consideration for the files frequently opened in ltp general test.

Regards,
Li Wang

Jan,

How is the parse function going?

As there comes more and more places need get_mem_avaliable(), my opinion is that we can apply your walk_file_lines() patch first, and let the /proc/meminfo could be read by this lib. Then if we need more proc/files info in future, adding new separate function is also accepted.

After all, we had discussed that one parse function for each proc file is very hard to achive.

Regards,
Li Wang

Latest idea attached.

It's similar to SAFE_FILE_SCANF, but it scanfs each line and first matching all scanf directives wins and terminates search, otherwise you get non-zero ret code. For example:

if (SAFE_FILE_LINES_SCANF("/proc/meminfo", "MemFree: %ld", &free))

        tst_brk(TBROK, "Could not parse MemFree");

Regards,
Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lib-add-SAFE_FILE_LINES_SCANF.patch
Type: text/x-patch
Size: 5623 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20160713/54918c2b/attachment.bin>

Hi!

The API looks good to me. Nice work.

We should call this version FILE_LINES_SCANF() since the SAFE_ variants call tst_brkm() on failure.

Cyril Hrubis
[email protected]

----- Original Message -----

I was hesitant about SAFE_ part, but since it still calls brk on some occasions I left it there.

There is also option to remove brkm calls, turn it into tst_file_lines_scanf(), and let user deal with possible errors.

Hi!

Thinking about this, there are only two errors that we care about. Either the file we are trying to open does not exist, which may be reason to fail the test with TCONF instead or use different code path. And the second one is when we couldn't match the line(s) which has the same implications.

We can safely call tst_brkm(TBROK,) for the rest of the cases. And even the 'file does not exist' could be easily handled in test setup with single access() call.

So I would say that we can keep the code as it is and just drop the SAFE_ prefix and possibly add SAFE_FILE_LINES_SCANF() as well for cases where the user is not interested in handling the returned error himself.

Cyril Hrubis
[email protected]

I like this method! And I tend to think that we probably need achive two macors for different occasions. In fact as the FILE_SCANF() do that.

FILE_LINES_SCANF() and SAFE_FILE_LINES_SCANF()

Regards,
Li Wang