Re: [LTP] [PATCH] fix "hugetlb" several tests

Mitani
2010-04-16T02:04:34Z

> -----Original Message-----
> From: Garrett Cooper [mailto:yanegomi@gmail.com]
> Sent: Thursday, April 15, 2010 5:43 PM
> To: Mitani; Garrett Cooper; ltp-list@lists.sourceforge.net;
> kamimura@ryobi.co.jp
> Subject: Re: [LTP] [PATCH] fix "hugetlb" several tests
>
> On Wed, Apr 14, 2010 at 11:54 PM, Rishikesh K Rajak
> <risrajak@linux.vnet.ibm.com> wrote:
> > On Thu, Apr 15, 2010 at 09:56:22AM +0900, Mitani wrote:
> >>
> >> > -----Original Message-----
> >> > From: Garrett Cooper [mailto:yanegomi@gmail.com]
> >> > Sent: Wednesday, April 14, 2010 6:31 PM
> >> > To: Mitani
> >> > Cc: ltp-list@lists.sourceforge.net
> >> > Subject: Re: [LTP] [PATCH] fix "hugetlb" several tests
> >> >
> >> > On Wed, Apr 14, 2010 at 1:58 AM, Mitani <mitani@ryobi.co.jp> wrote:
> >> > >> -----Original Message-----
> >> > >> From: Garrett Cooper [mailto:yanegomi@gmail.com]
> >> > >> Sent: Wednesday, April 14, 2010 3:18 PM
> >> > >> To: Mitani
> >> > >> Cc: Randy Dunlap; ltp-list@lists.sourceforge.net
> >> > >> Subject: Re: [LTP] [PATCH] fix "hugetlb" several tests
> >> > >>
> >> > >> On Tue, Apr 13, 2010 at 7:17 PM, Mitani <mitani@ryobi.co.jp>
> wrote:
> >> > >> > Hi Randy,
> >> > >> >
> >> > >> > I couldn't notice a misspelling. Sorry.
> >> > >> > I decided to use "due to" according to your advice.
> >> > >> >
> >> > >> > May I suggest revised patch?
> >> > >> >
> >> > > [...]
> >> > >> >
> >> > >> > Thank you--
> >> > >> >
> >> > >> > -Tomonori Mitani
> >> > >> >
> >> > >> >
> >> > >> >> -----Original Message-----
> >> > >> >> From: Randy Dunlap [mailto:rdunlap@xenotime.net]
> >> > >> >> Sent: Wednesday, April 14, 2010 12:04 AM
> >> > >> >> To: Mitani
> >> > >> >> Cc: ltp-list@lists.sourceforge.net
> >> > >> >> Subject: Re: [LTP] [PATCH] fix "hugetlb" several tests
> >> > >> >>
> >> > >> >> On 04/12/10 23:58, Mitani wrote:
> >> > >> >> > ------------
> >> > >> >> >  a) All tests:
> >> > >> >> > "TBROK  :  Test cannot be continued owning to sufficient
> >> > >> >> availability of
> >> > >> >> > Hugepages on the system"
> >> > >> >> >
> >> > >> >> >  b) 2), 3), 5), 6), 8), 10), 11) tests:
> >> > >> >> > "TWARN  :  tst_rmdir(): TESTDIR was NULL; no removal
> >> > attempted"
> >> > >> >> > ------------
> >> > >> >> >
> >> > >> >> > Both case a) and case b) are caused by the same reason.
> >> > >> >> >
> >> > >> >> > All of case a) failures occured at the following points
> (for
> >> > >> example
> >> > >> >> > hugemmap04):
> >> > >> >> > ------------<hugemmap04.c - main()>
> >> > >> >> >         /* Check number of hugepages */
> >> > >> >> >         if (get_no_of_hugepages() <= 0 ||
> hugepages_size()
> >> > <=
> >> > >> 0)
> >> > >> >> >                 tst_brkm(TBROK, cleanup, "Test cannot
> be
> >> > >> continued
> >> > >> >> owning to
> >> > >> >> > \
> >> > >> >> >                                 sufficient
> availability
> >> > of
> >> > >> >> Hugepages on the
> >> > >> >> > system");
> >> > >> >> > ------------
> >> > >> >> >
> >> > >> >> > I found out that "HugePages_Total" parameter of
> "/proc/meminfo"
> >> > >> file
> >> > >> >> > is set to "0". This caused above TBROK failure. It is
> environment
> >> > >> >> problem.
> >> > >> >> >
> >> > >> >> > But, in this case, tests must not return with TBROK, but
> with
> >> > TCONF,
> >> > >> >> > I think.
> >> > >> >>
> >> > >> >> That makes sense to me.
> >> > >> >>
> >> > >> >> > And, in case b), these tests try to delete "TESTDIR"
> directory
> >> > >> by
> >> > >> >> > calling "tst_rmdir()" function in "cleanup()" function.
> >> > >> >> > But, "TESTDIR" never set if "tst_tmpdir()" function isn't
> >> > called.
> >> > >> >> > I think that case b)'s tests must not call cleanup()
> function.
> >> > >> >> >
> >> > >> >> >
> >> > >> >> > I want to suggest following patch.
> >> > >> >> >
> >> > >> >> > Signed-off-by: Tomonori Mitani <mitani@ryobi.co.jp>
> >> >
> >> >     Sorry to ask again, but Gmail always mangles patches. Could
> you
> >> > please attach it as a file?
> >> > Thanks,
> >> > -Garrett
> >>
> >>
> >> Sorry.
> >> I attach a patch file.
> >
> >
> > Applied to next branch. Will wait for someone to Acked/test it before
> merging to
> > master.
>
> Inspecting the files more closely, here all cases should be
> tst_exit, not cleanup. Otherwise it's going to do some operations not
> required as setup hasn't be run yet.
> Thanks,
> -Garrett

HI,

There is a problem.
In hugeshmget01.c, setup() calling is posted before than the judgment
of hugepages support:
------------
setup(); /* global setup */

/* The following loop checks looping state if -i option given */
if ( get_no_of_hugepages() <= 0 || hugepages_size() <= 0 )
tst_brkm(TBROK, cleanup, "Test cannot be continued owning to
sufficient avail
ability of Hugepages on the system");
else
huge_pages_shm_to_be_allocated = ( get_no_of_hugepages() *
hugepages_size()
* 1024) / 2 ;
------------

Therefore, with this order of processing, clean of setup is necessary,
I think

Can setup() calling be replaced after the judgment of hugepages support?
Perhaps, it's OK, I think ...

Regards--

-Tomonori Mitani