All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c
@ 2022-03-26 16:31 kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-03-26 16:31 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 14558 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220326002013.483394-2-dlatypov@google.com>
References: <20220326002013.483394-2-dlatypov@google.com>
TO: Daniel Latypov <dlatypov@google.com>
TO: brendanhiggins(a)google.com
TO: davidgow(a)google.com
CC: linux-kernel(a)vger.kernel.org
CC: kunit-dev(a)googlegroups.com
CC: linux-kselftest(a)vger.kernel.org
CC: skhan(a)linuxfoundation.org
CC: Daniel Latypov <dlatypov@google.com>

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on b14ffae378aa1db993e62b01392e70d1e585fb23]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Latypov/kunit-split-resource-API-from-test-h-into-new-resource-h/20220326-082202
base:   b14ffae378aa1db993e62b01392e70d1e585fb23
:::::: branch date: 16 hours ago
:::::: commit date: 16 hours ago
config: riscv-randconfig-c006-20220324 (https://download.01.org/0day-ci/archive/20220327/202203270042.5RTZ2j0a-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/38eca10f33d99e00e81b999ebeff960bb7f2e77b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Daniel-Latypov/kunit-split-resource-API-from-test-h-into-new-resource-h/20220326-082202
        git checkout 38eca10f33d99e00e81b999ebeff960bb7f2e77b
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
                            ^~~
   crypto/drbg.c:1858:18: note: '?' condition is true
                   u32 cryptlen = min3(inlen, outlen, (u32)DRBG_OUTSCRATCHLEN);
                                  ^
   include/linux/minmax.h:60:23: note: expanded from macro 'min3'
   #define min3(x, y, z) min((typeof(x))min(x, y), z)
                         ^
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^
   include/linux/minmax.h:38:3: note: expanded from macro '__careful_cmp'
                   __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), op))
                   ^
   include/linux/minmax.h:33:3: note: expanded from macro '__cmp_once'
                   __cmp(unique_x, unique_y, op); })
                   ^
   include/linux/minmax.h:28:26: note: expanded from macro '__cmp'
   #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                            ^
   crypto/drbg.c:1863:9: note: Calling 'crypto_wait_req'
                   ret = crypto_wait_req(crypto_skcipher_encrypt(drbg->ctr_req),
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/crypto.h:602:2: note: 'Default' branch taken. Execution continues on line 611
           switch (err) {
           ^
   include/linux/crypto.h:611:2: note: Returning value (loaded from 'err'), which participates in a condition later
           return err;
           ^~~~~~~~~~
   crypto/drbg.c:1863:9: note: Returning from 'crypto_wait_req'
                   ret = crypto_wait_req(crypto_skcipher_encrypt(drbg->ctr_req),
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   crypto/drbg.c:1865:7: note: Assuming 'ret' is 0
                   if (ret)
                       ^~~
   crypto/drbg.c:1865:3: note: Taking false branch
                   if (ret)
                   ^
   crypto/drbg.c:1870:3: note: Null pointer passed as 1st argument to memory copy function
                   memcpy(outbuf, drbg->outscratchpad, cryptlen);
                   ^      ~~~~~~
   Suppressed 13 warnings (6 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   5 warnings generated.
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   Suppressed 6 warnings (6 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
   include/linux/scatterlist.h:76:9: warning: Access to field 'page_link' results in a dereference of a null pointer (loaded from variable 'sg') [clang-analyzer-core.NullDereference]
           return sg->page_link & SG_PAGE_LINK_MASK;
                  ^
   lib/scatterlist.c:103:27: note: 'ret' initialized to a null pointer value
           struct scatterlist *sg, *ret = NULL;
                                    ^~~
   lib/scatterlist.c:106:30: note: Assuming 'i' is >= 'nents'
           for_each_sg(sgl, sg, nents, i)
                                       ^
   include/linux/scatterlist.h:169:31: note: expanded from macro 'for_each_sg'
           for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
                                        ^~~~~~~~~~
   lib/scatterlist.c:106:2: note: Loop condition is false. Execution continues on line 109
           for_each_sg(sgl, sg, nents, i)
           ^
   include/linux/scatterlist.h:169:2: note: expanded from macro 'for_each_sg'
           for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
           ^
   lib/scatterlist.c:109:21: note: Passing null pointer value via 1st parameter 'sg'
           BUG_ON(!sg_is_last(ret));
                              ^
   include/asm-generic/bug.h:161:45: note: expanded from macro 'BUG_ON'
   #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
                                               ^~~~~~~~~
   include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   lib/scatterlist.c:109:10: note: Calling 'sg_is_last'
           BUG_ON(!sg_is_last(ret));
                   ^
   include/asm-generic/bug.h:161:45: note: expanded from macro 'BUG_ON'
   #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
                                               ^~~~~~~~~
   include/linux/compiler.h:78:42: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                                               ^
   include/linux/scatterlist.h:91:20: note: Passing null pointer value via 1st parameter 'sg'
           return __sg_flags(sg) & SG_END;
                             ^~
   include/linux/scatterlist.h:91:9: note: Calling '__sg_flags'
           return __sg_flags(sg) & SG_END;
                  ^~~~~~~~~~~~~~
   include/linux/scatterlist.h:76:9: note: Access to field 'page_link' results in a dereference of a null pointer (loaded from variable 'sg')
           return sg->page_link & SG_PAGE_LINK_MASK;
                  ^~
   Suppressed 13 warnings (6 in non-user code, 7 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
>> lib/kunit/resource.c:121:2: warning: Use of memory after it is freed [clang-analyzer-unix.Malloc]
           kunit_put_resource(res);
           ^                  ~~~
   lib/kunit/resource.c:115:6: note: Assuming 'res' is non-null
           if (!res)
               ^~~~
   lib/kunit/resource.c:115:2: note: Taking false branch
           if (!res)
           ^
   lib/kunit/resource.c:118:2: note: Calling 'kunit_remove_resource'
           kunit_remove_resource(test, res);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   lib/kunit/resource.c:102:2: note: Loop condition is false.  Exiting loop
           spin_lock_irqsave(&test->lock, flags);
           ^
   include/linux/spinlock.h:379:2: note: expanded from macro 'spin_lock_irqsave'
           raw_spin_lock_irqsave(spinlock_check(lock), flags);     \
           ^
   include/linux/spinlock.h:240:2: note: expanded from macro 'raw_spin_lock_irqsave'
           do {                                            \
           ^
   lib/kunit/resource.c:102:2: note: Loop condition is false.  Exiting loop
           spin_lock_irqsave(&test->lock, flags);
           ^
   include/linux/spinlock.h:377:43: note: expanded from macro 'spin_lock_irqsave'
   #define spin_lock_irqsave(lock, flags)                          \
                                                                   ^
   lib/kunit/resource.c:105:2: note: Calling 'kunit_put_resource'
           kunit_put_resource(res);
           ^~~~~~~~~~~~~~~~~~~~~~~
   include/kunit/resource.h:142:2: note: Calling 'kref_put'
           kref_put(&res->refcount, kunit_release_resource);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/kref.h:64:6: note: Assuming the condition is true
           if (refcount_dec_and_test(&kref->refcount)) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/kref.h:64:2: note: Taking true branch
           if (refcount_dec_and_test(&kref->refcount)) {
           ^
   include/linux/kref.h:65:3: note: Calling 'kunit_release_resource'
                   release(kref);
                   ^~~~~~~~~~~~~
   include/kunit/resource.h:122:6: note: Assuming field 'free' is non-null
           if (res->free) {
               ^~~~~~~~~
   include/kunit/resource.h:122:2: note: Taking true branch
           if (res->free) {
           ^
   include/kunit/resource.h:124:3: note: Memory is released
                   kfree(res);
                   ^~~~~~~~~~
   include/linux/kref.h:65:3: note: Returning; memory was released
                   release(kref);
                   ^~~~~~~~~~~~~
   include/kunit/resource.h:142:2: note: Returning; memory was released
           kref_put(&res->refcount, kunit_release_resource);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   lib/kunit/resource.c:105:2: note: Returning; memory was released via 1st parameter
           kunit_put_resource(res);
           ^~~~~~~~~~~~~~~~~~~~~~~
   lib/kunit/resource.c:118:2: note: Returning; memory was released via 2nd parameter
           kunit_remove_resource(test, res);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   lib/kunit/resource.c:121:2: note: Use of memory after it is freed
           kunit_put_resource(res);
           ^                  ~~~
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   5 warnings generated.
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
   include/linux/mmzone.h:982:9: warning: Access to field 'pgdat' results in a dereference of a null pointer (loaded from variable 'lruvec') [clang-analyzer-core.NullDereference]
           return lruvec->pgdat;
                  ^
   mm/swap.c:741:2: note: Calling 'lru_add_and_bh_lrus_drain'
           lru_add_and_bh_lrus_drain();
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   mm/swap.c:721:2: note: Loop condition is false.  Exiting loop
           local_lock(&lru_pvecs.lock);
           ^
   include/linux/local_lock.h:16:27: note: expanded from macro 'local_lock'
   #define local_lock(lock)                __local_lock(lock)
                                           ^
   include/linux/local_lock_internal.h:67:3: note: expanded from macro '__local_lock'
                   preempt_disable();                              \
                   ^
   include/linux/preempt.h:201:27: note: expanded from macro 'preempt_disable'
   #define preempt_disable() \
                             ^
   mm/swap.c:721:2: note: Loop condition is false.  Exiting loop
           local_lock(&lru_pvecs.lock);
           ^
   include/linux/local_lock.h:16:27: note: expanded from macro 'local_lock'
   #define local_lock(lock)                __local_lock(lock)
                                           ^
   include/linux/local_lock_internal.h:68:22: note: expanded from macro '__local_lock'
                   local_lock_acquire(this_cpu_ptr(lock));         \
                                      ^
   include/linux/percpu-defs.h:252:27: note: expanded from macro 'this_cpu_ptr'
   #define this_cpu_ptr(ptr) raw_cpu_ptr(ptr)

vim +121 lib/kunit/resource.c

38eca10f33d99e0 Daniel Latypov 2022-03-25  108  
38eca10f33d99e0 Daniel Latypov 2022-03-25  109  int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
38eca10f33d99e0 Daniel Latypov 2022-03-25  110  			   void *match_data)
38eca10f33d99e0 Daniel Latypov 2022-03-25  111  {
38eca10f33d99e0 Daniel Latypov 2022-03-25  112  	struct kunit_resource *res = kunit_find_resource(test, match,
38eca10f33d99e0 Daniel Latypov 2022-03-25  113  							 match_data);
38eca10f33d99e0 Daniel Latypov 2022-03-25  114  
38eca10f33d99e0 Daniel Latypov 2022-03-25  115  	if (!res)
38eca10f33d99e0 Daniel Latypov 2022-03-25  116  		return -ENOENT;
38eca10f33d99e0 Daniel Latypov 2022-03-25  117  
38eca10f33d99e0 Daniel Latypov 2022-03-25  118  	kunit_remove_resource(test, res);
38eca10f33d99e0 Daniel Latypov 2022-03-25  119  
38eca10f33d99e0 Daniel Latypov 2022-03-25  120  	/* We have a reference also via _find(); drop it. */
38eca10f33d99e0 Daniel Latypov 2022-03-25 @121  	kunit_put_resource(res);

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c
  2022-03-26  4:27   ` David Gow
@ 2022-03-28 17:40     ` Daniel Latypov
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Latypov @ 2022-03-28 17:40 UTC (permalink / raw)
  To: David Gow
  Cc: Brendan Higgins, Linux Kernel Mailing List, KUnit Development,
	open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan

On Fri, Mar 25, 2022 at 11:27 PM David Gow <davidgow@google.com> wrote:
>
> On Sat, Mar 26, 2022 at 8:20 AM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > We've split out the declarations from include/kunit/test.h into
> > resource.h.
> > This patch splits out the definitions as well for consistency.
> >
> > A side effect of this is git blame won't properly track history by
> > default, users need to run
> > $ git blame -L ,1 -C13 lib/kunit/resource.c
> >
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > ---
>
> Looks good and works fine here. I'm going to try to rebase most of the
> other resource system stuff I'm working on on top of these (which will
> likely end up moving a bunch of code _again_, but is probably the
> least terrible of all the available options).
>
> One nitpick (newline at end of file) below, otherwise this is good.

Huh, I had thought checkpatch --strict would catch this.
I guess not.

Fixing this and sending out a v3.

>
> Reviewed-by: David Gow <davidgow@google.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c
  2022-03-26  0:20 ` [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c Daniel Latypov
  2022-03-26  4:27   ` David Gow
@ 2022-03-28 16:58   ` Brendan Higgins
  1 sibling, 0 replies; 5+ messages in thread
From: Brendan Higgins @ 2022-03-28 16:58 UTC (permalink / raw)
  To: Daniel Latypov; +Cc: davidgow, linux-kernel, kunit-dev, linux-kselftest, skhan

On Fri, Mar 25, 2022 at 8:20 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> We've split out the declarations from include/kunit/test.h into
> resource.h.
> This patch splits out the definitions as well for consistency.
>
> A side effect of this is git blame won't properly track history by
> default, users need to run
> $ git blame -L ,1 -C13 lib/kunit/resource.c
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c
  2022-03-26  0:20 ` [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c Daniel Latypov
@ 2022-03-26  4:27   ` David Gow
  2022-03-28 17:40     ` Daniel Latypov
  2022-03-28 16:58   ` Brendan Higgins
  1 sibling, 1 reply; 5+ messages in thread
From: David Gow @ 2022-03-26  4:27 UTC (permalink / raw)
  To: Daniel Latypov
  Cc: Brendan Higgins, Linux Kernel Mailing List, KUnit Development,
	open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan

On Sat, Mar 26, 2022 at 8:20 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> We've split out the declarations from include/kunit/test.h into
> resource.h.
> This patch splits out the definitions as well for consistency.
>
> A side effect of this is git blame won't properly track history by
> default, users need to run
> $ git blame -L ,1 -C13 lib/kunit/resource.c
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

Looks good and works fine here. I'm going to try to rebase most of the
other resource system stuff I'm working on on top of these (which will
likely end up moving a bunch of code _again_, but is probably the
least terrible of all the available options).

One nitpick (newline at end of file) below, otherwise this is good.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

>  lib/kunit/Makefile   |   1 +
>  lib/kunit/resource.c | 126 +++++++++++++++++++++++++++++++++++++++++++
>  lib/kunit/test.c     | 116 +--------------------------------------
>  3 files changed, 128 insertions(+), 115 deletions(-)
>  create mode 100644 lib/kunit/resource.c
>
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index c49f4ffb6273..29aff6562b42 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -1,6 +1,7 @@
>  obj-$(CONFIG_KUNIT) +=                 kunit.o
>
>  kunit-objs +=                          test.o \
> +                                       resource.o \
>                                         string-stream.o \
>                                         assert.o \
>                                         try-catch.o \
> diff --git a/lib/kunit/resource.c b/lib/kunit/resource.c
> new file mode 100644
> index 000000000000..b8bced246217
> --- /dev/null
> +++ b/lib/kunit/resource.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit resource API for test managed resources (allocations, etc.).
> + *
> + * Copyright (C) 2022, Google LLC.
> + * Author: Daniel Latypov <dlatypov@google.com>
> + */
> +
> +#include <kunit/resource.h>
> +#include <kunit/test.h>
> +#include <linux/kref.h>
> +
> +/*
> + * Used for static resources and when a kunit_resource * has been created by
> + * kunit_alloc_resource().  When an init function is supplied, @data is passed
> + * into the init function; otherwise, we simply set the resource data field to
> + * the data value passed in.
> + */
> +int kunit_add_resource(struct kunit *test,
> +                      kunit_resource_init_t init,
> +                      kunit_resource_free_t free,
> +                      struct kunit_resource *res,
> +                      void *data)
> +{
> +       int ret = 0;
> +       unsigned long flags;
> +
> +       res->free = free;
> +       kref_init(&res->refcount);
> +
> +       if (init) {
> +               ret = init(res, data);
> +               if (ret)
> +                       return ret;
> +       } else {
> +               res->data = data;
> +       }
> +
> +       spin_lock_irqsave(&test->lock, flags);
> +       list_add_tail(&res->node, &test->resources);
> +       /* refcount for list is established by kref_init() */
> +       spin_unlock_irqrestore(&test->lock, flags);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(kunit_add_resource);
> +
> +int kunit_add_named_resource(struct kunit *test,
> +                            kunit_resource_init_t init,
> +                            kunit_resource_free_t free,
> +                            struct kunit_resource *res,
> +                            const char *name,
> +                            void *data)
> +{
> +       struct kunit_resource *existing;
> +
> +       if (!name)
> +               return -EINVAL;
> +
> +       existing = kunit_find_named_resource(test, name);
> +       if (existing) {
> +               kunit_put_resource(existing);
> +               return -EEXIST;
> +       }
> +
> +       res->name = name;
> +
> +       return kunit_add_resource(test, init, free, res, data);
> +}
> +EXPORT_SYMBOL_GPL(kunit_add_named_resource);
> +
> +struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
> +                                                   kunit_resource_init_t init,
> +                                                   kunit_resource_free_t free,
> +                                                   gfp_t internal_gfp,
> +                                                   void *data)
> +{
> +       struct kunit_resource *res;
> +       int ret;
> +
> +       res = kzalloc(sizeof(*res), internal_gfp);
> +       if (!res)
> +               return NULL;
> +
> +       ret = kunit_add_resource(test, init, free, res, data);
> +       if (!ret) {
> +               /*
> +                * bump refcount for get; kunit_resource_put() should be called
> +                * when done.
> +                */
> +               kunit_get_resource(res);
> +               return res;
> +       }
> +       return NULL;
> +}
> +EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource);
> +
> +void kunit_remove_resource(struct kunit *test, struct kunit_resource *res)
> +{
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&test->lock, flags);
> +       list_del(&res->node);
> +       spin_unlock_irqrestore(&test->lock, flags);
> +       kunit_put_resource(res);
> +}
> +EXPORT_SYMBOL_GPL(kunit_remove_resource);
> +
> +int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
> +                          void *match_data)
> +{
> +       struct kunit_resource *res = kunit_find_resource(test, match,
> +                                                        match_data);
> +
> +       if (!res)
> +               return -ENOENT;
> +
> +       kunit_remove_resource(test, res);
> +
> +       /* We have a reference also via _find(); drop it. */
> +       kunit_put_resource(res);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(kunit_destroy_resource);
> +

.git/rebase-apply/patch:151: new blank line at EOF.

+

> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 3bca3bf5c15b..0f66c13d126e 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -6,10 +6,10 @@
>   * Author: Brendan Higgins <brendanhiggins@google.com>
>   */
>
> +#include <kunit/resource.h>
>  #include <kunit/test.h>
>  #include <kunit/test-bug.h>
>  #include <linux/kernel.h>
> -#include <linux/kref.h>
>  #include <linux/moduleparam.h>
>  #include <linux/sched/debug.h>
>  #include <linux/sched.h>
> @@ -592,120 +592,6 @@ void __kunit_test_suites_exit(struct kunit_suite **suites)
>  }
>  EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
>
> -/*
> - * Used for static resources and when a kunit_resource * has been created by
> - * kunit_alloc_resource().  When an init function is supplied, @data is passed
> - * into the init function; otherwise, we simply set the resource data field to
> - * the data value passed in.
> - */
> -int kunit_add_resource(struct kunit *test,
> -                      kunit_resource_init_t init,
> -                      kunit_resource_free_t free,
> -                      struct kunit_resource *res,
> -                      void *data)
> -{
> -       int ret = 0;
> -       unsigned long flags;
> -
> -       res->free = free;
> -       kref_init(&res->refcount);
> -
> -       if (init) {
> -               ret = init(res, data);
> -               if (ret)
> -                       return ret;
> -       } else {
> -               res->data = data;
> -       }
> -
> -       spin_lock_irqsave(&test->lock, flags);
> -       list_add_tail(&res->node, &test->resources);
> -       /* refcount for list is established by kref_init() */
> -       spin_unlock_irqrestore(&test->lock, flags);
> -
> -       return ret;
> -}
> -EXPORT_SYMBOL_GPL(kunit_add_resource);
> -
> -int kunit_add_named_resource(struct kunit *test,
> -                            kunit_resource_init_t init,
> -                            kunit_resource_free_t free,
> -                            struct kunit_resource *res,
> -                            const char *name,
> -                            void *data)
> -{
> -       struct kunit_resource *existing;
> -
> -       if (!name)
> -               return -EINVAL;
> -
> -       existing = kunit_find_named_resource(test, name);
> -       if (existing) {
> -               kunit_put_resource(existing);
> -               return -EEXIST;
> -       }
> -
> -       res->name = name;
> -
> -       return kunit_add_resource(test, init, free, res, data);
> -}
> -EXPORT_SYMBOL_GPL(kunit_add_named_resource);
> -
> -struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
> -                                                   kunit_resource_init_t init,
> -                                                   kunit_resource_free_t free,
> -                                                   gfp_t internal_gfp,
> -                                                   void *data)
> -{
> -       struct kunit_resource *res;
> -       int ret;
> -
> -       res = kzalloc(sizeof(*res), internal_gfp);
> -       if (!res)
> -               return NULL;
> -
> -       ret = kunit_add_resource(test, init, free, res, data);
> -       if (!ret) {
> -               /*
> -                * bump refcount for get; kunit_resource_put() should be called
> -                * when done.
> -                */
> -               kunit_get_resource(res);
> -               return res;
> -       }
> -       return NULL;
> -}
> -EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource);
> -
> -void kunit_remove_resource(struct kunit *test, struct kunit_resource *res)
> -{
> -       unsigned long flags;
> -
> -       spin_lock_irqsave(&test->lock, flags);
> -       list_del(&res->node);
> -       spin_unlock_irqrestore(&test->lock, flags);
> -       kunit_put_resource(res);
> -}
> -EXPORT_SYMBOL_GPL(kunit_remove_resource);
> -
> -int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
> -                          void *match_data)
> -{
> -       struct kunit_resource *res = kunit_find_resource(test, match,
> -                                                        match_data);
> -
> -       if (!res)
> -               return -ENOENT;
> -
> -       kunit_remove_resource(test, res);
> -
> -       /* We have a reference also via _find(); drop it. */
> -       kunit_put_resource(res);
> -
> -       return 0;
> -}
> -EXPORT_SYMBOL_GPL(kunit_destroy_resource);
> -
>  struct kunit_kmalloc_array_params {
>         size_t n;
>         size_t size;
> --
> 2.35.1.1021.g381101b075-goog
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c
  2022-03-26  0:20 [PATCH v2 1/2] kunit: split resource API from test.h into new resource.h Daniel Latypov
@ 2022-03-26  0:20 ` Daniel Latypov
  2022-03-26  4:27   ` David Gow
  2022-03-28 16:58   ` Brendan Higgins
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel Latypov @ 2022-03-26  0:20 UTC (permalink / raw)
  To: brendanhiggins, davidgow
  Cc: linux-kernel, kunit-dev, linux-kselftest, skhan, Daniel Latypov

We've split out the declarations from include/kunit/test.h into
resource.h.
This patch splits out the definitions as well for consistency.

A side effect of this is git blame won't properly track history by
default, users need to run
$ git blame -L ,1 -C13 lib/kunit/resource.c

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 lib/kunit/Makefile   |   1 +
 lib/kunit/resource.c | 126 +++++++++++++++++++++++++++++++++++++++++++
 lib/kunit/test.c     | 116 +--------------------------------------
 3 files changed, 128 insertions(+), 115 deletions(-)
 create mode 100644 lib/kunit/resource.c

diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index c49f4ffb6273..29aff6562b42 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_KUNIT) +=			kunit.o
 
 kunit-objs +=				test.o \
+					resource.o \
 					string-stream.o \
 					assert.o \
 					try-catch.o \
diff --git a/lib/kunit/resource.c b/lib/kunit/resource.c
new file mode 100644
index 000000000000..b8bced246217
--- /dev/null
+++ b/lib/kunit/resource.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit resource API for test managed resources (allocations, etc.).
+ *
+ * Copyright (C) 2022, Google LLC.
+ * Author: Daniel Latypov <dlatypov@google.com>
+ */
+
+#include <kunit/resource.h>
+#include <kunit/test.h>
+#include <linux/kref.h>
+
+/*
+ * Used for static resources and when a kunit_resource * has been created by
+ * kunit_alloc_resource().  When an init function is supplied, @data is passed
+ * into the init function; otherwise, we simply set the resource data field to
+ * the data value passed in.
+ */
+int kunit_add_resource(struct kunit *test,
+		       kunit_resource_init_t init,
+		       kunit_resource_free_t free,
+		       struct kunit_resource *res,
+		       void *data)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	res->free = free;
+	kref_init(&res->refcount);
+
+	if (init) {
+		ret = init(res, data);
+		if (ret)
+			return ret;
+	} else {
+		res->data = data;
+	}
+
+	spin_lock_irqsave(&test->lock, flags);
+	list_add_tail(&res->node, &test->resources);
+	/* refcount for list is established by kref_init() */
+	spin_unlock_irqrestore(&test->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kunit_add_resource);
+
+int kunit_add_named_resource(struct kunit *test,
+			     kunit_resource_init_t init,
+			     kunit_resource_free_t free,
+			     struct kunit_resource *res,
+			     const char *name,
+			     void *data)
+{
+	struct kunit_resource *existing;
+
+	if (!name)
+		return -EINVAL;
+
+	existing = kunit_find_named_resource(test, name);
+	if (existing) {
+		kunit_put_resource(existing);
+		return -EEXIST;
+	}
+
+	res->name = name;
+
+	return kunit_add_resource(test, init, free, res, data);
+}
+EXPORT_SYMBOL_GPL(kunit_add_named_resource);
+
+struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
+						    kunit_resource_init_t init,
+						    kunit_resource_free_t free,
+						    gfp_t internal_gfp,
+						    void *data)
+{
+	struct kunit_resource *res;
+	int ret;
+
+	res = kzalloc(sizeof(*res), internal_gfp);
+	if (!res)
+		return NULL;
+
+	ret = kunit_add_resource(test, init, free, res, data);
+	if (!ret) {
+		/*
+		 * bump refcount for get; kunit_resource_put() should be called
+		 * when done.
+		 */
+		kunit_get_resource(res);
+		return res;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource);
+
+void kunit_remove_resource(struct kunit *test, struct kunit_resource *res)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&test->lock, flags);
+	list_del(&res->node);
+	spin_unlock_irqrestore(&test->lock, flags);
+	kunit_put_resource(res);
+}
+EXPORT_SYMBOL_GPL(kunit_remove_resource);
+
+int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
+			   void *match_data)
+{
+	struct kunit_resource *res = kunit_find_resource(test, match,
+							 match_data);
+
+	if (!res)
+		return -ENOENT;
+
+	kunit_remove_resource(test, res);
+
+	/* We have a reference also via _find(); drop it. */
+	kunit_put_resource(res);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kunit_destroy_resource);
+
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 3bca3bf5c15b..0f66c13d126e 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -6,10 +6,10 @@
  * Author: Brendan Higgins <brendanhiggins@google.com>
  */
 
+#include <kunit/resource.h>
 #include <kunit/test.h>
 #include <kunit/test-bug.h>
 #include <linux/kernel.h>
-#include <linux/kref.h>
 #include <linux/moduleparam.h>
 #include <linux/sched/debug.h>
 #include <linux/sched.h>
@@ -592,120 +592,6 @@ void __kunit_test_suites_exit(struct kunit_suite **suites)
 }
 EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
 
-/*
- * Used for static resources and when a kunit_resource * has been created by
- * kunit_alloc_resource().  When an init function is supplied, @data is passed
- * into the init function; otherwise, we simply set the resource data field to
- * the data value passed in.
- */
-int kunit_add_resource(struct kunit *test,
-		       kunit_resource_init_t init,
-		       kunit_resource_free_t free,
-		       struct kunit_resource *res,
-		       void *data)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	res->free = free;
-	kref_init(&res->refcount);
-
-	if (init) {
-		ret = init(res, data);
-		if (ret)
-			return ret;
-	} else {
-		res->data = data;
-	}
-
-	spin_lock_irqsave(&test->lock, flags);
-	list_add_tail(&res->node, &test->resources);
-	/* refcount for list is established by kref_init() */
-	spin_unlock_irqrestore(&test->lock, flags);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(kunit_add_resource);
-
-int kunit_add_named_resource(struct kunit *test,
-			     kunit_resource_init_t init,
-			     kunit_resource_free_t free,
-			     struct kunit_resource *res,
-			     const char *name,
-			     void *data)
-{
-	struct kunit_resource *existing;
-
-	if (!name)
-		return -EINVAL;
-
-	existing = kunit_find_named_resource(test, name);
-	if (existing) {
-		kunit_put_resource(existing);
-		return -EEXIST;
-	}
-
-	res->name = name;
-
-	return kunit_add_resource(test, init, free, res, data);
-}
-EXPORT_SYMBOL_GPL(kunit_add_named_resource);
-
-struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
-						    kunit_resource_init_t init,
-						    kunit_resource_free_t free,
-						    gfp_t internal_gfp,
-						    void *data)
-{
-	struct kunit_resource *res;
-	int ret;
-
-	res = kzalloc(sizeof(*res), internal_gfp);
-	if (!res)
-		return NULL;
-
-	ret = kunit_add_resource(test, init, free, res, data);
-	if (!ret) {
-		/*
-		 * bump refcount for get; kunit_resource_put() should be called
-		 * when done.
-		 */
-		kunit_get_resource(res);
-		return res;
-	}
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource);
-
-void kunit_remove_resource(struct kunit *test, struct kunit_resource *res)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&test->lock, flags);
-	list_del(&res->node);
-	spin_unlock_irqrestore(&test->lock, flags);
-	kunit_put_resource(res);
-}
-EXPORT_SYMBOL_GPL(kunit_remove_resource);
-
-int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match,
-			   void *match_data)
-{
-	struct kunit_resource *res = kunit_find_resource(test, match,
-							 match_data);
-
-	if (!res)
-		return -ENOENT;
-
-	kunit_remove_resource(test, res);
-
-	/* We have a reference also via _find(); drop it. */
-	kunit_put_resource(res);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(kunit_destroy_resource);
-
 struct kunit_kmalloc_array_params {
 	size_t n;
 	size_t size;
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-28 17:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-26 16:31 [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2022-03-26  0:20 [PATCH v2 1/2] kunit: split resource API from test.h into new resource.h Daniel Latypov
2022-03-26  0:20 ` [PATCH v2 2/2] kunit: split resource API impl from test.c into new resource.c Daniel Latypov
2022-03-26  4:27   ` David Gow
2022-03-28 17:40     ` Daniel Latypov
2022-03-28 16:58   ` Brendan Higgins

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.