On Fri, Mar 27, 2020 at 5:45 AM Alan Maguire <alan.maguire@xxxxxxxxxx> 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@xxxxxxxxxx> One comment below, other than that: Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx> 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, ¶ms); > - * 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; > };
