From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1042663AbdDUW0y (ORCPT ); Fri, 21 Apr 2017 18:26:54 -0400 Received: from mail.kernel.org ([198.145.29.136]:56982 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1034805AbdDUW0w (ORCPT ); Fri, 21 Apr 2017 18:26:52 -0400 Date: Sat, 22 Apr 2017 07:26:41 +0900 From: Masami Hiramatsu To: Steven Rostedt Cc: LKML , Shuah Khan , Namhyung Kim Subject: Re: [RFC][PATCH] selftests: ftrace: Allow some tests to be run in a tracing instance Message-Id: <20170422072641.e1805c90b0dc13b9de3db276@kernel.org> In-Reply-To: <20170421095334.6b1c1c31@gandalf.local.home> References: <20170420133034.0746d5c1@gandalf.local.home> <20170421183617.fbab5eb08f137f436c1f69cf@kernel.org> <20170421190025.f398a44b662c9bff80b3cc10@kernel.org> <20170421095334.6b1c1c31@gandalf.local.home> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 21 Apr 2017 09:53:34 -0400 Steven Rostedt wrote: > On Fri, 21 Apr 2017 19:00:25 +0900 > Masami Hiramatsu wrote: > > > On Fri, 21 Apr 2017 18:36:17 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 20 Apr 2017 13:30:34 -0400 > > > Steven Rostedt wrote: > > > > > > > > An tracing instance has several of the same capabilities as the top level > > > > instance, but may be implemented slightly different. Instead of just writing > > > > tests that duplicat the same test cases of the top level instance, allow a > > > > test to be written for both the top level as well as for an instance. BTW, would you need "testcases only for instance" or "testcase for top-level and instance" ? I thought latter... > > > > > > > > If a test case can be run in both the top level as well as in an tracing > > > > instance directory, then it should have a ".itc" extension instead of just > > > > the ".tc" extension. > > > > > > Hm, I'd rather like to have a tag for each script instead of > > > using the extension. E.g. > > OK, I'll update. I just finished testing all this too :-/ > > > > > > > #!/bin/sh > > > # description: Kprobe dynamic event - busy event check > > > # test: instance > > Hmm, what about "flags:" instead. As a way to denote what this test has. OK, that's better. > > # flags: instance > > > > > BTW, I also have some comments, see below, > > > > [...] > > > > +INSTANCE_DIR="." > > > > __run_test() { # testfile > > > > # setup PID and PPID, $$ is not updated. > > > > - (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; . $1) > > > > + (cd $TRACING_DIR; read PID _ < /proc/self/stat; set -e; set -x; initialize_ftrace; cd $INSTANCE_DIR; . $1) > > > > You can just change "cd $TRACING_DIR;" with "cd $TRACING_DIR/$INSTANCE_DIR;" > > Then, the default value of INSTANCE_DIR can be "". > > I'll look into doing this. I originally had an option to run just > instance tests, and overriding TRACING_DIR wasn't an option at the time. My idea is using INSTANCE_DIR not only for instance name, but also for a flag which indicates "instance test is running, need to be removed". > > > > [ $? -ne 0 ] && kill -s $SIG_FAIL $SIG_PID > > > > } > > > > > > > > # Run one test case > > > > -run_test() { # testfile > > > > +run_test() { # [-i] testfile > > > > + local makeinstance=0 > > > > + if [ $1 == "-i" ]; then > > > > + makeinstance=1 > > > > + shift > > > > + fi > > > > No, don't parse argument in the function. You can just define > > MAKEINSTANCE=1 global variable before calling this. > > Again, this was due to the ftracetest option. I'll update. I see. > > > > > > > local testname=`basename $1` > > > > local testlog=`mktemp $LOG_DIR/${testname}-log.XXXXXX` > > > > export TMPDIR=`mktemp -d /tmp/ftracetest-dir.XXXXXX` > > > > testcase $1 > > > > Anyway, if we use a "tag" in the script header, we can get it > > in this testcase() and update TEST_TAGS global variable there. > > TEST_TAGS? Or you mean something like the way to get the description. > > > > > > > + if [ $makeinstance -eq 1 ]; then > > > > and check whether "instance" tag in $TEST_TAGS here. > > TEST_FLAGS? Yes, that's good to me :) > > > > > > > + INSTANCE_DIR=$TRACING_DIR/instances/${testname}_test > > > > + mkdir $INSTANCE_DIR > > > > + fi > > > > > > > > echo "execute: "$1 > $testlog > > > > SIG_RESULT=0 > > > > if [ $VERBOSE -ge 2 ]; then > > > > @@ -260,17 +274,36 @@ run_test() { # testfile > > > > [ $VERBOSE -ge 1 ] && catlog $testlog > > > > TOTAL_RESULT=1 > > > > fi > > > > + if [ $makeinstance -eq 1 ]; then > > > > Here also we can just check the "$INSTANCE_DIR" is not empty. > > Why? It never will be empty. It's a pseudo dir and files can not be > created or destroyed in it by the user. It should be created by the > test, and destroyed by the test. Hopefully it's a unique name. The name > is currently something like instances/func_event_triggers.tc_test > It's "instances/${testname}_test". I could even add a $$ just to be > more random here. As I said above, INSTANCE_DIR can be a test flag too, and that ensures we've actually made it. (Just for make sure there is) BTW, maybe we can use mktemp if you'd like avoid name conflict. Thank you, > > > > > Thank you, > > Thanks for you review! I'll have a new patch out soon. > > -- Steve > > > > > > > + rmdir $INSTANCE_DIR > > > > + INSTANCE_DIR="." > > > > + fi > > > > rm -rf $TMPDIR > > > > } > > > > > > > > # load in the helper functions > > > > . $TEST_DIR/functions > > > > > > > > +RUN_INSTANCES=0 > > > > + > > > > # Main loop > > > > for t in $TEST_CASES; do > > > > + if [ "${t/*./}" == "itc" ]; then > > > > + RUN_INSTANCES=1 > > > > + fi > > > > run_test $t > > > > done > > > > > > > > +if [ $RUN_INSTANCES -eq 1 ]; then > > > > + echo "Running tests in an tracing instance:" > > > > + for t in $TEST_CASES; do > > > > + if [ "${t/*./}" != "itc" ]; then > > > > + continue > > > > + fi > > > > + run_test -i $t > > > > + done > > > > +fi > > > > + > > > > prlog "" > > > > prlog "# of passed: " `echo $PASSED_CASES | wc -w` > > > > prlog "# of failed: " `echo $FAILED_CASES | wc -w` > > > > -- > > > > 2.9.3 > > > > > > > > > > > > > -- > > > Masami Hiramatsu > > > > > -- Masami Hiramatsu