| Message ID | 20220326002013.483394-2-dlatypov@google.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | [v2,1/2] kunit: split resource API from test.h into new resource.h | expand |
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 >
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>
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>
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;
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