Received: by 2002:a25:f815:0:0:0:0:0 with SMTP id u21csp4127224ybd; Tue, 25 Jun 2019 14:44:53 -0700 (PDT) X-Google-Smtp-Source: APXvYqy3G4BYS9K/dyF0xProTLxz/l2GcZrG5s8k6Z0aKPTjx8PmlksqrayLT9AvpOudXg8v23jp X-Received: by 2002:a63:e317:: with SMTP id f23mr3516388pgh.39.1561499092974; Tue, 25 Jun 2019 14:44:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1561499092; cv=none; d=google.com; s=arc-20160816; b=pqiRBHTXjd49Cf1IftF0YiwPB+BpC3HGaOn3xmJaNZfFgj13oBN6X/u6uZd8Vi7rl/ kRAUPODJrYfE+QxhyY0QNUeP2VLuFcA1SH5kez3M0Db4I0+X6xnYTL8asD+jdkGGaxN7 neWO0I32IW801vnBd1ygq/Dyp1sZBEIDHnW8zmNDikCMCFS36SPqCwAnDkMQ5W93Iw9o Q7+//B6hFIabk03vhQFkUkdIbi7FferESxyt1iWAQafM2bkwFFCspVoW4PcVCNF48phv vZk74jyORQyOVGNkXr5AtZTRWIfEbTbQstutTn0JZ+5ExEH9TK23fmYOltBtJoLOfJpr 8kHQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=9dxyIPqcP8jDSy0GUtYumC+DRq05muFlYTTdqsbRigE=; b=m1l3Wlxh7T5eKDgorDEHZK1j66cnbkupCw+yKIM1iWb3ybfgDrrFaEzeJp//eD4iBE liH+RMIp4tEC72sUgOV/RyiDdlKIYE8dFXrd8BwOom24lQ2UxV0wKSCmdRfn4itlnE7d hVn9iaC6uxf1hgJkVqH99SwB2ZB6ftoqHx6yB0s8VicmDbtyH10kdIUKpBsqkuMPbXp3 S1wWCk2qtEfvSy913Ct9VRycFSSBnZ03T50ewm7nIB+kTNGCqxgBKNR9vkd3PmQS6KVX pXa4D1njf+2CbjsrRI24o9od/Dd0flmnKOuWs/eaXWELvKGZ8eHUcOXWRPPTQ1JL7PO9 9CUA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id g22si15590245pfh.219.2019.06.25.14.44.35; Tue, 25 Jun 2019 14:44:52 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726396AbfFYVob (ORCPT + 99 others); Tue, 25 Jun 2019 17:44:31 -0400 Received: from mail-pg1-f195.google.com ([209.85.215.195]:42541 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726014AbfFYVob (ORCPT ); Tue, 25 Jun 2019 17:44:31 -0400 Received: by mail-pg1-f195.google.com with SMTP id k13so83720pgq.9; Tue, 25 Jun 2019 14:44:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=9dxyIPqcP8jDSy0GUtYumC+DRq05muFlYTTdqsbRigE=; b=Np3R7TUmILE29O//nVuQfb8yO1wVcahnUfTjdtvU2lq8kSBEy4G01ML0evh6hXqY3P JdJYiB/SOPSeyLonNoAeydK24aduweVZFxgapxiS1BvMs+R+ufmdNabKiXQ68OMlYl5R OiT0I+k75dkcqvFHgl3euEV/F5j4Xdr+yVOMXIfF4BAHPkwgtuyvuNjjYmr5zmfzJ+Wq KTfPIkrvbwNh+FOflVSPWWuqa6B/n5xZg6GgkyLRI/y5wjlFWE2NUEMER3MNI1vK4hhn ymJG85juyts/9Kt1otvUbLtOqZAzR/jnICFuI2BBq6jU/pHBZY8aw1LmhPwmRIBp/TL0 rD5g== X-Gm-Message-State: APjAAAX1NsvFzVybXC8wdG79ohNTc/zd7YpsIDRW+vpp1whgTT/IlNwj Q1NKZROB0q1QN6I6Tsw1un4= X-Received: by 2002:a63:7d18:: with SMTP id y24mr36133502pgc.438.1561499069415; Tue, 25 Jun 2019 14:44:29 -0700 (PDT) Received: from 42.do-not-panic.com (42.do-not-panic.com. [157.230.128.187]) by smtp.gmail.com with ESMTPSA id a20sm13423142pgb.72.2019.06.25.14.44.27 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Tue, 25 Jun 2019 14:44:27 -0700 (PDT) Received: by 42.do-not-panic.com (Postfix, from userid 1000) id 3C710401EB; Tue, 25 Jun 2019 21:44:27 +0000 (UTC) Date: Tue, 25 Jun 2019 21:44:27 +0000 From: Luis Chamberlain To: Brendan Higgins Cc: Stephen Boyd , Frank Rowand , Greg KH , Josh Poimboeuf , Kees Cook , Kieran Bingham , Peter Zijlstra , Rob Herring , shuah , Theodore Ts'o , Masahiro Yamada , devicetree , dri-devel , kunit-dev@googlegroups.com, "open list:DOCUMENTATION" , linux-fsdevel@vger.kernel.org, linux-kbuild , Linux Kernel Mailing List , "open list:KERNEL SELFTEST FRAMEWORK" , linux-nvdimm , linux-um@lists.infradead.org, Sasha Levin , "Bird, Timothy" , Amir Goldstein , Dan Carpenter , Daniel Vetter , Jeff Dike , Joel Stanley , Julia Lawall , Kevin Hilman , Knut Omang , Logan Gunthorpe , Michael Ellerman , Petr Mladek , Randy Dunlap , Richard Weinberger , David Rientjes , Steven Rostedt , wfg@linux.intel.com Subject: Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core Message-ID: <20190625214427.GN19023@42.do-not-panic.com> References: <20190617082613.109131-1-brendanhiggins@google.com> <20190617082613.109131-2-brendanhiggins@google.com> <20190620001526.93426218BE@mail.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 25, 2019 at 01:28:25PM -0700, Brendan Higgins wrote: > On Wed, Jun 19, 2019 at 5:15 PM Stephen Boyd wrote: > > > > Quoting Brendan Higgins (2019-06-17 01:25:56) > > > diff --git a/kunit/test.c b/kunit/test.c > > > new file mode 100644 > > > index 0000000000000..d05d254f1521f > > > --- /dev/null > > > +++ b/kunit/test.c > > > @@ -0,0 +1,210 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Base unit test (KUnit) API. > > > + * > > > + * Copyright (C) 2019, Google LLC. > > > + * Author: Brendan Higgins > > > + */ > > > + > > > +#include > > > +#include > > > + > > > +static bool kunit_get_success(struct kunit *test) > > > +{ > > > + unsigned long flags; > > > + bool success; > > > + > > > + spin_lock_irqsave(&test->lock, flags); > > > + success = test->success; > > > + spin_unlock_irqrestore(&test->lock, flags); > > > > I still don't understand the locking scheme in this code. Is the > > intention to make getter and setter APIs that are "safe" by adding in a > > spinlock that is held around getting and setting various members in the > > kunit structure? > > Yes, your understanding is correct. It is possible for a user to write > a test such that certain elements may be updated in different threads; > this would most likely happen in the case where someone wants to make > an assertion or an expectation in a thread created by a piece of code > under test. Although this should generally be avoided, it is possible, > and there are occasionally good reasons to do so, so it is > functionality that we should support. > > Do you think I should add a comment to this effect? > > > In what situation is there more than one thread reading or writing the > > kunit struct? Isn't it only a single process that is going to be > > As I said above, it is possible that the code under test may spawn a > new thread that may make an expectation or an assertion. It is not a > super common use case, but it is possible. I wonder if it is worth to have then different types of tests based on locking requirements. One with no locking, since it seems you imply most tests would fall under this category, then locking, and another with IRQ context. If no locking is done at all for all tests which do not require locking, is there any gains at run time? I'm sure it might be minimum but curious. > > operating on this structure? And why do we need to disable irqs? Are we > > expecting to be modifying the unit tests from irq contexts? > > There are instances where someone may want to test a driver which has > an interrupt handler in it. I actually have (not the greatest) example > here. Now in these cases, I expect someone to use a mock irqchip or > some other fake mechanism to trigger the interrupt handler and not > actual hardware; technically speaking in this case, it is not going to > be accessed from a "real" irq context; however, the code under test > should think that it is in an irq context; given that, I figured it is > best to just treat it as a real irq context. Does that make sense? Since its a new architecture and since you seem to imply most tests don't require locking or even IRQs disabled, I think its worth to consider the impact of adding such extreme locking requirements for an initial ramp up. Luis