All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Stancek <jstancek@redhat.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v3] syscalls: Add set_mempolicy numa tests.
Date: Wed, 16 Jan 2019 06:49:25 -0500 (EST)	[thread overview]
Message-ID: <1825051207.96340275.1547639365640.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20190116111201.GC23245@rei.lan>


----- Original Message -----
> Hi!
> > > +
> > > +
> > > +void tst_numa_alloc_parse(struct tst_nodemap *nodes, const char *path,
> > > +                          unsigned int pages)
> > 
> > I'd change this to a function that only counts allocated pages,
> > and increases counters.
> 
> Fair enough.
> 
> > > +{
> > > +	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);
> > 
> > I think all the alloc stuff shouldn't be in library. It's quite
> > possible tests will need different ways to allocate memory.
> > 
> > Maybe the test will want to mmap, but not touch any pages,
> > or pass different flags to mmap. Allocate with other functions
> > than mmap, etc.
> 
> It should be put into some kind of common place though since it's used
> from several tests even at this point.

set_mempolicy sub-dir?

> 
> What about splitting the tst_numa_alloc_parse() to tst_numa_alloc() and
> tst_numa_parse() and keeping the functionality in the library?

That looks more flexible.

Though I'm not 100% sold on single alloc func in library. My concern is
it will be difficult for it to cover all possible scenarios and API
will keep changing as we find more.

Regards,
Jan

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 

  reply	other threads:[~2019-01-16 11:49 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-18 15:57 [LTP] [PATCH v3] syscalls: Add set_mempolicy numa tests Cyril Hrubis
2019-01-03 11:39 ` Jan Stancek
2019-01-16 11:12   ` Cyril Hrubis
2019-01-16 11:49     ` Jan Stancek [this message]
2019-01-16 12:36       ` 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=1825051207.96340275.1547639365640.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.