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

[Tarun Sahu]

On Nov 17 2022, Tarun Sahu wrote:
> 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?
> 
I missed it,
I could make use of mktemp. will replace all this. and update it in next
version.

> > >  	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
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp