Re: [LTP] [RFC][PATCH 0/2] Add safe_get_nodemap()

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> not a huge improvement, but because all tst_get_nodemap() are in the
> setup (and only ksm06.c allows input parameter) we could have struct
> tst_test member which would call safe_get_nodemap().
> 
> e.g.:
> 
> .nodemap = (const struct tst_path_val) {
> 		.type = TST_NUMA_MEM
> 		.required = 2,
> },

I do not get this, the struct tst_path_val is something completely
different.

> safe_get_nodemap(tst_test->nodemap->type,
> 	tst_test->nodemap->required * getpagesize() / 1024);
> 
> This would not work for non - page sized nodes, e.g.:
> nodes = tst_get_nodemap(TST_NUMA_MEM, 2 * PAGES_ALLOCATED * page_size / 1024);
> 
> => extra member would need to be added:
> 
> .nodemap = (const struct tst_path_val) {
> 		.type = TST_NUMA_MEM
> 		.required = 2,
> 		.size = PAGES_ALLOCATED, // default == 1
> },

I've avoided the size to be in pages and choosen kilobytes instead
because the page size can differ a lot.

> would call:
> 
> safe_get_nodemap(tst_test->nodemap->type,
> 	tst_test->nodemap->required * tst_test->nodemap->size * getpagesize() / 1024,
         ^
	 This does not make any sense.

The memory parameter is supposed to be minimal free memory on each node,
for each node we check that it has at least min_mem_kb in the
node_has_enough_memory() function.

There is no point in multiplying that by the number of nodes we require.

And this looks like it uses kilobytes not pages.

> 	tst_test->nodemap->required);

I guess that what you meant is that we will add tst_numa structure such as:

struct tst_numa {
	enum tst_numa_types node_type;
	unsigned int min_nodes;
	unsigned int min_mem_kb;
};

And then use that in tst_test structure. That would certainly make
sense, but I guess that we would have to move the numa library to the
lib/ as well. I'm not sure that we can have a function call from
tst_test.c library to something that is not compiled in by default.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp