[LTP] [PATCH v4 1/7] Hugetlb: Add new argument flags in tst_creat_unlinked

[Tarun Sahu]

Hi, 
Thanks for reviewing the patch.
On Nov 16 2022, Cyril Hrubis wrote:
> Hi!
> > Some test requires custom flags for file opened by tst_creat_unlinked
> > along with O_CREAT|O_EXCL|O_RDWR. This patch creates support to pass
> > custom flags in tst_creat_unlinked.
> > 
> > Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> > ---
> >  include/tst_test.h                            |  2 +-
> >  lib/tst_test.c                                | 19 ++++++++++++++-----
> >  .../kernel/mem/hugetlb/hugemmap/hugemmap07.c  |  2 +-
> >  .../kernel/mem/hugetlb/hugemmap/hugemmap08.c  |  2 +-
> >  .../kernel/mem/hugetlb/hugemmap/hugemmap09.c  |  2 +-
> >  5 files changed, 18 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index acf2421de..a62515bfe 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -365,7 +365,7 @@ void tst_set_max_runtime(int max_runtime);
> >   * Create and open a random file inside the given dir path.
> >   * It unlinks the file after opening and return file descriptor.
> >   */
> > -int tst_creat_unlinked(const char *path);
> > +int tst_creat_unlinked(const char *path, int flags);
> >  
> >  /*
> >   * Returns path to the test temporary directory in a newly allocated buffer.
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index b225ba082..d17b15ee8 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1027,18 +1027,27 @@ static void prepare_and_mount_hugetlb_fs(void)
> >  	mntpoint_mounted = 1;
> >  }
> >  
> > -int tst_creat_unlinked(const char *path)
> > +int tst_creat_unlinked(const char *path, int flags)
> >  {
> >  	char template[PATH_MAX];
> > +	int len, c, range;
> >  	int fd;
> > +	int start[3] = {'0', 'a', 'A'};
> >  
> >  	snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",
> >  			path, tid);
> > +	len = strlen(template) - 1;
> > +
> > +	srand(time(NULL));
> > +	while (template[len] == 'X') {
> > +		c = rand() % 3;
> > +		range = start[c] == '0' ? 10 : 26;
> > +		c = start[c] + (rand() % range);
> > +		template[len--] = (char)c;
> > +	}
> >  
> > -	fd = mkstemp(template);
> > -	if (fd < 0)
> > -		tst_brk(TBROK | TERRNO, "mkstemp(%s) failed", template);
> > -
> > +	flags |= O_CREAT|O_EXCL|O_RDWR;
> > +	fd = SAFE_OPEN(template, flags);
> 
> I wonder if it would be easier to add the O_DIRECT flag with F_GETFL and
> F_SETFL fcntl(), but I guess that this is fine as it is.
> 
> The only potential problem I see is that setting the random seed may
> interfere with anything that would print what seed has been used for
> random data in order to be able to reproduce the same random sequence if
> needed. So maybe I wouldn't attempt to set the seed in this function at
> all.
> 
Ok. can remove it.

One more concern, I have, it wont be thread safe. Is it acceptable?
Though, an alternative is to use mkostemp, but it is only defined in
__GNU_SOURCE, defining it for the whole library tst_test.c doesnt seem good.

instead using rand_r will require _thread static seed to make it
thread safe. but doesn't seem to be worth the effort, if we can simply use,
mkostemp or fcntl to set flags manually later.

What do you think?

> >  	SAFE_UNLINK(template);
> >  	return fd;
> >  }
> > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > index bd0fb440a..3122d5b9d 100644
> > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap07.c
> > @@ -113,7 +113,7 @@ cleanup:
> >  static void setup(void)
> >  {
> >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> >  }
> >  
> >  static void cleanup(void)
> > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > index ce40e7b69..f66b331dc 100644
> > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap08.c
> > @@ -118,7 +118,7 @@ static void run_test(unsigned int test_type)
> >  static void setup(void)
> >  {
> >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> >  }
> >  
> >  static void cleanup(void)
> > diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > index 1008395a4..ceb0f64a1 100644
> > --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap09.c
> > @@ -60,7 +60,7 @@ static void run_test(void)
> >  static void setup(void)
> >  {
> >  	hpage_size = SAFE_READ_MEMINFO(MEMINFO_HPAGE_SIZE)*1024;
> > -	huge_fd = tst_creat_unlinked(MNTPOINT);
> > +	huge_fd = tst_creat_unlinked(MNTPOINT, 0);
> >  }
> >  
> >  static void cleanup(void)
> > -- 
> > 2.31.1
> > 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp