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, ¶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;};