All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Stancek <jstancek@redhat.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v2] syscalls: Add set_mempolicy numa tests.
Date: Fri, 24 Aug 2018 05:09:06 -0400 (EDT)	[thread overview]
Message-ID: <1686534238.42299708.1535101746576.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20180823135457.15806-1-chrubis@suse.cz>


----- Original Message -----
> This is initial attempt to replace numa.sh tests that despite having
> been fixed several times have still many shortcommings that wouldn't be
> easy to fix. It's not finished nor 100% replacement but I'm sending this
> anyway because I would like to get feedback at this point.
> 
> The main selling points of these testcases are:
> 
> The memory allocated for the testing is tracked exactly. We are using
> get_mempolicy() with MPOL_F_NODE | MPOL_F_ADDR that returns the node ID on
> which specified address is allocated on to count pages allocated per node
> after
> we set desired memory policy.
> 
> The tests for file based shared interleaved mappings are no longer
> mapping a single small file but rather than that we accumulate statistic
> for larger amount of files over longer period of time and we also allow
> for small offset (currently 10%). We should probably also increase the
> number of samples we take as currently it's about 5MB in total on x86
> although I haven't managed to make this test fail so far. This also
> fixes the test on Btrfs where the synthetic test that expects the pages
> to be distributed exactly equally fails.
> 
> What is not finished is compilation without libnuma, that will fail
> currently, but that is only a matter of adding a few ifdefs. And the
> coverage is still lacking, ideas for interesting testcases are welcomed
> as well.
> 
> I tested this testcases back to SLES11, which is basically the oldest
> distribution we really care for and everything was working fine.
> 

I don't have objections to this approach. Just want to re-iterate, that
we should also check each node has enough free memory. Couple comments below:

<snip>

+static void inc_counter(unsigned int node, struct tst_nodemap *nodes)
+{
+        unsigned int i;
+
+        for (i = 0; i < nodes->cnt; i++) {
+                if (nodes->map[i] == node) {
+                        nodes->counters[i]++;
+                        break;
+                }

I'd add some check here to warn or TBROK, in case we somehow end up with node id
that's not in the map.

> +void tst_numa_alloc_parse(struct tst_nodemap *nodes, const char *path,
> +                          unsigned int pages)
> +{
> +	size_t page_size = getpagesize();
> +	char *ptr;
> +	int fd = -1;
> +	int flags = MAP_PRIVATE|MAP_ANONYMOUS;
> +	int node;
> +	unsigned int i;
> +
> +	if (path) {
> +		fd = SAFE_OPEN(path, O_CREAT | O_EXCL | O_RDWR, 0666);
> +		SAFE_FTRUNCATE(fd, pages * page_size);
> +		flags = MAP_SHARED;
> +	}
> +
> +	ptr = SAFE_MMAP(NULL, pages  * page_size,
> +	                PROT_READ|PROT_WRITE, flags, fd, 0);
> +
> +	if (path) {
> +		SAFE_CLOSE(fd);
> +		SAFE_UNLINK(path);
> +	}
> +
> +	memset(ptr, 'a', pages * page_size);
> +
> +	for (i = 0; i < pages; i++) {
> +		get_mempolicy(&node, NULL, 0, ptr + i * page_size, MPOL_F_NODE |
> MPOL_F_ADDR);
> +
> +		if (node < 0 || (unsigned int)node >= nodes->cnt) {
> +			tst_res(TWARN, "get_mempolicy(...) returned invalid node %i\n", node);
> +			continue;
> +		}
> +
> +		inc_counter(node, nodes);
> +	}

This seems useful as a separate function, for example: tst_nodemap_count_area(ptr, size).
Maybe the user wants to count already allocated area.

> +++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy01.c
> + * We are testing set_mempolicy() with MPOL_BIND and MPOL_PREFERRED.

> +++ b/testcases/kernel/syscalls/set_mempolicy/set_mempolicy03.c
> + * We are testing set_mempolicy() with MPOL_BIND and MPOL_PREFERRED backed
> by a file.

First and third test look very similar, any chance we can combine them?

Regards,
Jan

  reply	other threads:[~2018-08-24  9:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-23 13:54 [LTP] [PATCH v2] syscalls: Add set_mempolicy numa tests Cyril Hrubis
2018-08-24  9:09 ` Jan Stancek [this message]
2018-08-24 12:10   ` Cyril Hrubis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1686534238.42299708.1535101746576.JavaMail.zimbra@redhat.com \
    --to=jstancek@redhat.com \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.