From patchwork Wed Apr 19 08:54:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gow X-Patchwork-Id: 13216530 X-Patchwork-Delegate: brendanhiggins@google.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 07A11C77B73 for ; Wed, 19 Apr 2023 08:54:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232319AbjDSIyf (ORCPT ); Wed, 19 Apr 2023 04:54:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59878 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231352AbjDSIye (ORCPT ); Wed, 19 Apr 2023 04:54:34 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E15D14C22 for ; Wed, 19 Apr 2023 01:54:32 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id t66-20020a254645000000b00b74680a7904so28605428yba.15 for ; Wed, 19 Apr 2023 01:54:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681894472; x=1684486472; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=EuxYuWlS9hmgllFa5Y4ge5RSLAeCGI0qG9xInGWh4Ro=; b=cE9nE1n72Kn5bHCXKXQlgGmbw3IOWLOSk6RamJ6u9TWsrspBENjtZ2S9Ieqe8Q4ywJ uWaDkNlrOmkvlOYLGvbbVD/XHQs/lU0IRkpm3hzdAzl6T7bJKezOXuGB7tjFKUaChprT i7uIiIaqkmcLmp8h0R4WgMRbsmkep7UwYv2JUMJP9UtaAjxSr9wFWI43ZNj0QFPBm/88 JptOcu+v+1GQzNmtHy+bzmoGJ4kQYrREcCAPChlII3ffhAPezmLWo3eT8zLmUwPPRG9S 3vYADKiWYdBcR4+mgpT+kZzoH5kBOKcxtjHnMjFjMLZVi9TOKZOOunG7xO1n+X2Hn143 /RWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681894472; x=1684486472; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=EuxYuWlS9hmgllFa5Y4ge5RSLAeCGI0qG9xInGWh4Ro=; b=fqXOmnpfgKLACfxpwgtoOjh28cuIKDHYXNvIVW70DeIdzrRi2zRJctLRZ6u7s8kn+e xJIEGhk+h5eEeIF+aETMt40FHgE0X1Sj4G+13avlXkWonWTj1YAzd7J29vJpnGWqlQ+n 8wwnx2XT9McvWh7EmFTq6wQqfg4U8r5y1pwIenJ+SlWZ6WDWhfOrDpe6JNELCtOiayB0 Hc9Q1BOxYAdJ713AYbWBQUCtMoatHiV+e5Qq+FWFD/vB5e9E3s6wJc94+JIKMYXW+XM8 1yyyPVd5WrNYmH+/k2ysM4K5hOqKxhUm8UsPzNpGer+X7wTZsmJULVDMR6pz24oD2C1/ 5dpA== X-Gm-Message-State: AAQBX9emA3L/nqhWsTVbSDUierFhLKRfklkUpYRW1Vxxaf8YZWbA5YHq jCoSVtuJGi2h9GX9AwT6jpc2jhBRUo0NxA== X-Google-Smtp-Source: AKy350YMVnCHPNkwtrZScJ/kXXz27Dk8pqoxWRoqrVGbkL/ZYtUR4Kdy+awK3eYEJ45ir+lIISXz2u9ULmrHqA== X-Received: from slicestar.c.googlers.com ([fda3:e722:ac3:cc00:4f:4b78:c0a8:20a1]) (user=davidgow job=sendgmr) by 2002:a25:d617:0:b0:b96:3344:c211 with SMTP id n23-20020a25d617000000b00b963344c211mr1700228ybg.10.1681894472175; Wed, 19 Apr 2023 01:54:32 -0700 (PDT) Date: Wed, 19 Apr 2023 16:54:24 +0800 Mime-Version: 1.0 X-Mailer: git-send-email 2.40.0.634.g4ca3ef3211-goog Message-ID: <20230419085426.1671703-1-davidgow@google.com> Subject: [PATCH v2 1/3] kunit: Always run cleanup from a test kthread From: David Gow To: Benjamin Berg , Brendan Higgins , Shuah Khan , Rae Moar , Daniel Latypov Cc: David Gow , maxime@cerno.tech, Stephen Boyd , kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Sadiya Kazi Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org KUnit tests run in a kthread, with the current->kunit_test pointer set to the test's context. This allows the kunit_get_current_test() and kunit_fail_current_test() macros to work. Normally, this pointer is still valid during test shutdown (i.e., the suite->exit function, and any resource cleanup). However, if the test has exited early (e.g., due to a failed assertion), the cleanup is done in the parent KUnit thread, which does not have an active context. Instead, in the event test terminates early, run the test exit and cleanup from a new 'cleanup' kthread, which sets current->kunit_test, and better isolates the rest of KUnit from issues which arise in test cleanup. If a test cleanup function itself aborts (e.g., due to an assertion failing), there will be no further attempts to clean up: an error will be logged and the test failed. This should also make it easier to get access to the KUnit context, particularly from within resource cleanup functions, which may, for example, need access to data in test->priv. Signed-off-by: David Gow --- This is an updated version of / replacement of "kunit: Set the current KUnit context when cleaning up", which instead creates a new kthread for cleanup tasks if the original test kthread is aborted. This protects us from failed assertions during cleanup, if the test exited early. Changes since v1: https://lore.kernel.org/linux-kselftest/20230415091401.681395-1-davidgow@google.com/ - Move cleanup execution to another kthread - (Thanks, Benjamin, for pointing out the assertion issues) --- lib/kunit/test.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/lib/kunit/test.c b/lib/kunit/test.c index e2910b261112..caeae0dfd82b 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -423,8 +423,51 @@ static void kunit_try_run_case(void *data) kunit_run_case_cleanup(test, suite); } +static void kunit_try_run_case_cleanup(void *data) +{ + struct kunit_try_catch_context *ctx = data; + struct kunit *test = ctx->test; + struct kunit_suite *suite = ctx->suite; + + current->kunit_test = test; + + kunit_run_case_cleanup(test, suite); +} + +static void kunit_catch_run_case_cleanup(void *data) +{ + struct kunit_try_catch_context *ctx = data; + struct kunit *test = ctx->test; + int try_exit_code = kunit_try_catch_get_result(&test->try_catch); + + /* It is always a failure if cleanup aborts. */ + kunit_set_failure(test); + + if (try_exit_code) { + /* + * 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 cleanup 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 during test case cleanup: %d\n", + try_exit_code); + } + return; + } + + kunit_err(test, "test aborted during cleanup. continuing without cleaning up\n"); +} + + static void kunit_catch_run_case(void *data) { + struct kunit_try_catch cleanup; struct kunit_try_catch_context *ctx = data; struct kunit *test = ctx->test; struct kunit_suite *suite = ctx->suite; @@ -451,9 +494,16 @@ static void kunit_catch_run_case(void *data) /* * Test case was run, but aborted. It is the test case's business as to - * whether it failed or not, we just need to clean up. + * whether it failed or not, we just need to clean up. Do this in a new + * try / catch context, in case it asserts, too. */ - kunit_run_case_cleanup(test, suite); + kunit_try_catch_init(&cleanup, + test, + kunit_try_run_case_cleanup, + kunit_catch_run_case_cleanup); + ctx->test = test; + ctx->suite = suite; + kunit_try_catch_run(&cleanup, ctx); } /* From patchwork Wed Apr 19 08:54:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gow X-Patchwork-Id: 13216531 X-Patchwork-Delegate: brendanhiggins@google.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2B4E9C6FD18 for ; Wed, 19 Apr 2023 08:54:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232594AbjDSIyn (ORCPT ); Wed, 19 Apr 2023 04:54:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59936 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232441AbjDSIyi (ORCPT ); Wed, 19 Apr 2023 04:54:38 -0400 Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 87F75B774 for ; Wed, 19 Apr 2023 01:54:37 -0700 (PDT) Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-54f8d4f1ca1so188667357b3.20 for ; Wed, 19 Apr 2023 01:54:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681894477; x=1684486477; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=dtKdQL060Yw/1K7e9ydb0dOQvyHGNeqc0Foxtu26QeI=; b=QGG9O8x8XvxtEgvh3W5bVpurqntEm7UdQlqHEtD69PL2bltf0siMC7wMCw41Dhj6P+ Jdn56bP9YkotX8mGPsIzmVX1U5PKJnCzv5qSADujeSG0lI9k+/k4y2Alj4jIIsrHmse3 QRCjhyky38iNE/EbTWQjA4xl/D66MIEMRxRZ8oMtV+0fHSiC41mDBGnwc9u6kvBbaMrb LRUBh0He2c5zp7+b4+ZQgRp5NiAeSXW1vpk6C/pHZYyEEXDpn8EmssMvq8u1rC7NuA7E ZVM1HZ6OWIicM9XxWcdqZmRlCucLoyreFT16rlK7wct6DtI7JMGryewkDf23mm0eFilk muYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681894477; x=1684486477; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=dtKdQL060Yw/1K7e9ydb0dOQvyHGNeqc0Foxtu26QeI=; b=Bh4w6BQ7P5PqQMOh3VvXn7p39wsZdgdfN+clH81j4N8MuoiojIt9t3RVbMfP3SLu8K zF/2UmcTcd8giH3G1TOB8n1bLoDi4rilVzHIBetw92Zb643/dhi6bENj/NMTjGd8/sAE jUDqbDbL4WmHXtbLHwyYYzJ0JXNWSULvQcfg0aB5GDXktmwrzXUFgq1TZ78UUpHAu4OL jW/8BfxqTBv9gdR9bVclRFWVJVrGtIdZlhBGeEBv4GseT5ed5V57z3HJkew/N95lVB4E 2cGR/zNPi8V8JW4yLezk/qjEiIMZU+Uu82CWDf+26t9LrQKoHpXij/oPvEJd/OBQ2ZOu 7pYA== X-Gm-Message-State: AAQBX9cFAEAgazMx1jKAiXQM38LipdHbifLocdelYw5PgUSKGH/rRoBS jMV+pv/UY9p3e9uvPorfzdWIlP5b9ViFHA== X-Google-Smtp-Source: AKy350b8nDZXE4llL9sEg/LKd73vVSzjIuT6+GWy/LxpPO0DKOI73YZ94zWlP31MS/VEPo5jDxFm6zk/tQd8mA== X-Received: from slicestar.c.googlers.com ([fda3:e722:ac3:cc00:4f:4b78:c0a8:20a1]) (user=davidgow job=sendgmr) by 2002:a81:764d:0:b0:52e:e095:d840 with SMTP id j13-20020a81764d000000b0052ee095d840mr1691645ywk.0.1681894476855; Wed, 19 Apr 2023 01:54:36 -0700 (PDT) Date: Wed, 19 Apr 2023 16:54:25 +0800 In-Reply-To: <20230419085426.1671703-1-davidgow@google.com> Mime-Version: 1.0 References: <20230419085426.1671703-1-davidgow@google.com> X-Mailer: git-send-email 2.40.0.634.g4ca3ef3211-goog Message-ID: <20230419085426.1671703-2-davidgow@google.com> Subject: [PATCH v2 2/3] Documentation: kunit: Note that assertions should not be used in cleanup From: David Gow To: Benjamin Berg , Brendan Higgins , Shuah Khan , Rae Moar , Daniel Latypov Cc: David Gow , maxime@cerno.tech, Stephen Boyd , kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Sadiya Kazi Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org As assertions abort the test cleanup process, they should be avoided from within a suite's exit function, or from within resource 'free' functions. Unlike with initialisation or main test execution, no further cleanup will be performed after a failed assertion, potentially causing a leak of resources. Signed-off-by: David Gow --- This patch is new in v2. --- Documentation/dev-tools/kunit/usage.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index 9faf2b4153fc..9f720f1317d3 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -121,6 +121,12 @@ there's an allocation error. ``return`` so they only work from the test function. In KUnit, we stop the current kthread on failure, so you can call them from anywhere. +.. note:: + Warning: There is an exception to the above rule. You shouldn't use assertions + in the suite's exit() function, or in the free function for a resource. These + run when a test is shutting down, and an assertion here prevents further + cleanup code from running, potentially leading to a memory leak. + Customizing error messages -------------------------- From patchwork Wed Apr 19 08:54:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gow X-Patchwork-Id: 13216532 X-Patchwork-Delegate: brendanhiggins@google.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9895DC6FD18 for ; Wed, 19 Apr 2023 08:54:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232629AbjDSIyq (ORCPT ); Wed, 19 Apr 2023 04:54:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60038 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232635AbjDSIyo (ORCPT ); Wed, 19 Apr 2023 04:54:44 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B3D9D975A for ; Wed, 19 Apr 2023 01:54:41 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id u188-20020a2560c5000000b00b8f15f2111dso22886297ybb.4 for ; Wed, 19 Apr 2023 01:54:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681894481; x=1684486481; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=Vsadv1gG3CDyEQYTrAe8ESn/fsRH88WCdjO7AUKZ6mI=; b=GC9NXcZveJJfKa28bmp/brjsU9DlnA7il8SpiO2vjXq165ktn7pY8f3a9gKPbQsYFh rDnG8lbt0KQq43V+J4LPoag8yycHse5JYdFnD8weeosCSO4NgPn8AqYkCyaOjQo43if4 jZKizq59s9pbd1YWcDuWl5ZDDxRXhnFugR8FX10BLK3Lp2MpdcR1AGFboL5FCERPVq9M 4I02vg7QmzhsO4hg289OEjfU06p+Nj4iWFx6N78jRj04NpkyeVaNsiI/kCmBfnaSWUq1 IyfTouUETKPnwJoLbNav6OfMwg0kGE0RWlia9a98mVGQAT3+y+EHbsxUjhU31y3JxkVQ rueA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681894481; x=1684486481; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Vsadv1gG3CDyEQYTrAe8ESn/fsRH88WCdjO7AUKZ6mI=; b=BhUdpi7YbUDjOOvFYoCIzPpYHZtf9UqDFpOxwLDnX5ePAHw7I+CR5rW7yEno6OkoM+ PsqymM21XxSDHbxxBq5wrRY/b/ZV1dfwc5ZCwBqk33bwfoVOKcdpN0YKUETcoIhCsfhK snIiU/6TNmmnnVWBo4ODta8DsJctsTXTuJGt9jjctRdnar5Ipekb4eGUWV1UYcrdWVEL VVrT70NVi7hDEy0XtM5TspcYXi8uTZ5VnDGk0FLUhMjJ9JJpekW2n/ZNCP1tOwba5WeX UHm0y4Yt0eF2o90I25A0GORGXysNwu+p3VCJK2FMQVWK3kQdEPPFy3eVpXYSZxDW7nGq yfyQ== X-Gm-Message-State: AAQBX9eCqbWyEnViDFmQtTkga4qTzIOXuM38wDOthyeXLo476sSubytv hayobcr9XUeLkMZFlnzLJ+rxHaSlgeyuEg== X-Google-Smtp-Source: AKy350Zl269OSH2BlaSwkyOfBXt7dTNFIcXx9j9QAC6LHOPv8qed2YPjjSQLXFLl+hSKzsH/ymalqQc95dwvyA== X-Received: from slicestar.c.googlers.com ([fda3:e722:ac3:cc00:4f:4b78:c0a8:20a1]) (user=davidgow job=sendgmr) by 2002:a81:9f01:0:b0:545:1d7f:acbf with SMTP id s1-20020a819f01000000b005451d7facbfmr1501533ywn.10.1681894481486; Wed, 19 Apr 2023 01:54:41 -0700 (PDT) Date: Wed, 19 Apr 2023 16:54:26 +0800 In-Reply-To: <20230419085426.1671703-1-davidgow@google.com> Mime-Version: 1.0 References: <20230419085426.1671703-1-davidgow@google.com> X-Mailer: git-send-email 2.40.0.634.g4ca3ef3211-goog Message-ID: <20230419085426.1671703-3-davidgow@google.com> Subject: [PATCH v2 3/3] Documentation: kunit: Warn that exit functions run even if init fails From: David Gow To: Benjamin Berg , Brendan Higgins , Shuah Khan , Rae Moar , Daniel Latypov Cc: David Gow , maxime@cerno.tech, Stephen Boyd , kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Sadiya Kazi Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org KUnit's exit functions will run even if the corresponding init function fails. It's easy, when writing an exit function, to assume the init function succeeded, and (for example) access uninitialised memory or dereference NULL pointers. Note that this case exists and should be handled in the documentation. Suggested-by: Benjamin Berg Link: https://lore.kernel.org/linux-kselftest/a39af0400abedb2e9b31d84c37551cecc3eed0e1.camel@sipsolutions.net/ Signed-off-by: David Gow --- This patch is new in v2. --- Documentation/dev-tools/kunit/usage.rst | 12 ++++++++++-- include/kunit/test.h | 3 +++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index 9f720f1317d3..f6d6c9a9ff54 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -166,7 +166,12 @@ many similar tests. In order to reduce duplication in these closely related tests, most unit testing frameworks (including KUnit) provide the concept of a *test suite*. A test suite is a collection of test cases for a unit of code with optional setup and teardown functions that run before/after the whole -suite and/or every test case. For example: +suite and/or every test case. + +.. note:: + A test case will only run if it is associated with a test suite. + +For example: .. code-block:: c @@ -196,7 +201,10 @@ after everything else. ``kunit_test_suite(example_test_suite)`` registers the test suite with the KUnit test framework. .. note:: - A test case will only run if it is associated with a test suite. + The ``exit`` and ``suite_exit`` functions will run even if ``init`` or + ``suite_init`` fail. Make sure that they can handle any inconsistent + state which may result from ``init`` or ``suite_init`` encoutering errors + or exiting early. ``kunit_test_suite(...)`` is a macro which tells the linker to put the specified test suite in a special linker section so that it can be run by KUnit diff --git a/include/kunit/test.h b/include/kunit/test.h index 57b309c6ca27..3028a1a3fcad 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -168,6 +168,9 @@ static inline char *kunit_status_to_ok_not_ok(enum kunit_status status) * test case, similar to the notion of a *test fixture* or a *test class* * in other unit testing frameworks like JUnit or Googletest. * + * Note that @exit and @suite_exit will run even if @init or @suite_init + * fail: make sure they can handle any inconsistent state which may result. + * * Every &struct kunit_case must be associated with a kunit_suite for KUnit * to run it. */