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

[Cyril Hrubis <chrubis@suse.cz>]

> -- 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.

> > > +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.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp