Received: by 2002:a25:f815:0:0:0:0:0 with SMTP id u21csp2850ybd; Tue, 25 Jun 2019 15:15:22 -0700 (PDT) X-Google-Smtp-Source: APXvYqwmNElCMtJBb8DBOWs9qlVdUYPEAQS0+7woaNzFpu6fyw6a06mMD6h0ClcJjZHVczTMpGy/ X-Received: by 2002:a17:902:8f93:: with SMTP id z19mr1010621plo.97.1561500922836; Tue, 25 Jun 2019 15:15:22 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1561500922; cv=none; d=google.com; s=arc-20160816; b=Kx+2/Pvuch5w9P94wn7UUac9fvz1h0ufvcpbW3gRuPd+oQVENuZK57Tj4Nl75Jlr0G rjZM4uKn8qc6C9M4Kfyy7U4zhD4nGUmDFG5R/fKK83RHUx7UoRcFs/st1aWPK7HbytOe LTaHWRXwDfd7AlHgw0ajtjZnDVBw4hQRb+JNM2zqGa+/PL0gr398QJVYfQ2KMG5zqsPd jweTnlaqugupBnBMFLegkuRhtZJf4ZnChPoEbMi2XLkUObNEgNXOhky9wUyGI0ySR08A hNSIpC7to65119uiD7DrwhAEUZOgwIg2bb1uNGp1avGWhDCZj3G7nXVMrIJ2Y44LoPnq Hixg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=3T/u7V091HbAklhSEBy2pvLlCGAKMdVNlTSclQyRnpc=; b=nZr2XS0+KK2Z9giKbK99dYIurFhY8jkppvS6bW+86pqlHWcrFH5xIdNe/8/mgbrRK+ bPup9p9/eayjgKvJff+5wzOQgWNPYaM6vMoui5CMrPrSv3plVcnBvXsIrmPvndL6wVQU AnRA5GjJErk3Ct1zd1g9oSfIzCAToutxgsXYYwNC1IJONrPahfehmNB3i8P6Ejg7nJ51 /PEMO+mN1jCULdX6Uixcb+EFjFLLDjPBZdp7E0DAOvW3x117yLTdrMysRH7qYH2G3wvC mEAWN90FlGGRrmrux2AAlMnG4PJAqcSXwd/ICWXybB0QGHnk7scyTQPYqKTtzm8EPMrm WLRA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=ZoosysAk; 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=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o127si14696167pga.593.2019.06.25.15.15.06; Tue, 25 Jun 2019 15:15:22 -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; dkim=pass header.i=@google.com header.s=20161025 header.b=ZoosysAk; 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=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726526AbfFYWO7 (ORCPT + 99 others); Tue, 25 Jun 2019 18:14:59 -0400 Received: from mail-pg1-f193.google.com ([209.85.215.193]:42653 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726468AbfFYWO6 (ORCPT ); Tue, 25 Jun 2019 18:14:58 -0400 Received: by mail-pg1-f193.google.com with SMTP id k13so116137pgq.9 for ; Tue, 25 Jun 2019 15:14:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3T/u7V091HbAklhSEBy2pvLlCGAKMdVNlTSclQyRnpc=; b=ZoosysAkLc7RgErWxK5FnvM9RfcOs6mWm5WOqtPjUqxIqHpReyhsVFvslRzLoV3q6k miAcaLs/6/zELponnm9xpGa9FZgG6JHCtwbwwbsHrYPAVLPTXKnsAGZekzo/1E+v3r7q MAyz+XT8oIYL+RBKU6gcMfBPkx0i9G9kx5C+icFSHWtjKBEQssSIkQPmyDMJkhDb/APn rRRqx5pehUt6wHDwbMbahfsOf2X1S8Q3jv4bjOdkgKsJGdYPvje9VOWjIuTF7Q/SCVfF wDn/9hiMVLqSYlkj9TG58JxRLAwgxPn7xvEpgJy0gDIbJSP6wZCqkWFGLqYmeLIB06YQ qR2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3T/u7V091HbAklhSEBy2pvLlCGAKMdVNlTSclQyRnpc=; b=KxqncYzeKlpF1nqV5RJQbG5AKK3qhPx8FRuEQrBcW44KISG6cf4XNAWjiLXA860VL0 V0uLBoMY60lYbujU3HkbHHiEuPB5iw9x8xQR/+wBCVuHKfPudX28KBkkuSBRoVIn9HDw U2gmD3Ma2CYAiw2z7Pq8H5CyKUKsBxpHaZqzefGjkcQr77Chy+F8SEnI5xMYVgza/WfK GsB8uWQ+FdUZDtfo8LfPD2W1Verr30q5pFUsIR70soOLysJBn9YPil6R9z2wnkATcO7s vmCxKVYdBTb4EdX+TVykOx3APrdXWlHqUVSk5ji0ATi8me0/V4GEsZ8CvA0WlYVZzJq5 dP7A== X-Gm-Message-State: APjAAAXUUF0CfGhDfLVVsmU1+76CHx9y4L61mHaIWAHIsZaATu6NSgO6 u0pkh2JlwN1/x8SkXca9pl3UaWA6bvvfnWKh4bHQ7Q== X-Received: by 2002:a63:1459:: with SMTP id 25mr40704314pgu.201.1561500896729; Tue, 25 Jun 2019 15:14:56 -0700 (PDT) MIME-Version: 1.0 References: <20190617082613.109131-1-brendanhiggins@google.com> <20190617082613.109131-2-brendanhiggins@google.com> <20190620001526.93426218BE@mail.kernel.org> <20190625214427.GN19023@42.do-not-panic.com> In-Reply-To: <20190625214427.GN19023@42.do-not-panic.com> From: Brendan Higgins Date: Tue, 25 Jun 2019 15:14:45 -0700 Message-ID: Subject: Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core To: Luis Chamberlain 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 Content-Type: text/plain; charset="UTF-8" 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 2:44 PM Luis Chamberlain wrote: > > 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. Yeah, I don't think it is worth it. I don't think we need to be squeezing every ounce of performance out of unit tests, since they are inherently a cost and are not intended to be run in a production deployed kernel as part of normal production usage. > > > 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. Fair enough, I can see the point of not wanting to use irq disabled until we get someone complaining about it, but I think making it thread safe is reasonable. It means there is one less thing to confuse a KUnit user and the only penalty paid is some very minor performance.