From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3B42C7618F for ; Mon, 15 Jul 2019 20:30:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7220220659 for ; Mon, 15 Jul 2019 20:30:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="nmdIrQ2Z" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732159AbfGOUah (ORCPT ); Mon, 15 Jul 2019 16:30:37 -0400 Received: from mail-pg1-f193.google.com ([209.85.215.193]:39759 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731677AbfGOUaf (ORCPT ); Mon, 15 Jul 2019 16:30:35 -0400 Received: by mail-pg1-f193.google.com with SMTP id u17so8261900pgi.6 for ; Mon, 15 Jul 2019 13:30:34 -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=Guk3L6+8PLdqP7pYPvzhoPuDjq05e+2mmqgYxycfTpo=; b=nmdIrQ2ZG9zLY+uPnHaC+zU9b/FYlCcJUklqPlgIEvxvjrncOHXB+C4uonVCMPNT5U RDlsPPFfFHanzQy12v9LKBzsgYs//6dy4qqJq9wWWyI4pDK3aCgJ1J/JgmQXwTE7LK3E iw5kJfsnm+yh+B2qKZbTQBSYW2yNi7ZyoDweAfKbzMv3dfSuT0useTynnAjypl5lf7/d jTEh9cq2DJ6lxhwAR1P3fF5Tm12yriac2Lepz1S6WLSn9ipeqXXdzi8KqMKw91pnk6DG ICc14afUNRCpD25tMKIn+kqhek6winXiVWbGXLe13YMzt6iQrEchkA/y8vKWAxCaJIaL ulxg== 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=Guk3L6+8PLdqP7pYPvzhoPuDjq05e+2mmqgYxycfTpo=; b=HlkGpnaQaSo73TLX2+lfv1SF1yNJ8VanM5EhVV+QTeAoEWN5VVH+gmgLEZvZZbCsAa l+iE8NB25pisvkPp5Et466ndD84Ou+Iki0HZaQPLc0vuOnGFqmWeL/VVIy4g2/4g9woS uQJ3WL7h77A/IN5cDQFvvSeifJyfkW3VvC4qpcnMK+4FLtwaChb525aZjovgqr8mC3Lx KR2IvCOew5e4W6K7Cklj0GGEkcb7SMKP00VvXIdErmD0LyCRrkrbNqr4SkXeOE/8QkvC 1gSvxAzYwEFs3HCXORbIyvK8jNAagaUI0rZO+viB6oLYzLDS2es8LXWhbuALFCumkSxl Bbgg== X-Gm-Message-State: APjAAAWFdJMnNxd65Q2Q5nBISn2c7o9CuNF9XJoN2mIHCRxmpEXqybkJ eHLQht/3X69v/xdLrMXFAhHJryIVZOj+dg8XGnVJRA== X-Google-Smtp-Source: APXvYqzKQuaXpGEOS+dMEmBo8FPFiuVIjj5hhYTkizz5+oKRzaBKQU1vUVyuyXBWxZXEb/U0Rw/0GC/2FiD36LZR+90= X-Received: by 2002:a63:b919:: with SMTP id z25mr28642556pge.201.1563222633591; Mon, 15 Jul 2019 13:30:33 -0700 (PDT) MIME-Version: 1.0 References: <20190712081744.87097-1-brendanhiggins@google.com> <20190712081744.87097-3-brendanhiggins@google.com> <20190715202425.CE64C20665@mail.kernel.org> In-Reply-To: <20190715202425.CE64C20665@mail.kernel.org> From: Brendan Higgins Date: Mon, 15 Jul 2019 13:30:22 -0700 Message-ID: Subject: Re: [PATCH v9 02/18] kunit: test: add test resource management API To: Stephen Boyd Cc: Frank Rowand , Greg KH , Josh Poimboeuf , Kees Cook , Kieran Bingham , Luis Chamberlain , 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 Mon, Jul 15, 2019 at 1:24 PM Stephen Boyd wrote: > > Quoting Brendan Higgins (2019-07-12 01:17:28) > > diff --git a/kunit/test.c b/kunit/test.c > > index 571e4c65deb5c..f165c9d8e10b0 100644 > > --- a/kunit/test.c > > +++ b/kunit/test.c > > @@ -171,6 +175,96 @@ int kunit_run_tests(struct kunit_suite *suite) > > return 0; > > } > > > > +struct kunit_resource *kunit_alloc_resource(struct kunit *test, > > + kunit_resource_init_t init, > > + kunit_resource_free_t free, > > + void *context) > > +{ > > + struct kunit_resource *res; > > + int ret; > > + > > + res = kzalloc(sizeof(*res), GFP_KERNEL); > > This uses GFP_KERNEL. > > > + if (!res) > > + return NULL; > > + > > + ret = init(res, context); > > + if (ret) > > + return NULL; > > + > > + res->free = free; > > + mutex_lock(&test->lock); > > And this can sleep. > > > + list_add_tail(&res->node, &test->resources); > > + mutex_unlock(&test->lock); > > + > > + return res; > > +} > > + > > +void kunit_free_resource(struct kunit *test, struct kunit_resource *res) > > Should probably add a note that we assume the test lock is held here, or > even add a lockdep_assert_held(&test->lock) into the function to > document that and assert it at the same time. Seems reasonable. > > +{ > > + res->free(res); > > + list_del(&res->node); > > + kfree(res); > > +} > > + > > +struct kunit_kmalloc_params { > > + size_t size; > > + gfp_t gfp; > > +}; > > + > > +static int kunit_kmalloc_init(struct kunit_resource *res, void *context) > > +{ > > + struct kunit_kmalloc_params *params = context; > > + > > + res->allocation = kmalloc(params->size, params->gfp); > > + if (!res->allocation) > > + return -ENOMEM; > > + > > + return 0; > > +} > > + > > +static void kunit_kmalloc_free(struct kunit_resource *res) > > +{ > > + kfree(res->allocation); > > +} > > + > > +void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) > > +{ > > + struct kunit_kmalloc_params params; > > + struct kunit_resource *res; > > + > > + params.size = size; > > + params.gfp = gfp; > > + > > + res = kunit_alloc_resource(test, > > This calls that sleeping function above... > > > + kunit_kmalloc_init, > > + kunit_kmalloc_free, > > + ¶ms); > > but this passes a GFP flags parameter through to the > kunit_kmalloc_init() function. How is this going to work if some code > uses GFP_ATOMIC, but then we try to allocate and sleep in > kunit_alloc_resource() with GFP_KERNEL? Yeah, that's an inconsistency. I need to fix that. > One solution would be to piggyback on all the existing devres allocation > logic we already have and make each struct kunit a device that we pass > into the devres functions. A far simpler solution would be to just > copy/paste what devres does and use a spinlock and an allocation > function that takes GFP flags. Yeah, that's what I did originally, but I thought from the discussion on patch 01 that you thought a spinlock was overkill for struct kunit. I take it you only meant in that initial patch? > > + > > + if (res) > > + return res->allocation; > > + > > + return NULL; > > +} Cheers