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=ham 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 4EBDA7D910 for ; Mon, 15 Jul 2019 20:24:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731924AbfGOUY1 (ORCPT ); Mon, 15 Jul 2019 16:24:27 -0400 Received: from mail.kernel.org ([198.145.29.99]:59174 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731055AbfGOUY1 (ORCPT ); Mon, 15 Jul 2019 16:24:27 -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 CE64C20665; Mon, 15 Jul 2019 20:24:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1563222265; bh=z/3S4xXt/Rio/ofj03vwkBIO5OR52dDSZ9Eeq6mO6XI=; h=In-Reply-To:References:From:To:Cc:Subject:Date:From; b=BXkGMbTaQqFfurRCy1g8au7AeTtgOFutLGJt3puQKKnveCArE1l5n+TvNB5nyLQN2 RewIrkkACOQLU4j8bdd/A1S3ea0t6aCgTcsSBEisWmgVGg22p/nmZ63GDKRcOxtMyZ 60uqJ7tvYGN5awKtcLQiKpJWBRakwwDtbA+09H2w= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20190712081744.87097-3-brendanhiggins@google.com> References: <20190712081744.87097-1-brendanhiggins@google.com> <20190712081744.87097-3-brendanhiggins@google.com> From: Stephen Boyd 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: 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 Subject: Re: [PATCH v9 02/18] kunit: test: add test resource management API User-Agent: alot/0.8.1 Date: Mon, 15 Jul 2019 13:24:25 -0700 Message-Id: <20190715202425.CE64C20665@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-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; > } > =20 > +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 =3D kzalloc(sizeof(*res), GFP_KERNEL); This uses GFP_KERNEL. > + if (!res) > + return NULL; > + > + ret =3D init(res, context); > + if (ret) > + return NULL; > + > + res->free =3D 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. > +{ > + 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 =3D context; > + > + res->allocation =3D 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 =3D size; > + params.gfp =3D gfp; > + > + res =3D 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?=20 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. > + > + if (res) > + return res->allocation; > + > + return NULL; > +} From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:59174 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731055AbfGOUY1 (ORCPT ); Mon, 15 Jul 2019 16:24:27 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20190712081744.87097-3-brendanhiggins@google.com> References: <20190712081744.87097-1-brendanhiggins@google.com> <20190712081744.87097-3-brendanhiggins@google.com> From: Stephen Boyd Subject: Re: [PATCH v9 02/18] kunit: test: add test resource management API Date: Mon, 15 Jul 2019 13:24:25 -0700 Message-Id: <20190715202425.CE64C20665@mail.kernel.org> Sender: linux-kbuild-owner@vger.kernel.org List-ID: 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: 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 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; > } > =20 > +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 =3D kzalloc(sizeof(*res), GFP_KERNEL); This uses GFP_KERNEL. > + if (!res) > + return NULL; > + > + ret =3D init(res, context); > + if (ret) > + return NULL; > + > + res->free =3D 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. > +{ > + 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 =3D context; > + > + res->allocation =3D 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 =3D size; > + params.gfp =3D gfp; > + > + res =3D 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?=20 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. > + > + if (res) > + return res->allocation; > + > + return NULL; > +} From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6D68721A070B8 for ; Mon, 15 Jul 2019 13:26:54 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20190712081744.87097-3-brendanhiggins@google.com> References: <20190712081744.87097-1-brendanhiggins@google.com> <20190712081744.87097-3-brendanhiggins@google.com> From: Stephen Boyd Subject: Re: [PATCH v9 02/18] kunit: test: add test resource management API Date: Mon, 15 Jul 2019 13:24:25 -0700 Message-Id: <20190715202425.CE64C20665@mail.kernel.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" 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, 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 List-ID: 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. > +{ > + 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? 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. > + > + if (res) > + return res->allocation; > + > + return NULL; > +} _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm 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 1hn7WM-00019x-Hn for linux-um@lists.infradead.org; Mon, 15 Jul 2019 20:24:27 +0000 MIME-Version: 1.0 In-Reply-To: <20190712081744.87097-3-brendanhiggins@google.com> References: <20190712081744.87097-1-brendanhiggins@google.com> <20190712081744.87097-3-brendanhiggins@google.com> From: Stephen Boyd Subject: Re: [PATCH v9 02/18] kunit: test: add test resource management API Date: Mon, 15 Jul 2019 13:24:25 -0700 Message-Id: <20190715202425.CE64C20665@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-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. > +{ > + 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? 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. > + > + if (res) > + return res->allocation; > + > + return NULL; > +} _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH v9 02/18] kunit: test: add test resource management API Date: Mon, 15 Jul 2019 13:24:25 -0700 Message-ID: <20190715202425.CE64C20665@mail.kernel.org> References: <20190712081744.87097-1-brendanhiggins@google.com> <20190712081744.87097-3-brendanhiggins@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190712081744.87097-3-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, jpoimboe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, kieran.bingham-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, shuah-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, tytso-3s7WtUTddSA@public.gmane.org, yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org Cc: pmladek-IBi9RG/b67k@public.gmane.org, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Brendan Higgins , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Alexander.Levin-0li6OtcxBFHby3iVrkZq2A@public.gmane.org, linux-kselftest-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org, knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, wfg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org, rientjes-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, jdike-OPE4K8JWMJJBDgjK7y7TUQ@public.gmane.org, dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kbuild-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Tim.Bird-7U/KSKJipcs@public.gmane.org, linux-um-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org, julia.lawall-L2FTfq7BK8M@public.gmane.org, kunit-dev-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org, richard-/L3Ra7n9ekc@public.gmane.org, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, daniel-/w4YWyX8dFk@public.gmane.org, mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org 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. > +{ > + 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? 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. > + > + if (res) > + return res->allocation; > + > + return NULL; > +}