From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-6.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id 31D637D90D for ; Tue, 13 Aug 2019 04:22:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726086AbfHMEWB (ORCPT ); Tue, 13 Aug 2019 00:22:01 -0400 Received: from mail.kernel.org ([198.145.29.99]:54932 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725899AbfHMEWA (ORCPT ); Tue, 13 Aug 2019 00:22:00 -0400 Received: from kernel.org (unknown [104.132.0.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 46814206C2; Tue, 13 Aug 2019 04:21:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1565670119; bh=2wsciK8EQNkiIDpIzT9gqNOBT1vFkTwhm0156mR8+vg=; h=In-Reply-To:References:Subject:From:Cc:To:Date:From; b=wecwCow5B+6BYtZFhLgSBtfBg3HYwUxRc7BK/brCT0tDKGApHgFlM+vfqd81P64X0 04JP0Nu9+FaPoa3LNBz2rHGjoJcRlNjXPSlwk78/5FIAJiFy49LChFOYIg/iHUwNr0 72imQjPEbEhZyBYzzpi7XEUQSlI3qDs3pOlrbs2w= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20190812182421.141150-10-brendanhiggins@google.com> References: <20190812182421.141150-1-brendanhiggins@google.com> <20190812182421.141150-10-brendanhiggins@google.com> Subject: Re: [PATCH v12 09/18] kunit: test: add support for test abort From: Stephen Boyd Cc: devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, kunit-dev@googlegroups.com, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-um@lists.infradead.org, Alexander.Levin@microsoft.com, Tim.Bird@sony.com, amir73il@gmail.com, dan.carpenter@oracle.com, daniel@ffwll.ch, jdike@addtoit.com, joel@jms.id.au, julia.lawall@lip6.fr, khilman@baylibre.com, knut.omang@oracle.com, logang@deltatee.com, mpe@ellerman.id.au, pmladek@suse.com, rdunlap@infradead.org, richard@nod.at, rientjes@google.com, rostedt@goodmis.org, wfg@linux.intel.com, Brendan Higgins To: Brendan Higgins , frowand.list@gmail.com, gregkh@linuxfoundation.org, jpoimboe@redhat.com, keescook@google.com, kieran.bingham@ideasonboard.com, mcgrof@kernel.org, peterz@infradead.org, robh@kernel.org, shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com User-Agent: alot/0.8.1 Date: Mon, 12 Aug 2019 21:21:58 -0700 Message-Id: <20190813042159.46814206C2@mail.kernel.org> Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org Quoting Brendan Higgins (2019-08-12 11:24:12) > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 2625bcfeb19ac..93381f841e09f 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > =20 > struct kunit_resource; > =20 > @@ -167,6 +168,7 @@ struct kunit { > =20 > /* private: internal use only. */ > const char *name; /* Read only after initialization! */ > + struct kunit_try_catch try_catch; > /* > * success starts as true, and may only be set to false during a = test > * case; thus, it is safe to update this across multiple threads = using > @@ -176,6 +178,11 @@ struct kunit { > */ > bool success; /* Read only after test_case finishes! */ > spinlock_t lock; /* Gaurds all mutable test state. */ > + /* > + * death_test may be both set and unset from multiple threads in = a test > + * case. > + */ > + bool death_test; /* Protected by lock. */ > /* > * Because resources is a list that may be updated multiple times= (with > * new resources) from any thread associated with a test case, we= must > @@ -184,6 +191,13 @@ struct kunit { > struct list_head resources; /* Protected by lock. */ > }; > =20 > +static inline void kunit_set_death_test(struct kunit *test, bool death_t= est) > +{ > + spin_lock(&test->lock); > + test->death_test =3D death_test; > + spin_unlock(&test->lock); > +} These getters and setters are using spinlocks again. It doesn't make any sense. It probably needs a rework like was done for the other bool member, success. > + > void kunit_init_test(struct kunit *test, const char *name); > =20 > int kunit_run_tests(struct kunit_suite *suite); > diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h > new file mode 100644 > index 0000000000000..8a414a9af0b64 > --- /dev/null > +++ b/include/kunit/try-catch.h > @@ -0,0 +1,69 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * An API to allow a function, that may fail, to be executed, and recove= r in a > + * controlled manner. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#ifndef _KUNIT_TRY_CATCH_H > +#define _KUNIT_TRY_CATCH_H > + > +#include > + > +typedef void (*kunit_try_catch_func_t)(void *); > + > +struct kunit; Forward declare struct completion? > + > +/* > + * struct kunit_try_catch - provides a generic way to run code which mig= ht fail. > + * @context: used to pass user data to the try and catch functions. > + * > + * kunit_try_catch provides a generic, architecture independent way to e= xecute > + * an arbitrary function of type kunit_try_catch_func_t which may bail o= ut by > + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called= , @try > + * is stopped at the site of invocation and @catch is catch is called. > + * > + * struct kunit_try_catch provides a generic interface for the functiona= lity > + * needed to implement kunit->abort() which in turn is needed for implem= enting > + * assertions. Assertions allow stating a precondition for a test simpli= fying > + * how test cases are written and presented. > + * > + * Assertions are like expectations, except they abort (call > + * kunit_try_catch_throw()) when the specified condition is not met. Thi= s is > + * useful when you look at a test case as a logical statement about some= piece > + * of code, where assertions are the premises for the test case, and the > + * conclusion is a set of predicates, rather expectations, that must all= be > + * true. If your premises are violated, it does not makes sense to conti= nue. > + */ > +struct kunit_try_catch { > + /* private: internal use only. */ > + struct kunit *test; > + struct completion *try_completion; > + int try_result; > + kunit_try_catch_func_t try; > + kunit_try_catch_func_t catch; Can these other variables be documented in the kernel doc? And should context be marked as 'public'? > + void *context; > +}; > + > +void kunit_try_catch_init(struct kunit_try_catch *try_catch, > + struct kunit *test, > + kunit_try_catch_func_t try, > + kunit_try_catch_func_t catch); > + > +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *contex= t); > + > +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch); > + > +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try= _catch) > +{ > + return try_catch->try_result; > +} > + > +/* > + * Exposed for testing only. Ugh that's sad. I hope we don't expose more functions just for testing in other cases. > + */ > +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch); > + > +#endif /* _KUNIT_TRY_CATCH_H */ > diff --git a/kunit/test.c b/kunit/test.c > index e5080a2c6b29c..995cb53fe4ee9 100644 > --- a/kunit/test.c > +++ b/kunit/test.c > @@ -7,13 +7,26 @@ > */ > =20 > #include > +#include > #include > +#include > =20 > static void kunit_set_failure(struct kunit *test) > { > WRITE_ONCE(test->success, false); > } > =20 > +static bool kunit_get_death_test(struct kunit *test) > +{ > + bool death_test; > + > + spin_lock(&test->lock); > + death_test =3D test->death_test; > + spin_unlock(&test->lock); > + > + return death_test; > +} > + > static int kunit_vprintk_emit(int level, const char *fmt, va_list args) > { > return vprintk_emit(0, level, NULL, 0, fmt, args); > @@ -158,6 +171,21 @@ static void kunit_fail(struct kunit *test, struct ku= nit_assert *assert) > kunit_print_string_stream(test, stream); > } > =20 > +void __noreturn kunit_abort(struct kunit *test) > +{ > + kunit_set_death_test(test, true); > + > + kunit_try_catch_throw(&test->try_catch); > + > + /* > + * Throw could not abort from test. > + * > + * XXX: we should never reach this line! As kunit_try_catch_throw= is > + * marked __noreturn. > + */ > + WARN_ONCE(true, "Throw could not abort from test!\n"); Should this just be a BUG_ON? It's supposedly impossible. > +} > + > void kunit_do_assertion(struct kunit *test, > struct kunit_assert *assert, > bool pass, > @@ -176,6 +204,9 @@ void kunit_do_assertion(struct kunit *test, > kunit_fail(test, assert); > =20 > va_end(args); > + > + if (assert->type =3D=3D KUNIT_ASSERTION) > + kunit_abort(test); > } > =20 > void kunit_init_test(struct kunit *test, const char *name) > @@ -184,36 +215,154 @@ void kunit_init_test(struct kunit *test, const cha= r *name) > INIT_LIST_HEAD(&test->resources); > test->name =3D name; > test->success =3D true; > + test->death_test =3D false; > } > =20 > /* > - * Performs all logic to run a test case. > + * Initializes and runs test case. Does not clean up or do post validati= ons. > */ > -static void kunit_run_case(struct kunit_suite *suite, > - struct kunit_case *test_case) > +static void kunit_run_case_internal(struct kunit *test, > + struct kunit_suite *suite, > + struct kunit_case *test_case) > { > - struct kunit test; > - > - kunit_init_test(&test, test_case->name); > - > if (suite->init) { > int ret; > =20 > - ret =3D suite->init(&test); > + ret =3D suite->init(test); > if (ret) { > - kunit_err(&test, "failed to initialize: %d\n", re= t); > - kunit_set_failure(&test); > - test_case->success =3D test.success; > + kunit_err(test, "failed to initialize: %d\n", ret= ); > + kunit_set_failure(test); > return; > } > } > =20 > - test_case->run_case(&test); > + test_case->run_case(test); > +} > + > +static void kunit_case_internal_cleanup(struct kunit *test) > +{ > + kunit_cleanup(test); > +} > =20 > +/* > + * Performs post validations and cleanup after a test case was run. > + * XXX: Should ONLY BE CALLED AFTER kunit_run_case_internal! > + */ > +static void kunit_run_case_cleanup(struct kunit *test, > + struct kunit_suite *suite) > +{ > if (suite->exit) > - suite->exit(&test); > + suite->exit(test); > + > + kunit_case_internal_cleanup(test); > +} > + > +/* > + * Handles an unexpected crash in a test case. > + */ > +static void kunit_handle_test_crash(struct kunit *test, > + struct kunit_suite *suite, > + struct kunit_case *test_case) > +{ > + kunit_err(test, "kunit test case crashed!"); Does this need a newline? > + /* > + * TODO(brendanhiggins@google.com): This prints the stack trace up > + * through this frame, not up to the frame that caused the crash. > + */ > + show_stack(NULL, NULL); > + > + kunit_case_internal_cleanup(test); > +} > + > +struct kunit_try_catch_context { > + struct kunit *test; > + struct kunit_suite *suite; > + struct kunit_case *test_case; > +}; > + > +static void kunit_try_run_case(void *data) > +{ > + struct kunit_try_catch_context *ctx =3D data; > + struct kunit *test =3D ctx->test; > + struct kunit_suite *suite =3D ctx->suite; > + struct kunit_case *test_case =3D ctx->test_case; > + > + /* > + * kunit_run_case_internal may encounter a fatal error; if it doe= s, > + * abort will be called, this thread will exit, and finally the p= arent > + * thread will resume control and handle any necessary clean up. > + */ > + kunit_run_case_internal(test, suite, test_case); > + /* This line may never be reached. */ > + kunit_run_case_cleanup(test, suite); > +} > + > +static void kunit_catch_run_case(void *data) > +{ > + struct kunit_try_catch_context *ctx =3D data; > + struct kunit *test =3D ctx->test; > + struct kunit_suite *suite =3D ctx->suite; > + struct kunit_case *test_case =3D ctx->test_case; > + int try_exit_code =3D kunit_try_catch_get_result(&test->try_catch= ); > + > + if (try_exit_code) { > + kunit_set_failure(test); > + /* > + * Test case could not finish, we have no idea what state= it is > + * in, so don't do clean up. > + */ > + if (try_exit_code =3D=3D -ETIMEDOUT) > + kunit_err(test, "test case timed out\n"); > + /* > + * Unknown internal error occurred preventing test case f= rom > + * running, so there is nothing to clean up. > + */ > + else > + kunit_err(test, "internal error occurred preventi= ng test case from running: %d\n", > + try_exit_code); Nitpick: I would add braces here because you make the if statement into multi-line arms for each case. > + return; > + } > + > + if (kunit_get_death_test(test)) { > + /* > + * EXPECTED DEATH: kunit_run_case_internal encountered > + * anticipated fatal error. Everything should be in a safe > + * state. > + */ > + kunit_run_case_cleanup(test, suite); > + } else { > + /* > + * UNEXPECTED DEATH: kunit_run_case_internal encountered = an > + * unanticipated fatal error. We have no idea what the st= ate of > + * the test case is in. > + */ > + kunit_handle_test_crash(test, suite, test_case); > + kunit_set_failure(test); Like was done here. > + } > +} > + > +/* > + * Performs all logic to run a test case. It also catches most errors th= at > + * occurs in a test case and reports them as failures. s/occurs/occur/ > + */ > +static void kunit_run_case_catch_errors(struct kunit_suite *suite, [...] > diff --git a/kunit/try-catch.c b/kunit/try-catch.c > new file mode 100644 > index 0000000000000..de580f074387b > --- /dev/null > +++ b/kunit/try-catch.c > @@ -0,0 +1,95 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * An API to allow a function, that may fail, to be executed, and recove= r in a > + * controlled manner. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#include > +#include > +#include > +#include > + > +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch) > +{ > + try_catch->try_result =3D -EFAULT; > + complete_and_exit(try_catch->try_completion, -EFAULT); > +} > + > +static int kunit_generic_run_threadfn_adapter(void *data) > +{ > + struct kunit_try_catch *try_catch =3D data; > + > + try_catch->try(try_catch->context); > + > + complete_and_exit(try_catch->try_completion, 0); > +} > + > +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *contex= t) > +{ > + DECLARE_COMPLETION_ONSTACK(try_completion); > + struct kunit *test =3D try_catch->test; > + struct task_struct *task_struct; > + int exit_code, status; > + > + try_catch->context =3D context; > + try_catch->try_completion =3D &try_completion; > + try_catch->try_result =3D 0; > + task_struct =3D kthread_run(kunit_generic_run_threadfn_adapter, > + try_catch, > + "kunit_try_catch_thread"); > + if (IS_ERR(task_struct)) { > + try_catch->catch(try_catch->context); > + return; > + } > + > + /* > + * TODO(brendanhiggins@google.com): We should probably have some = type of > + * variable timeout here. The only question is what that timeout = value > + * should be. > + * > + * The intention has always been, at some point, to be able to la= bel > + * tests with some type of size bucket (unit/small, integration/m= edium, > + * large/system/end-to-end, etc), where each size bucket would ge= t a > + * default timeout value kind of like what Bazel does: > + * https://docs.bazel.build/versions/master/be/common-definitions= .html#test.size > + * There is still some debate to be had on exactly how we do this= . (For > + * one, we probably want to have some sort of test runner level > + * timeout.) > + * > + * For more background on this topic, see: > + * https://mike-bland.com/2011/11/01/small-medium-large.html > + */ > + status =3D wait_for_completion_timeout(&try_completion, > + 300 * MSEC_PER_SEC); /* 5 mi= n */ > + if (status < 0) { wait_for_completion_timeout() doesn't return a negative value on timeout. It returns 0. Please rename 'status' to 'time_remaining' and test with if (!time_remaining) instead or some other suitably named variable name indicating that the return value is the time remaining before the timeout. May also want to clamp this to the hung task timeout value, which is typically less than 5 minutes. Otherwise, the hung task detector may find the problem first before this timeout happens. > + kunit_err(test, "try timed out\n"); > + try_catch->try_result =3D -ETIMEDOUT; > + } From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:54932 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725899AbfHMEWA (ORCPT ); Tue, 13 Aug 2019 00:22:00 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20190812182421.141150-10-brendanhiggins@google.com> References: <20190812182421.141150-1-brendanhiggins@google.com> <20190812182421.141150-10-brendanhiggins@google.com> Subject: Re: [PATCH v12 09/18] kunit: test: add support for test abort From: Stephen Boyd Date: Mon, 12 Aug 2019 21:21:58 -0700 Message-Id: <20190813042159.46814206C2@mail.kernel.org> Sender: linux-kbuild-owner@vger.kernel.org List-ID: To: frowand.list@gmail.com, gregkh@linuxfoundation.org, jpoimboe@redhat.com, keescook@google.com, kieran.bingham@ideasonboard.com, mcgrof@kernel.org, peterz@infradead.org, robh@kernel.org, shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com Cc: devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, kunit-dev@googlegroups.com, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-um@lists.infradead.org, Alexander.Levin@microsoft.com, Tim.Bird@sony.com, amir73il@gmail.com, dan.carpenter@oracle.com, daniel@ffwll.ch, jdike@addtoit.com, joel@jms.id.au, julia.lawall@lip6.fr, khilman@baylibre.com, knut.omang@oracle.com, logang@deltatee.com, mpe@ellerman.id.au, pmladek@suse.com, rdunlap@infradead.org, richard@nod.at, rientjes@google.com, rostedt@goodmis.org, wfg@linux.intel.com, Brendan Higgins Quoting Brendan Higgins (2019-08-12 11:24:12) > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 2625bcfeb19ac..93381f841e09f 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > =20 > struct kunit_resource; > =20 > @@ -167,6 +168,7 @@ struct kunit { > =20 > /* private: internal use only. */ > const char *name; /* Read only after initialization! */ > + struct kunit_try_catch try_catch; > /* > * success starts as true, and may only be set to false during a = test > * case; thus, it is safe to update this across multiple threads = using > @@ -176,6 +178,11 @@ struct kunit { > */ > bool success; /* Read only after test_case finishes! */ > spinlock_t lock; /* Gaurds all mutable test state. */ > + /* > + * death_test may be both set and unset from multiple threads in = a test > + * case. > + */ > + bool death_test; /* Protected by lock. */ > /* > * Because resources is a list that may be updated multiple times= (with > * new resources) from any thread associated with a test case, we= must > @@ -184,6 +191,13 @@ struct kunit { > struct list_head resources; /* Protected by lock. */ > }; > =20 > +static inline void kunit_set_death_test(struct kunit *test, bool death_t= est) > +{ > + spin_lock(&test->lock); > + test->death_test =3D death_test; > + spin_unlock(&test->lock); > +} These getters and setters are using spinlocks again. It doesn't make any sense. It probably needs a rework like was done for the other bool member, success. > + > void kunit_init_test(struct kunit *test, const char *name); > =20 > int kunit_run_tests(struct kunit_suite *suite); > diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h > new file mode 100644 > index 0000000000000..8a414a9af0b64 > --- /dev/null > +++ b/include/kunit/try-catch.h > @@ -0,0 +1,69 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * An API to allow a function, that may fail, to be executed, and recove= r in a > + * controlled manner. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#ifndef _KUNIT_TRY_CATCH_H > +#define _KUNIT_TRY_CATCH_H > + > +#include > + > +typedef void (*kunit_try_catch_func_t)(void *); > + > +struct kunit; Forward declare struct completion? > + > +/* > + * struct kunit_try_catch - provides a generic way to run code which mig= ht fail. > + * @context: used to pass user data to the try and catch functions. > + * > + * kunit_try_catch provides a generic, architecture independent way to e= xecute > + * an arbitrary function of type kunit_try_catch_func_t which may bail o= ut by > + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called= , @try > + * is stopped at the site of invocation and @catch is catch is called. > + * > + * struct kunit_try_catch provides a generic interface for the functiona= lity > + * needed to implement kunit->abort() which in turn is needed for implem= enting > + * assertions. Assertions allow stating a precondition for a test simpli= fying > + * how test cases are written and presented. > + * > + * Assertions are like expectations, except they abort (call > + * kunit_try_catch_throw()) when the specified condition is not met. Thi= s is > + * useful when you look at a test case as a logical statement about some= piece > + * of code, where assertions are the premises for the test case, and the > + * conclusion is a set of predicates, rather expectations, that must all= be > + * true. If your premises are violated, it does not makes sense to conti= nue. > + */ > +struct kunit_try_catch { > + /* private: internal use only. */ > + struct kunit *test; > + struct completion *try_completion; > + int try_result; > + kunit_try_catch_func_t try; > + kunit_try_catch_func_t catch; Can these other variables be documented in the kernel doc? And should context be marked as 'public'? > + void *context; > +}; > + > +void kunit_try_catch_init(struct kunit_try_catch *try_catch, > + struct kunit *test, > + kunit_try_catch_func_t try, > + kunit_try_catch_func_t catch); > + > +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *contex= t); > + > +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch); > + > +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try= _catch) > +{ > + return try_catch->try_result; > +} > + > +/* > + * Exposed for testing only. Ugh that's sad. I hope we don't expose more functions just for testing in other cases. > + */ > +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch); > + > +#endif /* _KUNIT_TRY_CATCH_H */ > diff --git a/kunit/test.c b/kunit/test.c > index e5080a2c6b29c..995cb53fe4ee9 100644 > --- a/kunit/test.c > +++ b/kunit/test.c > @@ -7,13 +7,26 @@ > */ > =20 > #include > +#include > #include > +#include > =20 > static void kunit_set_failure(struct kunit *test) > { > WRITE_ONCE(test->success, false); > } > =20 > +static bool kunit_get_death_test(struct kunit *test) > +{ > + bool death_test; > + > + spin_lock(&test->lock); > + death_test =3D test->death_test; > + spin_unlock(&test->lock); > + > + return death_test; > +} > + > static int kunit_vprintk_emit(int level, const char *fmt, va_list args) > { > return vprintk_emit(0, level, NULL, 0, fmt, args); > @@ -158,6 +171,21 @@ static void kunit_fail(struct kunit *test, struct ku= nit_assert *assert) > kunit_print_string_stream(test, stream); > } > =20 > +void __noreturn kunit_abort(struct kunit *test) > +{ > + kunit_set_death_test(test, true); > + > + kunit_try_catch_throw(&test->try_catch); > + > + /* > + * Throw could not abort from test. > + * > + * XXX: we should never reach this line! As kunit_try_catch_throw= is > + * marked __noreturn. > + */ > + WARN_ONCE(true, "Throw could not abort from test!\n"); Should this just be a BUG_ON? It's supposedly impossible. > +} > + > void kunit_do_assertion(struct kunit *test, > struct kunit_assert *assert, > bool pass, > @@ -176,6 +204,9 @@ void kunit_do_assertion(struct kunit *test, > kunit_fail(test, assert); > =20 > va_end(args); > + > + if (assert->type =3D=3D KUNIT_ASSERTION) > + kunit_abort(test); > } > =20 > void kunit_init_test(struct kunit *test, const char *name) > @@ -184,36 +215,154 @@ void kunit_init_test(struct kunit *test, const cha= r *name) > INIT_LIST_HEAD(&test->resources); > test->name =3D name; > test->success =3D true; > + test->death_test =3D false; > } > =20 > /* > - * Performs all logic to run a test case. > + * Initializes and runs test case. Does not clean up or do post validati= ons. > */ > -static void kunit_run_case(struct kunit_suite *suite, > - struct kunit_case *test_case) > +static void kunit_run_case_internal(struct kunit *test, > + struct kunit_suite *suite, > + struct kunit_case *test_case) > { > - struct kunit test; > - > - kunit_init_test(&test, test_case->name); > - > if (suite->init) { > int ret; > =20 > - ret =3D suite->init(&test); > + ret =3D suite->init(test); > if (ret) { > - kunit_err(&test, "failed to initialize: %d\n", re= t); > - kunit_set_failure(&test); > - test_case->success =3D test.success; > + kunit_err(test, "failed to initialize: %d\n", ret= ); > + kunit_set_failure(test); > return; > } > } > =20 > - test_case->run_case(&test); > + test_case->run_case(test); > +} > + > +static void kunit_case_internal_cleanup(struct kunit *test) > +{ > + kunit_cleanup(test); > +} > =20 > +/* > + * Performs post validations and cleanup after a test case was run. > + * XXX: Should ONLY BE CALLED AFTER kunit_run_case_internal! > + */ > +static void kunit_run_case_cleanup(struct kunit *test, > + struct kunit_suite *suite) > +{ > if (suite->exit) > - suite->exit(&test); > + suite->exit(test); > + > + kunit_case_internal_cleanup(test); > +} > + > +/* > + * Handles an unexpected crash in a test case. > + */ > +static void kunit_handle_test_crash(struct kunit *test, > + struct kunit_suite *suite, > + struct kunit_case *test_case) > +{ > + kunit_err(test, "kunit test case crashed!"); Does this need a newline? > + /* > + * TODO(brendanhiggins@google.com): This prints the stack trace up > + * through this frame, not up to the frame that caused the crash. > + */ > + show_stack(NULL, NULL); > + > + kunit_case_internal_cleanup(test); > +} > + > +struct kunit_try_catch_context { > + struct kunit *test; > + struct kunit_suite *suite; > + struct kunit_case *test_case; > +}; > + > +static void kunit_try_run_case(void *data) > +{ > + struct kunit_try_catch_context *ctx =3D data; > + struct kunit *test =3D ctx->test; > + struct kunit_suite *suite =3D ctx->suite; > + struct kunit_case *test_case =3D ctx->test_case; > + > + /* > + * kunit_run_case_internal may encounter a fatal error; if it doe= s, > + * abort will be called, this thread will exit, and finally the p= arent > + * thread will resume control and handle any necessary clean up. > + */ > + kunit_run_case_internal(test, suite, test_case); > + /* This line may never be reached. */ > + kunit_run_case_cleanup(test, suite); > +} > + > +static void kunit_catch_run_case(void *data) > +{ > + struct kunit_try_catch_context *ctx =3D data; > + struct kunit *test =3D ctx->test; > + struct kunit_suite *suite =3D ctx->suite; > + struct kunit_case *test_case =3D ctx->test_case; > + int try_exit_code =3D kunit_try_catch_get_result(&test->try_catch= ); > + > + if (try_exit_code) { > + kunit_set_failure(test); > + /* > + * Test case could not finish, we have no idea what state= it is > + * in, so don't do clean up. > + */ > + if (try_exit_code =3D=3D -ETIMEDOUT) > + kunit_err(test, "test case timed out\n"); > + /* > + * Unknown internal error occurred preventing test case f= rom > + * running, so there is nothing to clean up. > + */ > + else > + kunit_err(test, "internal error occurred preventi= ng test case from running: %d\n", > + try_exit_code); Nitpick: I would add braces here because you make the if statement into multi-line arms for each case. > + return; > + } > + > + if (kunit_get_death_test(test)) { > + /* > + * EXPECTED DEATH: kunit_run_case_internal encountered > + * anticipated fatal error. Everything should be in a safe > + * state. > + */ > + kunit_run_case_cleanup(test, suite); > + } else { > + /* > + * UNEXPECTED DEATH: kunit_run_case_internal encountered = an > + * unanticipated fatal error. We have no idea what the st= ate of > + * the test case is in. > + */ > + kunit_handle_test_crash(test, suite, test_case); > + kunit_set_failure(test); Like was done here. > + } > +} > + > +/* > + * Performs all logic to run a test case. It also catches most errors th= at > + * occurs in a test case and reports them as failures. s/occurs/occur/ > + */ > +static void kunit_run_case_catch_errors(struct kunit_suite *suite, [...] > diff --git a/kunit/try-catch.c b/kunit/try-catch.c > new file mode 100644 > index 0000000000000..de580f074387b > --- /dev/null > +++ b/kunit/try-catch.c > @@ -0,0 +1,95 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * An API to allow a function, that may fail, to be executed, and recove= r in a > + * controlled manner. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#include > +#include > +#include > +#include > + > +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch) > +{ > + try_catch->try_result =3D -EFAULT; > + complete_and_exit(try_catch->try_completion, -EFAULT); > +} > + > +static int kunit_generic_run_threadfn_adapter(void *data) > +{ > + struct kunit_try_catch *try_catch =3D data; > + > + try_catch->try(try_catch->context); > + > + complete_and_exit(try_catch->try_completion, 0); > +} > + > +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *contex= t) > +{ > + DECLARE_COMPLETION_ONSTACK(try_completion); > + struct kunit *test =3D try_catch->test; > + struct task_struct *task_struct; > + int exit_code, status; > + > + try_catch->context =3D context; > + try_catch->try_completion =3D &try_completion; > + try_catch->try_result =3D 0; > + task_struct =3D kthread_run(kunit_generic_run_threadfn_adapter, > + try_catch, > + "kunit_try_catch_thread"); > + if (IS_ERR(task_struct)) { > + try_catch->catch(try_catch->context); > + return; > + } > + > + /* > + * TODO(brendanhiggins@google.com): We should probably have some = type of > + * variable timeout here. The only question is what that timeout = value > + * should be. > + * > + * The intention has always been, at some point, to be able to la= bel > + * tests with some type of size bucket (unit/small, integration/m= edium, > + * large/system/end-to-end, etc), where each size bucket would ge= t a > + * default timeout value kind of like what Bazel does: > + * https://docs.bazel.build/versions/master/be/common-definitions= .html#test.size > + * There is still some debate to be had on exactly how we do this= . (For > + * one, we probably want to have some sort of test runner level > + * timeout.) > + * > + * For more background on this topic, see: > + * https://mike-bland.com/2011/11/01/small-medium-large.html > + */ > + status =3D wait_for_completion_timeout(&try_completion, > + 300 * MSEC_PER_SEC); /* 5 mi= n */ > + if (status < 0) { wait_for_completion_timeout() doesn't return a negative value on timeout. It returns 0. Please rename 'status' to 'time_remaining' and test with if (!time_remaining) instead or some other suitably named variable name indicating that the return value is the time remaining before the timeout. May also want to clamp this to the hung task timeout value, which is typically less than 5 minutes. Otherwise, the hung task detector may find the problem first before this timeout happens. > + kunit_err(test, "try timed out\n"); > + try_catch->try_result =3D -ETIMEDOUT; > + } From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH v12 09/18] kunit: test: add support for test abort Date: Mon, 12 Aug 2019 21:21:58 -0700 Message-ID: <20190813042159.46814206C2@mail.kernel.org> References: <20190812182421.141150-1-brendanhiggins@google.com> <20190812182421.141150-10-brendanhiggins@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20190812182421.141150-10-brendanhiggins@google.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: frowand.list@gmail.com, gregkh@linuxfoundation.org, jpoimboe@redhat.com, keescook@google.com, kieran.bingham@ideasonboard.com, mcgrof@kernel.org, peterz@infradead.org, robh@kernel.org, shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com Cc: pmladek@suse.com, linux-doc@vger.kernel.org, amir73il@gmail.com, Brendan Higgins , dri-devel@lists.freedesktop.org, Alexander.Levin@microsoft.com, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, khilman@baylibre.com, knut.omang@oracle.com, wfg@linux.intel.com, joel@jms.id.au, rientjes@google.com, jdike@addtoit.com, dan.carpenter@oracle.com, devicetree@vger.kernel.org, linux-kbuild@vger.kernel.org, Tim.Bird@sony.com, linux-um@lists.infradead.org, rostedt@goodmis.org, julia.lawall@lip6.fr, kunit-dev@googlegroups.com, richard@nod.at, rdunlap@infradead.org, linux-kernel@vger.kernel.org, mpe@ellerman.id.au, linux-fsdevel@vger.kernel.org, logang@deltatee.com List-Id: linux-nvdimm@lists.01.org UXVvdGluZyBCcmVuZGFuIEhpZ2dpbnMgKDIwMTktMDgtMTIgMTE6MjQ6MTIpCj4gZGlmZiAtLWdp dCBhL2luY2x1ZGUva3VuaXQvdGVzdC5oIGIvaW5jbHVkZS9rdW5pdC90ZXN0LmgKPiBpbmRleCAy NjI1YmNmZWIxOWFjLi45MzM4MWY4NDFlMDlmIDEwMDY0NAo+IC0tLSBhL2luY2x1ZGUva3VuaXQv dGVzdC5oCj4gKysrIGIvaW5jbHVkZS9rdW5pdC90ZXN0LmgKPiBAQCAtMTMsNiArMTMsNyBAQAo+ ICAjaW5jbHVkZSA8bGludXgvdHlwZXMuaD4KPiAgI2luY2x1ZGUgPGxpbnV4L3NsYWIuaD4KPiAg I2luY2x1ZGUgPGt1bml0L2Fzc2VydC5oPgo+ICsjaW5jbHVkZSA8a3VuaXQvdHJ5LWNhdGNoLmg+ Cj4gIAo+ICBzdHJ1Y3Qga3VuaXRfcmVzb3VyY2U7Cj4gIAo+IEBAIC0xNjcsNiArMTY4LDcgQEAg c3RydWN0IGt1bml0IHsKPiAgCj4gICAgICAgICAvKiBwcml2YXRlOiBpbnRlcm5hbCB1c2Ugb25s eS4gKi8KPiAgICAgICAgIGNvbnN0IGNoYXIgKm5hbWU7IC8qIFJlYWQgb25seSBhZnRlciBpbml0 aWFsaXphdGlvbiEgKi8KPiArICAgICAgIHN0cnVjdCBrdW5pdF90cnlfY2F0Y2ggdHJ5X2NhdGNo Owo+ICAgICAgICAgLyoKPiAgICAgICAgICAqIHN1Y2Nlc3Mgc3RhcnRzIGFzIHRydWUsIGFuZCBt YXkgb25seSBiZSBzZXQgdG8gZmFsc2UgZHVyaW5nIGEgdGVzdAo+ICAgICAgICAgICogY2FzZTsg dGh1cywgaXQgaXMgc2FmZSB0byB1cGRhdGUgdGhpcyBhY3Jvc3MgbXVsdGlwbGUgdGhyZWFkcyB1 c2luZwo+IEBAIC0xNzYsNiArMTc4LDExIEBAIHN0cnVjdCBrdW5pdCB7Cj4gICAgICAgICAgKi8K PiAgICAgICAgIGJvb2wgc3VjY2VzczsgLyogUmVhZCBvbmx5IGFmdGVyIHRlc3RfY2FzZSBmaW5p c2hlcyEgKi8KPiAgICAgICAgIHNwaW5sb2NrX3QgbG9jazsgLyogR2F1cmRzIGFsbCBtdXRhYmxl IHRlc3Qgc3RhdGUuICovCj4gKyAgICAgICAvKgo+ICsgICAgICAgICogZGVhdGhfdGVzdCBtYXkg YmUgYm90aCBzZXQgYW5kIHVuc2V0IGZyb20gbXVsdGlwbGUgdGhyZWFkcyBpbiBhIHRlc3QKPiAr ICAgICAgICAqIGNhc2UuCj4gKyAgICAgICAgKi8KPiArICAgICAgIGJvb2wgZGVhdGhfdGVzdDsg LyogUHJvdGVjdGVkIGJ5IGxvY2suICovCj4gICAgICAgICAvKgo+ICAgICAgICAgICogQmVjYXVz ZSByZXNvdXJjZXMgaXMgYSBsaXN0IHRoYXQgbWF5IGJlIHVwZGF0ZWQgbXVsdGlwbGUgdGltZXMg KHdpdGgKPiAgICAgICAgICAqIG5ldyByZXNvdXJjZXMpIGZyb20gYW55IHRocmVhZCBhc3NvY2lh dGVkIHdpdGggYSB0ZXN0IGNhc2UsIHdlIG11c3QKPiBAQCAtMTg0LDYgKzE5MSwxMyBAQCBzdHJ1 Y3Qga3VuaXQgewo+ICAgICAgICAgc3RydWN0IGxpc3RfaGVhZCByZXNvdXJjZXM7IC8qIFByb3Rl Y3RlZCBieSBsb2NrLiAqLwo+ICB9Owo+ICAKPiArc3RhdGljIGlubGluZSB2b2lkIGt1bml0X3Nl dF9kZWF0aF90ZXN0KHN0cnVjdCBrdW5pdCAqdGVzdCwgYm9vbCBkZWF0aF90ZXN0KQo+ICt7Cj4g KyAgICAgICBzcGluX2xvY2soJnRlc3QtPmxvY2spOwo+ICsgICAgICAgdGVzdC0+ZGVhdGhfdGVz dCA9IGRlYXRoX3Rlc3Q7Cj4gKyAgICAgICBzcGluX3VubG9jaygmdGVzdC0+bG9jayk7Cj4gK30K ClRoZXNlIGdldHRlcnMgYW5kIHNldHRlcnMgYXJlIHVzaW5nIHNwaW5sb2NrcyBhZ2Fpbi4gSXQg ZG9lc24ndCBtYWtlIGFueQpzZW5zZS4gSXQgcHJvYmFibHkgbmVlZHMgYSByZXdvcmsgbGlrZSB3 YXMgZG9uZSBmb3IgdGhlIG90aGVyIGJvb2wKbWVtYmVyLCBzdWNjZXNzLgoKPiArCj4gIHZvaWQg a3VuaXRfaW5pdF90ZXN0KHN0cnVjdCBrdW5pdCAqdGVzdCwgY29uc3QgY2hhciAqbmFtZSk7Cj4g IAo+ICBpbnQga3VuaXRfcnVuX3Rlc3RzKHN0cnVjdCBrdW5pdF9zdWl0ZSAqc3VpdGUpOwo+IGRp ZmYgLS1naXQgYS9pbmNsdWRlL2t1bml0L3RyeS1jYXRjaC5oIGIvaW5jbHVkZS9rdW5pdC90cnkt Y2F0Y2guaAo+IG5ldyBmaWxlIG1vZGUgMTAwNjQ0Cj4gaW5kZXggMDAwMDAwMDAwMDAwMC4uOGE0 MTRhOWFmMGI2NAo+IC0tLSAvZGV2L251bGwKPiArKysgYi9pbmNsdWRlL2t1bml0L3RyeS1jYXRj aC5oCj4gQEAgLTAsMCArMSw2OSBAQAo+ICsvKiBTUERYLUxpY2Vuc2UtSWRlbnRpZmllcjogR1BM LTIuMCAqLwo+ICsvKgo+ICsgKiBBbiBBUEkgdG8gYWxsb3cgYSBmdW5jdGlvbiwgdGhhdCBtYXkg ZmFpbCwgdG8gYmUgZXhlY3V0ZWQsIGFuZCByZWNvdmVyIGluIGEKPiArICogY29udHJvbGxlZCBt YW5uZXIuCj4gKyAqCj4gKyAqIENvcHlyaWdodCAoQykgMjAxOSwgR29vZ2xlIExMQy4KPiArICog QXV0aG9yOiBCcmVuZGFuIEhpZ2dpbnMgPGJyZW5kYW5oaWdnaW5zQGdvb2dsZS5jb20+Cj4gKyAq Lwo+ICsKPiArI2lmbmRlZiBfS1VOSVRfVFJZX0NBVENIX0gKPiArI2RlZmluZSBfS1VOSVRfVFJZ X0NBVENIX0gKPiArCj4gKyNpbmNsdWRlIDxsaW51eC90eXBlcy5oPgo+ICsKPiArdHlwZWRlZiB2 b2lkICgqa3VuaXRfdHJ5X2NhdGNoX2Z1bmNfdCkodm9pZCAqKTsKPiArCj4gK3N0cnVjdCBrdW5p dDsKCkZvcndhcmQgZGVjbGFyZSBzdHJ1Y3QgY29tcGxldGlvbj8KCj4gKwo+ICsvKgo+ICsgKiBz dHJ1Y3Qga3VuaXRfdHJ5X2NhdGNoIC0gcHJvdmlkZXMgYSBnZW5lcmljIHdheSB0byBydW4gY29k ZSB3aGljaCBtaWdodCBmYWlsLgo+ICsgKiBAY29udGV4dDogdXNlZCB0byBwYXNzIHVzZXIgZGF0 YSB0byB0aGUgdHJ5IGFuZCBjYXRjaCBmdW5jdGlvbnMuCj4gKyAqCj4gKyAqIGt1bml0X3RyeV9j YXRjaCBwcm92aWRlcyBhIGdlbmVyaWMsIGFyY2hpdGVjdHVyZSBpbmRlcGVuZGVudCB3YXkgdG8g ZXhlY3V0ZQo+ICsgKiBhbiBhcmJpdHJhcnkgZnVuY3Rpb24gb2YgdHlwZSBrdW5pdF90cnlfY2F0 Y2hfZnVuY190IHdoaWNoIG1heSBiYWlsIG91dCBieQo+ICsgKiBjYWxsaW5nIGt1bml0X3RyeV9j YXRjaF90aHJvdygpLiBJZiBrdW5pdF90cnlfY2F0Y2hfdGhyb3coKSBpcyBjYWxsZWQsIEB0cnkK PiArICogaXMgc3RvcHBlZCBhdCB0aGUgc2l0ZSBvZiBpbnZvY2F0aW9uIGFuZCBAY2F0Y2ggaXMg Y2F0Y2ggaXMgY2FsbGVkLgo+ICsgKgo+ICsgKiBzdHJ1Y3Qga3VuaXRfdHJ5X2NhdGNoIHByb3Zp ZGVzIGEgZ2VuZXJpYyBpbnRlcmZhY2UgZm9yIHRoZSBmdW5jdGlvbmFsaXR5Cj4gKyAqIG5lZWRl ZCB0byBpbXBsZW1lbnQga3VuaXQtPmFib3J0KCkgd2hpY2ggaW4gdHVybiBpcyBuZWVkZWQgZm9y IGltcGxlbWVudGluZwo+ICsgKiBhc3NlcnRpb25zLiBBc3NlcnRpb25zIGFsbG93IHN0YXRpbmcg YSBwcmVjb25kaXRpb24gZm9yIGEgdGVzdCBzaW1wbGlmeWluZwo+ICsgKiBob3cgdGVzdCBjYXNl cyBhcmUgd3JpdHRlbiBhbmQgcHJlc2VudGVkLgo+ICsgKgo+ICsgKiBBc3NlcnRpb25zIGFyZSBs aWtlIGV4cGVjdGF0aW9ucywgZXhjZXB0IHRoZXkgYWJvcnQgKGNhbGwKPiArICoga3VuaXRfdHJ5 X2NhdGNoX3Rocm93KCkpIHdoZW4gdGhlIHNwZWNpZmllZCBjb25kaXRpb24gaXMgbm90IG1ldC4g VGhpcyBpcwo+ICsgKiB1c2VmdWwgd2hlbiB5b3UgbG9vayBhdCBhIHRlc3QgY2FzZSBhcyBhIGxv Z2ljYWwgc3RhdGVtZW50IGFib3V0IHNvbWUgcGllY2UKPiArICogb2YgY29kZSwgd2hlcmUgYXNz ZXJ0aW9ucyBhcmUgdGhlIHByZW1pc2VzIGZvciB0aGUgdGVzdCBjYXNlLCBhbmQgdGhlCj4gKyAq IGNvbmNsdXNpb24gaXMgYSBzZXQgb2YgcHJlZGljYXRlcywgcmF0aGVyIGV4cGVjdGF0aW9ucywg dGhhdCBtdXN0IGFsbCBiZQo+ICsgKiB0cnVlLiBJZiB5b3VyIHByZW1pc2VzIGFyZSB2aW9sYXRl ZCwgaXQgZG9lcyBub3QgbWFrZXMgc2Vuc2UgdG8gY29udGludWUuCj4gKyAqLwo+ICtzdHJ1Y3Qg a3VuaXRfdHJ5X2NhdGNoIHsKPiArICAgICAgIC8qIHByaXZhdGU6IGludGVybmFsIHVzZSBvbmx5 LiAqLwo+ICsgICAgICAgc3RydWN0IGt1bml0ICp0ZXN0Owo+ICsgICAgICAgc3RydWN0IGNvbXBs ZXRpb24gKnRyeV9jb21wbGV0aW9uOwo+ICsgICAgICAgaW50IHRyeV9yZXN1bHQ7Cj4gKyAgICAg ICBrdW5pdF90cnlfY2F0Y2hfZnVuY190IHRyeTsKPiArICAgICAgIGt1bml0X3RyeV9jYXRjaF9m dW5jX3QgY2F0Y2g7CgpDYW4gdGhlc2Ugb3RoZXIgdmFyaWFibGVzIGJlIGRvY3VtZW50ZWQgaW4g dGhlIGtlcm5lbCBkb2M/IEFuZCBzaG91bGQKY29udGV4dCBiZSBtYXJrZWQgYXMgJ3B1YmxpYyc/ Cgo+ICsgICAgICAgdm9pZCAqY29udGV4dDsKPiArfTsKPiArCj4gK3ZvaWQga3VuaXRfdHJ5X2Nh dGNoX2luaXQoc3RydWN0IGt1bml0X3RyeV9jYXRjaCAqdHJ5X2NhdGNoLAo+ICsgICAgICAgICAg ICAgICAgICAgICAgICAgc3RydWN0IGt1bml0ICp0ZXN0LAo+ICsgICAgICAgICAgICAgICAgICAg ICAgICAga3VuaXRfdHJ5X2NhdGNoX2Z1bmNfdCB0cnksCj4gKyAgICAgICAgICAgICAgICAgICAg ICAgICBrdW5pdF90cnlfY2F0Y2hfZnVuY190IGNhdGNoKTsKPiArCj4gK3ZvaWQga3VuaXRfdHJ5 X2NhdGNoX3J1bihzdHJ1Y3Qga3VuaXRfdHJ5X2NhdGNoICp0cnlfY2F0Y2gsIHZvaWQgKmNvbnRl eHQpOwo+ICsKPiArdm9pZCBfX25vcmV0dXJuIGt1bml0X3RyeV9jYXRjaF90aHJvdyhzdHJ1Y3Qg a3VuaXRfdHJ5X2NhdGNoICp0cnlfY2F0Y2gpOwo+ICsKPiArc3RhdGljIGlubGluZSBpbnQga3Vu aXRfdHJ5X2NhdGNoX2dldF9yZXN1bHQoc3RydWN0IGt1bml0X3RyeV9jYXRjaCAqdHJ5X2NhdGNo KQo+ICt7Cj4gKyAgICAgICByZXR1cm4gdHJ5X2NhdGNoLT50cnlfcmVzdWx0Owo+ICt9Cj4gKwo+ ICsvKgo+ICsgKiBFeHBvc2VkIGZvciB0ZXN0aW5nIG9ubHkuCgpVZ2ggdGhhdCdzIHNhZC4gSSBo b3BlIHdlIGRvbid0IGV4cG9zZSBtb3JlIGZ1bmN0aW9ucyBqdXN0IGZvciB0ZXN0aW5nCmluIG90 aGVyIGNhc2VzLgoKPiArICovCj4gK3ZvaWQga3VuaXRfZ2VuZXJpY190cnlfY2F0Y2hfaW5pdChz dHJ1Y3Qga3VuaXRfdHJ5X2NhdGNoICp0cnlfY2F0Y2gpOwo+ICsKPiArI2VuZGlmIC8qIF9LVU5J VF9UUllfQ0FUQ0hfSCAqLwo+IGRpZmYgLS1naXQgYS9rdW5pdC90ZXN0LmMgYi9rdW5pdC90ZXN0 LmMKPiBpbmRleCBlNTA4MGEyYzZiMjljLi45OTVjYjUzZmU0ZWU5IDEwMDY0NAo+IC0tLSBhL2t1 bml0L3Rlc3QuYwo+ICsrKyBiL2t1bml0L3Rlc3QuYwo+IEBAIC03LDEzICs3LDI2IEBACj4gICAq Lwo+ICAKPiAgI2luY2x1ZGUgPGxpbnV4L2tlcm5lbC5oPgo+ICsjaW5jbHVkZSA8bGludXgvc2No ZWQvZGVidWcuaD4KPiAgI2luY2x1ZGUgPGt1bml0L3Rlc3QuaD4KPiArI2luY2x1ZGUgPGt1bml0 L3RyeS1jYXRjaC5oPgo+ICAKPiAgc3RhdGljIHZvaWQga3VuaXRfc2V0X2ZhaWx1cmUoc3RydWN0 IGt1bml0ICp0ZXN0KQo+ICB7Cj4gICAgICAgICBXUklURV9PTkNFKHRlc3QtPnN1Y2Nlc3MsIGZh bHNlKTsKPiAgfQo+ICAKPiArc3RhdGljIGJvb2wga3VuaXRfZ2V0X2RlYXRoX3Rlc3Qoc3RydWN0 IGt1bml0ICp0ZXN0KQo+ICt7Cj4gKyAgICAgICBib29sIGRlYXRoX3Rlc3Q7Cj4gKwo+ICsgICAg ICAgc3Bpbl9sb2NrKCZ0ZXN0LT5sb2NrKTsKPiArICAgICAgIGRlYXRoX3Rlc3QgPSB0ZXN0LT5k ZWF0aF90ZXN0Owo+ICsgICAgICAgc3Bpbl91bmxvY2soJnRlc3QtPmxvY2spOwo+ICsKPiArICAg ICAgIHJldHVybiBkZWF0aF90ZXN0Owo+ICt9Cj4gKwo+ICBzdGF0aWMgaW50IGt1bml0X3Zwcmlu dGtfZW1pdChpbnQgbGV2ZWwsIGNvbnN0IGNoYXIgKmZtdCwgdmFfbGlzdCBhcmdzKQo+ICB7Cj4g ICAgICAgICByZXR1cm4gdnByaW50a19lbWl0KDAsIGxldmVsLCBOVUxMLCAwLCBmbXQsIGFyZ3Mp Owo+IEBAIC0xNTgsNiArMTcxLDIxIEBAIHN0YXRpYyB2b2lkIGt1bml0X2ZhaWwoc3RydWN0IGt1 bml0ICp0ZXN0LCBzdHJ1Y3Qga3VuaXRfYXNzZXJ0ICphc3NlcnQpCj4gICAgICAgICBrdW5pdF9w cmludF9zdHJpbmdfc3RyZWFtKHRlc3QsIHN0cmVhbSk7Cj4gIH0KPiAgCj4gK3ZvaWQgX19ub3Jl dHVybiBrdW5pdF9hYm9ydChzdHJ1Y3Qga3VuaXQgKnRlc3QpCj4gK3sKPiArICAgICAgIGt1bml0 X3NldF9kZWF0aF90ZXN0KHRlc3QsIHRydWUpOwo+ICsKPiArICAgICAgIGt1bml0X3RyeV9jYXRj aF90aHJvdygmdGVzdC0+dHJ5X2NhdGNoKTsKPiArCj4gKyAgICAgICAvKgo+ICsgICAgICAgICog VGhyb3cgY291bGQgbm90IGFib3J0IGZyb20gdGVzdC4KPiArICAgICAgICAqCj4gKyAgICAgICAg KiBYWFg6IHdlIHNob3VsZCBuZXZlciByZWFjaCB0aGlzIGxpbmUhIEFzIGt1bml0X3RyeV9jYXRj aF90aHJvdyBpcwo+ICsgICAgICAgICogbWFya2VkIF9fbm9yZXR1cm4uCj4gKyAgICAgICAgKi8K PiArICAgICAgIFdBUk5fT05DRSh0cnVlLCAiVGhyb3cgY291bGQgbm90IGFib3J0IGZyb20gdGVz dCFcbiIpOwoKU2hvdWxkIHRoaXMganVzdCBiZSBhIEJVR19PTj8gSXQncyBzdXBwb3NlZGx5IGlt cG9zc2libGUuCgo+ICt9Cj4gKwo+ICB2b2lkIGt1bml0X2RvX2Fzc2VydGlvbihzdHJ1Y3Qga3Vu aXQgKnRlc3QsCj4gICAgICAgICAgICAgICAgICAgICAgICAgc3RydWN0IGt1bml0X2Fzc2VydCAq YXNzZXJ0LAo+ICAgICAgICAgICAgICAgICAgICAgICAgIGJvb2wgcGFzcywKPiBAQCAtMTc2LDYg KzIwNCw5IEBAIHZvaWQga3VuaXRfZG9fYXNzZXJ0aW9uKHN0cnVjdCBrdW5pdCAqdGVzdCwKPiAg ICAgICAgIGt1bml0X2ZhaWwodGVzdCwgYXNzZXJ0KTsKPiAgCj4gICAgICAgICB2YV9lbmQoYXJn cyk7Cj4gKwo+ICsgICAgICAgaWYgKGFzc2VydC0+dHlwZSA9PSBLVU5JVF9BU1NFUlRJT04pCj4g KyAgICAgICAgICAgICAgIGt1bml0X2Fib3J0KHRlc3QpOwo+ICB9Cj4gIAo+ICB2b2lkIGt1bml0 X2luaXRfdGVzdChzdHJ1Y3Qga3VuaXQgKnRlc3QsIGNvbnN0IGNoYXIgKm5hbWUpCj4gQEAgLTE4 NCwzNiArMjE1LDE1NCBAQCB2b2lkIGt1bml0X2luaXRfdGVzdChzdHJ1Y3Qga3VuaXQgKnRlc3Qs IGNvbnN0IGNoYXIgKm5hbWUpCj4gICAgICAgICBJTklUX0xJU1RfSEVBRCgmdGVzdC0+cmVzb3Vy Y2VzKTsKPiAgICAgICAgIHRlc3QtPm5hbWUgPSBuYW1lOwo+ICAgICAgICAgdGVzdC0+c3VjY2Vz cyA9IHRydWU7Cj4gKyAgICAgICB0ZXN0LT5kZWF0aF90ZXN0ID0gZmFsc2U7Cj4gIH0KPiAgCj4g IC8qCj4gLSAqIFBlcmZvcm1zIGFsbCBsb2dpYyB0byBydW4gYSB0ZXN0IGNhc2UuCj4gKyAqIElu aXRpYWxpemVzIGFuZCBydW5zIHRlc3QgY2FzZS4gRG9lcyBub3QgY2xlYW4gdXAgb3IgZG8gcG9z dCB2YWxpZGF0aW9ucy4KPiAgICovCj4gLXN0YXRpYyB2b2lkIGt1bml0X3J1bl9jYXNlKHN0cnVj dCBrdW5pdF9zdWl0ZSAqc3VpdGUsCj4gLSAgICAgICAgICAgICAgICAgICAgICAgICAgc3RydWN0 IGt1bml0X2Nhc2UgKnRlc3RfY2FzZSkKPiArc3RhdGljIHZvaWQga3VuaXRfcnVuX2Nhc2VfaW50 ZXJuYWwoc3RydWN0IGt1bml0ICp0ZXN0LAo+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIHN0cnVjdCBrdW5pdF9zdWl0ZSAqc3VpdGUsCj4gKyAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgc3RydWN0IGt1bml0X2Nhc2UgKnRlc3RfY2FzZSkKPiAgewo+IC0gICAg ICAgc3RydWN0IGt1bml0IHRlc3Q7Cj4gLQo+IC0gICAgICAga3VuaXRfaW5pdF90ZXN0KCZ0ZXN0 LCB0ZXN0X2Nhc2UtPm5hbWUpOwo+IC0KPiAgICAgICAgIGlmIChzdWl0ZS0+aW5pdCkgewo+ICAg ICAgICAgICAgICAgICBpbnQgcmV0Owo+ICAKPiAtICAgICAgICAgICAgICAgcmV0ID0gc3VpdGUt PmluaXQoJnRlc3QpOwo+ICsgICAgICAgICAgICAgICByZXQgPSBzdWl0ZS0+aW5pdCh0ZXN0KTsK PiAgICAgICAgICAgICAgICAgaWYgKHJldCkgewo+IC0gICAgICAgICAgICAgICAgICAgICAgIGt1 bml0X2VycigmdGVzdCwgImZhaWxlZCB0byBpbml0aWFsaXplOiAlZFxuIiwgcmV0KTsKPiAtICAg ICAgICAgICAgICAgICAgICAgICBrdW5pdF9zZXRfZmFpbHVyZSgmdGVzdCk7Cj4gLSAgICAgICAg ICAgICAgICAgICAgICAgdGVzdF9jYXNlLT5zdWNjZXNzID0gdGVzdC5zdWNjZXNzOwo+ICsgICAg ICAgICAgICAgICAgICAgICAgIGt1bml0X2Vycih0ZXN0LCAiZmFpbGVkIHRvIGluaXRpYWxpemU6 ICVkXG4iLCByZXQpOwo+ICsgICAgICAgICAgICAgICAgICAgICAgIGt1bml0X3NldF9mYWlsdXJl KHRlc3QpOwo+ICAgICAgICAgICAgICAgICAgICAgICAgIHJldHVybjsKPiAgICAgICAgICAgICAg ICAgfQo+ICAgICAgICAgfQo+ICAKPiAtICAgICAgIHRlc3RfY2FzZS0+cnVuX2Nhc2UoJnRlc3Qp Owo+ICsgICAgICAgdGVzdF9jYXNlLT5ydW5fY2FzZSh0ZXN0KTsKPiArfQo+ICsKPiArc3RhdGlj IHZvaWQga3VuaXRfY2FzZV9pbnRlcm5hbF9jbGVhbnVwKHN0cnVjdCBrdW5pdCAqdGVzdCkKPiAr ewo+ICsgICAgICAga3VuaXRfY2xlYW51cCh0ZXN0KTsKPiArfQo+ICAKPiArLyoKPiArICogUGVy Zm9ybXMgcG9zdCB2YWxpZGF0aW9ucyBhbmQgY2xlYW51cCBhZnRlciBhIHRlc3QgY2FzZSB3YXMg cnVuLgo+ICsgKiBYWFg6IFNob3VsZCBPTkxZIEJFIENBTExFRCBBRlRFUiBrdW5pdF9ydW5fY2Fz ZV9pbnRlcm5hbCEKPiArICovCj4gK3N0YXRpYyB2b2lkIGt1bml0X3J1bl9jYXNlX2NsZWFudXAo c3RydWN0IGt1bml0ICp0ZXN0LAo+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg c3RydWN0IGt1bml0X3N1aXRlICpzdWl0ZSkKPiArewo+ICAgICAgICAgaWYgKHN1aXRlLT5leGl0 KQo+IC0gICAgICAgICAgICAgICBzdWl0ZS0+ZXhpdCgmdGVzdCk7Cj4gKyAgICAgICAgICAgICAg IHN1aXRlLT5leGl0KHRlc3QpOwo+ICsKPiArICAgICAgIGt1bml0X2Nhc2VfaW50ZXJuYWxfY2xl YW51cCh0ZXN0KTsKPiArfQo+ICsKPiArLyoKPiArICogSGFuZGxlcyBhbiB1bmV4cGVjdGVkIGNy YXNoIGluIGEgdGVzdCBjYXNlLgo+ICsgKi8KPiArc3RhdGljIHZvaWQga3VuaXRfaGFuZGxlX3Rl c3RfY3Jhc2goc3RydWN0IGt1bml0ICp0ZXN0LAo+ICsgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgc3RydWN0IGt1bml0X3N1aXRlICpzdWl0ZSwKPiArICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgIHN0cnVjdCBrdW5pdF9jYXNlICp0ZXN0X2Nhc2UpCj4gK3sKPiArICAg ICAgIGt1bml0X2Vycih0ZXN0LCAia3VuaXQgdGVzdCBjYXNlIGNyYXNoZWQhIik7CgpEb2VzIHRo aXMgbmVlZCBhIG5ld2xpbmU/Cgo+ICsgICAgICAgLyoKPiArICAgICAgICAqIFRPRE8oYnJlbmRh bmhpZ2dpbnNAZ29vZ2xlLmNvbSk6IFRoaXMgcHJpbnRzIHRoZSBzdGFjayB0cmFjZSB1cAo+ICsg ICAgICAgICogdGhyb3VnaCB0aGlzIGZyYW1lLCBub3QgdXAgdG8gdGhlIGZyYW1lIHRoYXQgY2F1 c2VkIHRoZSBjcmFzaC4KPiArICAgICAgICAqLwo+ICsgICAgICAgc2hvd19zdGFjayhOVUxMLCBO VUxMKTsKPiArCj4gKyAgICAgICBrdW5pdF9jYXNlX2ludGVybmFsX2NsZWFudXAodGVzdCk7Cj4g K30KPiArCj4gK3N0cnVjdCBrdW5pdF90cnlfY2F0Y2hfY29udGV4dCB7Cj4gKyAgICAgICBzdHJ1 Y3Qga3VuaXQgKnRlc3Q7Cj4gKyAgICAgICBzdHJ1Y3Qga3VuaXRfc3VpdGUgKnN1aXRlOwo+ICsg ICAgICAgc3RydWN0IGt1bml0X2Nhc2UgKnRlc3RfY2FzZTsKPiArfTsKPiArCj4gK3N0YXRpYyB2 b2lkIGt1bml0X3RyeV9ydW5fY2FzZSh2b2lkICpkYXRhKQo+ICt7Cj4gKyAgICAgICBzdHJ1Y3Qg a3VuaXRfdHJ5X2NhdGNoX2NvbnRleHQgKmN0eCA9IGRhdGE7Cj4gKyAgICAgICBzdHJ1Y3Qga3Vu aXQgKnRlc3QgPSBjdHgtPnRlc3Q7Cj4gKyAgICAgICBzdHJ1Y3Qga3VuaXRfc3VpdGUgKnN1aXRl ID0gY3R4LT5zdWl0ZTsKPiArICAgICAgIHN0cnVjdCBrdW5pdF9jYXNlICp0ZXN0X2Nhc2UgPSBj dHgtPnRlc3RfY2FzZTsKPiArCj4gKyAgICAgICAvKgo+ICsgICAgICAgICoga3VuaXRfcnVuX2Nh c2VfaW50ZXJuYWwgbWF5IGVuY291bnRlciBhIGZhdGFsIGVycm9yOyBpZiBpdCBkb2VzLAo+ICsg ICAgICAgICogYWJvcnQgd2lsbCBiZSBjYWxsZWQsIHRoaXMgdGhyZWFkIHdpbGwgZXhpdCwgYW5k IGZpbmFsbHkgdGhlIHBhcmVudAo+ICsgICAgICAgICogdGhyZWFkIHdpbGwgcmVzdW1lIGNvbnRy b2wgYW5kIGhhbmRsZSBhbnkgbmVjZXNzYXJ5IGNsZWFuIHVwLgo+ICsgICAgICAgICovCj4gKyAg ICAgICBrdW5pdF9ydW5fY2FzZV9pbnRlcm5hbCh0ZXN0LCBzdWl0ZSwgdGVzdF9jYXNlKTsKPiAr ICAgICAgIC8qIFRoaXMgbGluZSBtYXkgbmV2ZXIgYmUgcmVhY2hlZC4gKi8KPiArICAgICAgIGt1 bml0X3J1bl9jYXNlX2NsZWFudXAodGVzdCwgc3VpdGUpOwo+ICt9Cj4gKwo+ICtzdGF0aWMgdm9p ZCBrdW5pdF9jYXRjaF9ydW5fY2FzZSh2b2lkICpkYXRhKQo+ICt7Cj4gKyAgICAgICBzdHJ1Y3Qg a3VuaXRfdHJ5X2NhdGNoX2NvbnRleHQgKmN0eCA9IGRhdGE7Cj4gKyAgICAgICBzdHJ1Y3Qga3Vu aXQgKnRlc3QgPSBjdHgtPnRlc3Q7Cj4gKyAgICAgICBzdHJ1Y3Qga3VuaXRfc3VpdGUgKnN1aXRl ID0gY3R4LT5zdWl0ZTsKPiArICAgICAgIHN0cnVjdCBrdW5pdF9jYXNlICp0ZXN0X2Nhc2UgPSBj dHgtPnRlc3RfY2FzZTsKPiArICAgICAgIGludCB0cnlfZXhpdF9jb2RlID0ga3VuaXRfdHJ5X2Nh dGNoX2dldF9yZXN1bHQoJnRlc3QtPnRyeV9jYXRjaCk7Cj4gKwo+ICsgICAgICAgaWYgKHRyeV9l eGl0X2NvZGUpIHsKPiArICAgICAgICAgICAgICAga3VuaXRfc2V0X2ZhaWx1cmUodGVzdCk7Cj4g KyAgICAgICAgICAgICAgIC8qCj4gKyAgICAgICAgICAgICAgICAqIFRlc3QgY2FzZSBjb3VsZCBu b3QgZmluaXNoLCB3ZSBoYXZlIG5vIGlkZWEgd2hhdCBzdGF0ZSBpdCBpcwo+ICsgICAgICAgICAg ICAgICAgKiBpbiwgc28gZG9uJ3QgZG8gY2xlYW4gdXAuCj4gKyAgICAgICAgICAgICAgICAqLwo+ ICsgICAgICAgICAgICAgICBpZiAodHJ5X2V4aXRfY29kZSA9PSAtRVRJTUVET1VUKQo+ICsgICAg ICAgICAgICAgICAgICAgICAgIGt1bml0X2Vycih0ZXN0LCAidGVzdCBjYXNlIHRpbWVkIG91dFxu Iik7Cj4gKyAgICAgICAgICAgICAgIC8qCj4gKyAgICAgICAgICAgICAgICAqIFVua25vd24gaW50 ZXJuYWwgZXJyb3Igb2NjdXJyZWQgcHJldmVudGluZyB0ZXN0IGNhc2UgZnJvbQo+ICsgICAgICAg ICAgICAgICAgKiBydW5uaW5nLCBzbyB0aGVyZSBpcyBub3RoaW5nIHRvIGNsZWFuIHVwLgo+ICsg ICAgICAgICAgICAgICAgKi8KPiArICAgICAgICAgICAgICAgZWxzZQo+ICsgICAgICAgICAgICAg ICAgICAgICAgIGt1bml0X2Vycih0ZXN0LCAiaW50ZXJuYWwgZXJyb3Igb2NjdXJyZWQgcHJldmVu dGluZyB0ZXN0IGNhc2UgZnJvbSBydW5uaW5nOiAlZFxuIiwKPiArICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgdHJ5X2V4aXRfY29kZSk7CgpOaXRwaWNrOiBJIHdvdWxkIGFkZCBicmFj ZXMgaGVyZSBiZWNhdXNlIHlvdSBtYWtlIHRoZSBpZiBzdGF0ZW1lbnQgaW50bwptdWx0aS1saW5l IGFybXMgZm9yIGVhY2ggY2FzZS4KCj4gKyAgICAgICAgICAgICAgIHJldHVybjsKPiArICAgICAg IH0KPiArCj4gKyAgICAgICBpZiAoa3VuaXRfZ2V0X2RlYXRoX3Rlc3QodGVzdCkpIHsKPiArICAg ICAgICAgICAgICAgLyoKPiArICAgICAgICAgICAgICAgICogRVhQRUNURUQgREVBVEg6IGt1bml0 X3J1bl9jYXNlX2ludGVybmFsIGVuY291bnRlcmVkCj4gKyAgICAgICAgICAgICAgICAqIGFudGlj aXBhdGVkIGZhdGFsIGVycm9yLiBFdmVyeXRoaW5nIHNob3VsZCBiZSBpbiBhIHNhZmUKPiArICAg ICAgICAgICAgICAgICogc3RhdGUuCj4gKyAgICAgICAgICAgICAgICAqLwo+ICsgICAgICAgICAg ICAgICBrdW5pdF9ydW5fY2FzZV9jbGVhbnVwKHRlc3QsIHN1aXRlKTsKPiArICAgICAgIH0gZWxz ZSB7Cj4gKyAgICAgICAgICAgICAgIC8qCj4gKyAgICAgICAgICAgICAgICAqIFVORVhQRUNURUQg REVBVEg6IGt1bml0X3J1bl9jYXNlX2ludGVybmFsIGVuY291bnRlcmVkIGFuCj4gKyAgICAgICAg ICAgICAgICAqIHVuYW50aWNpcGF0ZWQgZmF0YWwgZXJyb3IuIFdlIGhhdmUgbm8gaWRlYSB3aGF0 IHRoZSBzdGF0ZSBvZgo+ICsgICAgICAgICAgICAgICAgKiB0aGUgdGVzdCBjYXNlIGlzIGluLgo+ ICsgICAgICAgICAgICAgICAgKi8KPiArICAgICAgICAgICAgICAga3VuaXRfaGFuZGxlX3Rlc3Rf Y3Jhc2godGVzdCwgc3VpdGUsIHRlc3RfY2FzZSk7Cj4gKyAgICAgICAgICAgICAgIGt1bml0X3Nl dF9mYWlsdXJlKHRlc3QpOwoKTGlrZSB3YXMgZG9uZSBoZXJlLgoKPiArICAgICAgIH0KPiArfQo+ ICsKPiArLyoKPiArICogUGVyZm9ybXMgYWxsIGxvZ2ljIHRvIHJ1biBhIHRlc3QgY2FzZS4gSXQg YWxzbyBjYXRjaGVzIG1vc3QgZXJyb3JzIHRoYXQKPiArICogb2NjdXJzIGluIGEgdGVzdCBjYXNl IGFuZCByZXBvcnRzIHRoZW0gYXMgZmFpbHVyZXMuCgpzL29jY3Vycy9vY2N1ci8KCj4gKyAqLwo+ ICtzdGF0aWMgdm9pZCBrdW5pdF9ydW5fY2FzZV9jYXRjaF9lcnJvcnMoc3RydWN0IGt1bml0X3N1 aXRlICpzdWl0ZSwKWy4uLl0KPiBkaWZmIC0tZ2l0IGEva3VuaXQvdHJ5LWNhdGNoLmMgYi9rdW5p dC90cnktY2F0Y2guYwo+IG5ldyBmaWxlIG1vZGUgMTAwNjQ0Cj4gaW5kZXggMDAwMDAwMDAwMDAw MC4uZGU1ODBmMDc0Mzg3Ygo+IC0tLSAvZGV2L251bGwKPiArKysgYi9rdW5pdC90cnktY2F0Y2gu Ywo+IEBAIC0wLDAgKzEsOTUgQEAKPiArLy8gU1BEWC1MaWNlbnNlLUlkZW50aWZpZXI6IEdQTC0y LjAKPiArLyoKPiArICogQW4gQVBJIHRvIGFsbG93IGEgZnVuY3Rpb24sIHRoYXQgbWF5IGZhaWws IHRvIGJlIGV4ZWN1dGVkLCBhbmQgcmVjb3ZlciBpbiBhCj4gKyAqIGNvbnRyb2xsZWQgbWFubmVy Lgo+ICsgKgo+ICsgKiBDb3B5cmlnaHQgKEMpIDIwMTksIEdvb2dsZSBMTEMuCj4gKyAqIEF1dGhv cjogQnJlbmRhbiBIaWdnaW5zIDxicmVuZGFuaGlnZ2luc0Bnb29nbGUuY29tPgo+ICsgKi8KPiAr Cj4gKyNpbmNsdWRlIDxrdW5pdC90cnktY2F0Y2guaD4KPiArI2luY2x1ZGUgPGt1bml0L3Rlc3Qu aD4KPiArI2luY2x1ZGUgPGxpbnV4L2NvbXBsZXRpb24uaD4KPiArI2luY2x1ZGUgPGxpbnV4L2t0 aHJlYWQuaD4KPiArCj4gK3ZvaWQgX19ub3JldHVybiBrdW5pdF90cnlfY2F0Y2hfdGhyb3coc3Ry dWN0IGt1bml0X3RyeV9jYXRjaCAqdHJ5X2NhdGNoKQo+ICt7Cj4gKyAgICAgICB0cnlfY2F0Y2gt PnRyeV9yZXN1bHQgPSAtRUZBVUxUOwo+ICsgICAgICAgY29tcGxldGVfYW5kX2V4aXQodHJ5X2Nh dGNoLT50cnlfY29tcGxldGlvbiwgLUVGQVVMVCk7Cj4gK30KPiArCj4gK3N0YXRpYyBpbnQga3Vu aXRfZ2VuZXJpY19ydW5fdGhyZWFkZm5fYWRhcHRlcih2b2lkICpkYXRhKQo+ICt7Cj4gKyAgICAg ICBzdHJ1Y3Qga3VuaXRfdHJ5X2NhdGNoICp0cnlfY2F0Y2ggPSBkYXRhOwo+ICsKPiArICAgICAg IHRyeV9jYXRjaC0+dHJ5KHRyeV9jYXRjaC0+Y29udGV4dCk7Cj4gKwo+ICsgICAgICAgY29tcGxl dGVfYW5kX2V4aXQodHJ5X2NhdGNoLT50cnlfY29tcGxldGlvbiwgMCk7Cj4gK30KPiArCj4gK3Zv aWQga3VuaXRfdHJ5X2NhdGNoX3J1bihzdHJ1Y3Qga3VuaXRfdHJ5X2NhdGNoICp0cnlfY2F0Y2gs IHZvaWQgKmNvbnRleHQpCj4gK3sKPiArICAgICAgIERFQ0xBUkVfQ09NUExFVElPTl9PTlNUQUNL KHRyeV9jb21wbGV0aW9uKTsKPiArICAgICAgIHN0cnVjdCBrdW5pdCAqdGVzdCA9IHRyeV9jYXRj aC0+dGVzdDsKPiArICAgICAgIHN0cnVjdCB0YXNrX3N0cnVjdCAqdGFza19zdHJ1Y3Q7Cj4gKyAg ICAgICBpbnQgZXhpdF9jb2RlLCBzdGF0dXM7Cj4gKwo+ICsgICAgICAgdHJ5X2NhdGNoLT5jb250 ZXh0ID0gY29udGV4dDsKPiArICAgICAgIHRyeV9jYXRjaC0+dHJ5X2NvbXBsZXRpb24gPSAmdHJ5 X2NvbXBsZXRpb247Cj4gKyAgICAgICB0cnlfY2F0Y2gtPnRyeV9yZXN1bHQgPSAwOwo+ICsgICAg ICAgdGFza19zdHJ1Y3QgPSBrdGhyZWFkX3J1bihrdW5pdF9nZW5lcmljX3J1bl90aHJlYWRmbl9h ZGFwdGVyLAo+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB0cnlfY2F0Y2gsCj4g KyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICJrdW5pdF90cnlfY2F0Y2hfdGhyZWFk Iik7Cj4gKyAgICAgICBpZiAoSVNfRVJSKHRhc2tfc3RydWN0KSkgewo+ICsgICAgICAgICAgICAg ICB0cnlfY2F0Y2gtPmNhdGNoKHRyeV9jYXRjaC0+Y29udGV4dCk7Cj4gKyAgICAgICAgICAgICAg IHJldHVybjsKPiArICAgICAgIH0KPiArCj4gKyAgICAgICAvKgo+ICsgICAgICAgICogVE9ETyhi cmVuZGFuaGlnZ2luc0Bnb29nbGUuY29tKTogV2Ugc2hvdWxkIHByb2JhYmx5IGhhdmUgc29tZSB0 eXBlIG9mCj4gKyAgICAgICAgKiB2YXJpYWJsZSB0aW1lb3V0IGhlcmUuIFRoZSBvbmx5IHF1ZXN0 aW9uIGlzIHdoYXQgdGhhdCB0aW1lb3V0IHZhbHVlCj4gKyAgICAgICAgKiBzaG91bGQgYmUuCj4g KyAgICAgICAgKgo+ICsgICAgICAgICogVGhlIGludGVudGlvbiBoYXMgYWx3YXlzIGJlZW4sIGF0 IHNvbWUgcG9pbnQsIHRvIGJlIGFibGUgdG8gbGFiZWwKPiArICAgICAgICAqIHRlc3RzIHdpdGgg c29tZSB0eXBlIG9mIHNpemUgYnVja2V0ICh1bml0L3NtYWxsLCBpbnRlZ3JhdGlvbi9tZWRpdW0s Cj4gKyAgICAgICAgKiBsYXJnZS9zeXN0ZW0vZW5kLXRvLWVuZCwgZXRjKSwgd2hlcmUgZWFjaCBz aXplIGJ1Y2tldCB3b3VsZCBnZXQgYQo+ICsgICAgICAgICogZGVmYXVsdCB0aW1lb3V0IHZhbHVl IGtpbmQgb2YgbGlrZSB3aGF0IEJhemVsIGRvZXM6Cj4gKyAgICAgICAgKiBodHRwczovL2RvY3Mu YmF6ZWwuYnVpbGQvdmVyc2lvbnMvbWFzdGVyL2JlL2NvbW1vbi1kZWZpbml0aW9ucy5odG1sI3Rl c3Quc2l6ZQo+ICsgICAgICAgICogVGhlcmUgaXMgc3RpbGwgc29tZSBkZWJhdGUgdG8gYmUgaGFk IG9uIGV4YWN0bHkgaG93IHdlIGRvIHRoaXMuIChGb3IKPiArICAgICAgICAqIG9uZSwgd2UgcHJv YmFibHkgd2FudCB0byBoYXZlIHNvbWUgc29ydCBvZiB0ZXN0IHJ1bm5lciBsZXZlbAo+ICsgICAg ICAgICogdGltZW91dC4pCj4gKyAgICAgICAgKgo+ICsgICAgICAgICogRm9yIG1vcmUgYmFja2dy b3VuZCBvbiB0aGlzIHRvcGljLCBzZWU6Cj4gKyAgICAgICAgKiBodHRwczovL21pa2UtYmxhbmQu Y29tLzIwMTEvMTEvMDEvc21hbGwtbWVkaXVtLWxhcmdlLmh0bWwKPiArICAgICAgICAqLwo+ICsg ICAgICAgc3RhdHVzID0gd2FpdF9mb3JfY29tcGxldGlvbl90aW1lb3V0KCZ0cnlfY29tcGxldGlv biwKPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAzMDAgKiBN U0VDX1BFUl9TRUMpOyAvKiA1IG1pbiAqLwo+ICsgICAgICAgaWYgKHN0YXR1cyA8IDApIHsKCndh aXRfZm9yX2NvbXBsZXRpb25fdGltZW91dCgpIGRvZXNuJ3QgcmV0dXJuIGEgbmVnYXRpdmUgdmFs dWUgb24KdGltZW91dC4gSXQgcmV0dXJucyAwLiBQbGVhc2UgcmVuYW1lICdzdGF0dXMnIHRvICd0 aW1lX3JlbWFpbmluZycgYW5kCnRlc3Qgd2l0aCBpZiAoIXRpbWVfcmVtYWluaW5nKSBpbnN0ZWFk IG9yIHNvbWUgb3RoZXIgc3VpdGFibHkgbmFtZWQKdmFyaWFibGUgbmFtZSBpbmRpY2F0aW5nIHRo YXQgdGhlIHJldHVybiB2YWx1ZSBpcyB0aGUgdGltZSByZW1haW5pbmcKYmVmb3JlIHRoZSB0aW1l b3V0LgoKTWF5IGFsc28gd2FudCB0byBjbGFtcCB0aGlzIHRvIHRoZSBodW5nIHRhc2sgdGltZW91 dCB2YWx1ZSwgd2hpY2ggaXMKdHlwaWNhbGx5IGxlc3MgdGhhbiA1IG1pbnV0ZXMuIE90aGVyd2lz ZSwgdGhlIGh1bmcgdGFzayBkZXRlY3RvciBtYXkKZmluZCB0aGUgcHJvYmxlbSBmaXJzdCBiZWZv cmUgdGhpcyB0aW1lb3V0IGhhcHBlbnMuCgo+ICsgICAgICAgICAgICAgICBrdW5pdF9lcnIodGVz dCwgInRyeSB0aW1lZCBvdXRcbiIpOwo+ICsgICAgICAgICAgICAgICB0cnlfY2F0Y2gtPnRyeV9y ZXN1bHQgPSAtRVRJTUVET1VUOwo+ICsgICAgICAgfQpfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBs aXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1h bi9saXN0aW5mby9kcmktZGV2ZWw= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1hxOJs-00027B-EJ for linux-um@lists.infradead.org; Tue, 13 Aug 2019 04:22:02 +0000 MIME-Version: 1.0 In-Reply-To: <20190812182421.141150-10-brendanhiggins@google.com> References: <20190812182421.141150-1-brendanhiggins@google.com> <20190812182421.141150-10-brendanhiggins@google.com> Subject: Re: [PATCH v12 09/18] kunit: test: add support for test abort From: Stephen Boyd Date: Mon, 12 Aug 2019 21:21:58 -0700 Message-Id: <20190813042159.46814206C2@mail.kernel.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Brendan Higgins , frowand.list@gmail.com, gregkh@linuxfoundation.org, jpoimboe@redhat.com, keescook@google.com, kieran.bingham@ideasonboard.com, mcgrof@kernel.org, peterz@infradead.org, robh@kernel.org, shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com Cc: pmladek@suse.com, linux-doc@vger.kernel.org, amir73il@gmail.com, Brendan Higgins , dri-devel@lists.freedesktop.org, Alexander.Levin@microsoft.com, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, khilman@baylibre.com, knut.omang@oracle.com, wfg@linux.intel.com, joel@jms.id.au, rientjes@google.com, jdike@addtoit.com, dan.carpenter@oracle.com, devicetree@vger.kernel.org, linux-kbuild@vger.kernel.org, Tim.Bird@sony.com, linux-um@lists.infradead.org, rostedt@goodmis.org, julia.lawall@lip6.fr, kunit-dev@googlegroups.com, richard@nod.at, rdunlap@infradead.org, linux-kernel@vger.kernel.org, daniel@ffwll.ch, mpe@ellerman.id.au, linux-fsdevel@vger.kernel.org, logang@deltatee.com Quoting Brendan Higgins (2019-08-12 11:24:12) > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 2625bcfeb19ac..93381f841e09f 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > struct kunit_resource; > > @@ -167,6 +168,7 @@ struct kunit { > > /* private: internal use only. */ > const char *name; /* Read only after initialization! */ > + struct kunit_try_catch try_catch; > /* > * success starts as true, and may only be set to false during a test > * case; thus, it is safe to update this across multiple threads using > @@ -176,6 +178,11 @@ struct kunit { > */ > bool success; /* Read only after test_case finishes! */ > spinlock_t lock; /* Gaurds all mutable test state. */ > + /* > + * death_test may be both set and unset from multiple threads in a test > + * case. > + */ > + bool death_test; /* Protected by lock. */ > /* > * Because resources is a list that may be updated multiple times (with > * new resources) from any thread associated with a test case, we must > @@ -184,6 +191,13 @@ struct kunit { > struct list_head resources; /* Protected by lock. */ > }; > > +static inline void kunit_set_death_test(struct kunit *test, bool death_test) > +{ > + spin_lock(&test->lock); > + test->death_test = death_test; > + spin_unlock(&test->lock); > +} These getters and setters are using spinlocks again. It doesn't make any sense. It probably needs a rework like was done for the other bool member, success. > + > void kunit_init_test(struct kunit *test, const char *name); > > int kunit_run_tests(struct kunit_suite *suite); > diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h > new file mode 100644 > index 0000000000000..8a414a9af0b64 > --- /dev/null > +++ b/include/kunit/try-catch.h > @@ -0,0 +1,69 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * An API to allow a function, that may fail, to be executed, and recover in a > + * controlled manner. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#ifndef _KUNIT_TRY_CATCH_H > +#define _KUNIT_TRY_CATCH_H > + > +#include > + > +typedef void (*kunit_try_catch_func_t)(void *); > + > +struct kunit; Forward declare struct completion? > + > +/* > + * struct kunit_try_catch - provides a generic way to run code which might fail. > + * @context: used to pass user data to the try and catch functions. > + * > + * kunit_try_catch provides a generic, architecture independent way to execute > + * an arbitrary function of type kunit_try_catch_func_t which may bail out by > + * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try > + * is stopped at the site of invocation and @catch is catch is called. > + * > + * struct kunit_try_catch provides a generic interface for the functionality > + * needed to implement kunit->abort() which in turn is needed for implementing > + * assertions. Assertions allow stating a precondition for a test simplifying > + * how test cases are written and presented. > + * > + * Assertions are like expectations, except they abort (call > + * kunit_try_catch_throw()) when the specified condition is not met. This is > + * useful when you look at a test case as a logical statement about some piece > + * of code, where assertions are the premises for the test case, and the > + * conclusion is a set of predicates, rather expectations, that must all be > + * true. If your premises are violated, it does not makes sense to continue. > + */ > +struct kunit_try_catch { > + /* private: internal use only. */ > + struct kunit *test; > + struct completion *try_completion; > + int try_result; > + kunit_try_catch_func_t try; > + kunit_try_catch_func_t catch; Can these other variables be documented in the kernel doc? And should context be marked as 'public'? > + void *context; > +}; > + > +void kunit_try_catch_init(struct kunit_try_catch *try_catch, > + struct kunit *test, > + kunit_try_catch_func_t try, > + kunit_try_catch_func_t catch); > + > +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context); > + > +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch); > + > +static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch) > +{ > + return try_catch->try_result; > +} > + > +/* > + * Exposed for testing only. Ugh that's sad. I hope we don't expose more functions just for testing in other cases. > + */ > +void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch); > + > +#endif /* _KUNIT_TRY_CATCH_H */ > diff --git a/kunit/test.c b/kunit/test.c > index e5080a2c6b29c..995cb53fe4ee9 100644 > --- a/kunit/test.c > +++ b/kunit/test.c > @@ -7,13 +7,26 @@ > */ > > #include > +#include > #include > +#include > > static void kunit_set_failure(struct kunit *test) > { > WRITE_ONCE(test->success, false); > } > > +static bool kunit_get_death_test(struct kunit *test) > +{ > + bool death_test; > + > + spin_lock(&test->lock); > + death_test = test->death_test; > + spin_unlock(&test->lock); > + > + return death_test; > +} > + > static int kunit_vprintk_emit(int level, const char *fmt, va_list args) > { > return vprintk_emit(0, level, NULL, 0, fmt, args); > @@ -158,6 +171,21 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert) > kunit_print_string_stream(test, stream); > } > > +void __noreturn kunit_abort(struct kunit *test) > +{ > + kunit_set_death_test(test, true); > + > + kunit_try_catch_throw(&test->try_catch); > + > + /* > + * Throw could not abort from test. > + * > + * XXX: we should never reach this line! As kunit_try_catch_throw is > + * marked __noreturn. > + */ > + WARN_ONCE(true, "Throw could not abort from test!\n"); Should this just be a BUG_ON? It's supposedly impossible. > +} > + > void kunit_do_assertion(struct kunit *test, > struct kunit_assert *assert, > bool pass, > @@ -176,6 +204,9 @@ void kunit_do_assertion(struct kunit *test, > kunit_fail(test, assert); > > va_end(args); > + > + if (assert->type == KUNIT_ASSERTION) > + kunit_abort(test); > } > > void kunit_init_test(struct kunit *test, const char *name) > @@ -184,36 +215,154 @@ void kunit_init_test(struct kunit *test, const char *name) > INIT_LIST_HEAD(&test->resources); > test->name = name; > test->success = true; > + test->death_test = false; > } > > /* > - * Performs all logic to run a test case. > + * Initializes and runs test case. Does not clean up or do post validations. > */ > -static void kunit_run_case(struct kunit_suite *suite, > - struct kunit_case *test_case) > +static void kunit_run_case_internal(struct kunit *test, > + struct kunit_suite *suite, > + struct kunit_case *test_case) > { > - struct kunit test; > - > - kunit_init_test(&test, test_case->name); > - > if (suite->init) { > int ret; > > - ret = suite->init(&test); > + ret = suite->init(test); > if (ret) { > - kunit_err(&test, "failed to initialize: %d\n", ret); > - kunit_set_failure(&test); > - test_case->success = test.success; > + kunit_err(test, "failed to initialize: %d\n", ret); > + kunit_set_failure(test); > return; > } > } > > - test_case->run_case(&test); > + test_case->run_case(test); > +} > + > +static void kunit_case_internal_cleanup(struct kunit *test) > +{ > + kunit_cleanup(test); > +} > > +/* > + * Performs post validations and cleanup after a test case was run. > + * XXX: Should ONLY BE CALLED AFTER kunit_run_case_internal! > + */ > +static void kunit_run_case_cleanup(struct kunit *test, > + struct kunit_suite *suite) > +{ > if (suite->exit) > - suite->exit(&test); > + suite->exit(test); > + > + kunit_case_internal_cleanup(test); > +} > + > +/* > + * Handles an unexpected crash in a test case. > + */ > +static void kunit_handle_test_crash(struct kunit *test, > + struct kunit_suite *suite, > + struct kunit_case *test_case) > +{ > + kunit_err(test, "kunit test case crashed!"); Does this need a newline? > + /* > + * TODO(brendanhiggins@google.com): This prints the stack trace up > + * through this frame, not up to the frame that caused the crash. > + */ > + show_stack(NULL, NULL); > + > + kunit_case_internal_cleanup(test); > +} > + > +struct kunit_try_catch_context { > + struct kunit *test; > + struct kunit_suite *suite; > + struct kunit_case *test_case; > +}; > + > +static void kunit_try_run_case(void *data) > +{ > + struct kunit_try_catch_context *ctx = data; > + struct kunit *test = ctx->test; > + struct kunit_suite *suite = ctx->suite; > + struct kunit_case *test_case = ctx->test_case; > + > + /* > + * kunit_run_case_internal may encounter a fatal error; if it does, > + * abort will be called, this thread will exit, and finally the parent > + * thread will resume control and handle any necessary clean up. > + */ > + kunit_run_case_internal(test, suite, test_case); > + /* This line may never be reached. */ > + kunit_run_case_cleanup(test, suite); > +} > + > +static void kunit_catch_run_case(void *data) > +{ > + struct kunit_try_catch_context *ctx = data; > + struct kunit *test = ctx->test; > + struct kunit_suite *suite = ctx->suite; > + struct kunit_case *test_case = ctx->test_case; > + int try_exit_code = kunit_try_catch_get_result(&test->try_catch); > + > + if (try_exit_code) { > + kunit_set_failure(test); > + /* > + * Test case could not finish, we have no idea what state it is > + * in, so don't do clean up. > + */ > + if (try_exit_code == -ETIMEDOUT) > + kunit_err(test, "test case timed out\n"); > + /* > + * Unknown internal error occurred preventing test case from > + * running, so there is nothing to clean up. > + */ > + else > + kunit_err(test, "internal error occurred preventing test case from running: %d\n", > + try_exit_code); Nitpick: I would add braces here because you make the if statement into multi-line arms for each case. > + return; > + } > + > + if (kunit_get_death_test(test)) { > + /* > + * EXPECTED DEATH: kunit_run_case_internal encountered > + * anticipated fatal error. Everything should be in a safe > + * state. > + */ > + kunit_run_case_cleanup(test, suite); > + } else { > + /* > + * UNEXPECTED DEATH: kunit_run_case_internal encountered an > + * unanticipated fatal error. We have no idea what the state of > + * the test case is in. > + */ > + kunit_handle_test_crash(test, suite, test_case); > + kunit_set_failure(test); Like was done here. > + } > +} > + > +/* > + * Performs all logic to run a test case. It also catches most errors that > + * occurs in a test case and reports them as failures. s/occurs/occur/ > + */ > +static void kunit_run_case_catch_errors(struct kunit_suite *suite, [...] > diff --git a/kunit/try-catch.c b/kunit/try-catch.c > new file mode 100644 > index 0000000000000..de580f074387b > --- /dev/null > +++ b/kunit/try-catch.c > @@ -0,0 +1,95 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * An API to allow a function, that may fail, to be executed, and recover in a > + * controlled manner. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#include > +#include > +#include > +#include > + > +void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch) > +{ > + try_catch->try_result = -EFAULT; > + complete_and_exit(try_catch->try_completion, -EFAULT); > +} > + > +static int kunit_generic_run_threadfn_adapter(void *data) > +{ > + struct kunit_try_catch *try_catch = data; > + > + try_catch->try(try_catch->context); > + > + complete_and_exit(try_catch->try_completion, 0); > +} > + > +void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context) > +{ > + DECLARE_COMPLETION_ONSTACK(try_completion); > + struct kunit *test = try_catch->test; > + struct task_struct *task_struct; > + int exit_code, status; > + > + try_catch->context = context; > + try_catch->try_completion = &try_completion; > + try_catch->try_result = 0; > + task_struct = kthread_run(kunit_generic_run_threadfn_adapter, > + try_catch, > + "kunit_try_catch_thread"); > + if (IS_ERR(task_struct)) { > + try_catch->catch(try_catch->context); > + return; > + } > + > + /* > + * TODO(brendanhiggins@google.com): We should probably have some type of > + * variable timeout here. The only question is what that timeout value > + * should be. > + * > + * The intention has always been, at some point, to be able to label > + * tests with some type of size bucket (unit/small, integration/medium, > + * large/system/end-to-end, etc), where each size bucket would get a > + * default timeout value kind of like what Bazel does: > + * https://docs.bazel.build/versions/master/be/common-definitions.html#test.size > + * There is still some debate to be had on exactly how we do this. (For > + * one, we probably want to have some sort of test runner level > + * timeout.) > + * > + * For more background on this topic, see: > + * https://mike-bland.com/2011/11/01/small-medium-large.html > + */ > + status = wait_for_completion_timeout(&try_completion, > + 300 * MSEC_PER_SEC); /* 5 min */ > + if (status < 0) { wait_for_completion_timeout() doesn't return a negative value on timeout. It returns 0. Please rename 'status' to 'time_remaining' and test with if (!time_remaining) instead or some other suitably named variable name indicating that the return value is the time remaining before the timeout. May also want to clamp this to the hung task timeout value, which is typically less than 5 minutes. Otherwise, the hung task detector may find the problem first before this timeout happens. > + kunit_err(test, "try timed out\n"); > + try_catch->try_result = -ETIMEDOUT; > + } _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um