[LTP] [PATCH 01/29] Hugetlb: Migrating libhugetlbfs brk_near_huge

Tarun Sahu tsahu@linux.ibm.com
Tue Oct 25 07:56:20 CEST 2022


Hi Cyril, 

On Fri, 2022-10-21 at 10:58 +0200, Cyril Hrubis wrote:
> > -- skip
> > > > +	p = SAFE_MMAP(hugemap_addr, hpage_size,
> > > > PROT_READ|PROT_WRITE,
> > > > +			MAP_PRIVATE|MAP_FIXED, fd, 0);
> > > > +	if (p != hugemap_addr) {
> > > > +		tst_res(TFAIL, "mmap() at unexpected address %p
> > > > instead
> > > > of %p\n", p,
> > > > +		     hugemap_addr);
> > > > +		goto fail;
> > > 
> > > Can we just do return here instead. The failure has been reported
> > > there
> > > is no point in calling tst_brk(TBROK, ...) as well.
> > When we run one iteration only, tst_brk does not make sense, I
> > agree.
> > But if we are running more than one iteration (i >= 2), test
> > should not continue to next iteration if the current iteration
> > fails.
> > Only way I could find is to use tst_brk(TBROK,... , as
> > tst_brk(TFAIl...
> > is deprecated.
> 
> I do not see why we should abort on first failure as long as it's not
> unrecoverable error. The TBROK status is only for cases where
> something
> went really wrong and we cannot continue.

Understood. I have removed it & updated your suggestions here
https://lore.kernel.org/all/20221019184846.89318-1-tsahu@linux.ibm.com/
Here, I posted same patches in small batch (only 3 at first) based on
suggestion given by Richard.
> 
> > > > +static void cleanup(void)
> > > > +{
> > > > +	if (fd >= 0)
> > > > +		SAFE_CLOSE(fd);
> > > > +	umount2(Hopt, MNT_DETACH);
> > > 
> > > We whould umount here only if we actually have mounted something.
> > umount only, will require explicit unmap when test fails or break.
> > For
> > that, all the local variable for address mapping will have to be
> > static defined so that they can be accessed in cleanup() function.
> > 
> > I tried to avoid it by using umount2 which eventually umount when
> > process unmaps all the mappings after it finishes.
> 
> That's not what I meant, as long as you pass Hopt on a commandline
> the test shoud not attempt to umount it at all.

Hopt is supposed to be the location which test will use to mount/umount
the hugetlbfs fs. If Hopt is not provided, it will create a temp
location. It is only to avoid creating any temporary locations outside
user concerned test environment.

     if(!Hopt)
	Hopt = tst_get_tmpdir();
     SAFE_MOUNT("none", Hopt, "hugetlbfs", 0, NULL);

It is not expecting any already mounted hugetlbfs.
Please, check 
https://lore.kernel.org/all/20221019184846.89318-1-
tsahu@linux.ibm.com/
 for updated patch.

Thanks, 
Tarun



More information about the ltp mailing list