All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/1] syscalls/get_mempolicy01: Rewrite to new API
Date: Wed, 9 Dec 2020 23:10:56 +0100	[thread overview]
Message-ID: <20201209221056.GB69775@pevik> (raw)
In-Reply-To: <X9DO3uytgBaTrVwi@yuki.lan>

Hi Cyril,

...
> > -	if (!is_numa(NULL, NH_MEMS, 1))
> > -		tst_brkm(TCONF, NULL, "requires NUMA with at least 1 node");
> > +	if (get_allowed_nodes(NH_MEMS, 1, &test_node) < 0)
> > +		tst_brk(TBROK | TERRNO, "get_allowed_nodes failed");

> The is_numa() and get_allowed_nodes() is deprecated API, we do have new
> tst_get_nodemap() function that replaces them. See set_mempolicy()
> testcases for reference.
Thanks!

> > -	TEST_PAUSE;
> > -	tst_tmpdir();
> > +	nodemask = numa_allocate_nodemask();
> > +	getnodemask = numa_allocate_nodemask();
> > +	numa_bitmask_setbit(nodemask, test_node);
> >  }

> > -#else
> > -int main(void)
> > +static void do_test(unsigned int i)
> >  {
> > -	tst_brkm(TCONF, NULL, NUMA_ERROR_MSG);
> > +	struct test_case *tc = &tcase[i];
> > +	int policy;
> > +
> > +	tst_res(TINFO, "test #%d: %s", (i+1), tc->desc);
> > +
> > +	setup_node();
> > +
> > +	if (tc->pre_test)
> > +		if (tc->pre_test(i) == -1)
> > +			return;
> > +
> > +	if (tc->test) {
> > +		tc->test(i);
> > +
> > +		if (TST_RET < 0) {
> > +			tst_res(TFAIL | TERRNO, ".test failed");
> > +			return;
> > +		}
> > +	}

> We call test_mbind() here for each iteration which calls mmap()
> and the memory is never freed. Which means that this will fail sooner or
> later with the -i option.

> Why can't we allocate all the blocks with different mempolicy and
> or/bind the memory once in the test setup instead? We can keep the
> callback in-place as they are we just need to loop over them in the
> setup() instead. Also I would rename them to alloc, setup, or something
> like that so that it's clear that they are just preparing the
> environment and not doing the actuall test.
Thanks for catching this. I actually run it more with -i500,
but it looks laptop has enough memory :). Anyway, what you suggest is obviously
much better solution, thanks!

> Also I would pass the struct test_case pointer to these instead of i
> since they convert the i to the testcase pointer as the first thing
> anyways.
+1

Kind regards,
Petr

  reply	other threads:[~2020-12-09 22:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-08 13:28 [LTP] [PATCH 1/1] syscalls/get_mempolicy01: Rewrite to new API Petr Vorel
2020-12-08 13:54 ` Petr Vorel
2020-12-09 13:19 ` Cyril Hrubis
2020-12-09 22:10   ` Petr Vorel [this message]
2021-01-06 11:23   ` Petr Vorel

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=20201209221056.GB69775@pevik \
    --to=pvorel@suse.cz \
    --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.