lkml.org 
[lkml]   [2020]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3 kunit-next 1/2] kunit: generalize kunit_resource API beyond allocated resources
On Fri, Mar 27, 2020 at 5:45 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> In its original form, the kunit resources API - consisting the
> struct kunit_resource and associated functions - was focused on
> adding allocated resources during test operation that would be
> automatically cleaned up on test completion.
>
> The recent RFC patch proposing converting KASAN tests to KUnit [1]
> showed another potential model - where outside of test context,
> but with a pointer to the test state, we wish to access/update
> test-related data, but expressly want to avoid allocations.
>
> It turns out we can generalize the kunit_resource to support
> static resources where the struct kunit_resource * is passed
> in and initialized for us. As part of this work, we also
> change the "allocation" field to the more general "data" name,
> as instead of associating an allocation, we can associate a
> pointer to static data. Static data is distinguished by a NULL
> free functions. A test is added to cover using kunit_add_resource()
> with a static resource and data.
>
> Finally we also make use of the kernel's krefcount interfaces
> to manage reference counting of KUnit resources. The motivation
> for this is simple; if we have kernel threads accessing and
> using resources (say via kunit_find_resource()) we need to
> ensure we do not remove said resources (or indeed free them
> if they were dynamically allocated) until the reference count
> reaches zero. A new function - kunit_put_resource() - is
> added to handle this, and it should be called after a
> thread using kunit_find_resource() is finished with the
> retrieved resource.
>
> We ensure that the functions needed to look up, use and
> drop reference count are "static inline"-defined so that
> they can be used by builtin code as well as modules in
> the case that KUnit is built as a module.
>
> A cosmetic change here also; I've tried moving to
> kunit_[action]_resource() as the format of function names
> for consistency and readability.
>
> [1] https://lkml.org/lkml/2020/2/26/1286
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

One comment below, other than that:

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

Sorry for not keeping up with this. I forgot that I didn't give this a
reviewed-by.

> ---
> include/kunit/test.h | 157 +++++++++++++++++++++++++++++++++++++---------
> lib/kunit/kunit-test.c | 74 ++++++++++++++++------
> lib/kunit/string-stream.c | 14 ++---
> lib/kunit/test.c | 154 ++++++++++++++++++++++++---------------------
> 4 files changed, 270 insertions(+), 129 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 9b0c46a..8c7f3ff 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -52,30 +59,27 @@
> *
> * static void kunit_kmalloc_free(struct kunit_resource *res)
> * {
> - * kfree(res->allocation);
> + * kfree(res->data);
> * }
> *
> * 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, kunit_kmalloc_init,
> + * return kunit_alloc_resource(test, kunit_kmalloc_init,
> * kunit_kmalloc_free, &params);
> - * if (res)
> - * return res->allocation;
> - *
> - * return NULL;
> * }
> */
> struct kunit_resource {
> - void *allocation;
> - kunit_resource_free_t free;
> + void *data;
>
> /* private: internal use only. */
> + kunit_resource_init_t init;

Apologies for bringing this up so late, but it looks like you never
addressed my comment from v1:

I don't think you use this `init` field anywhere; I only see it passed
as a parameter. Is there something obvious that I am missing?

> + kunit_resource_free_t free;
> + struct kref refcount;
> struct list_head node;
> };

\
 
 \ /
  Last update: 2020-05-28 21:40    [W:0.057 / U:1.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site