| Message ID | 20230331080411.981038-4-davidgow@google.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: Deferred action helpers | expand |
Hi, On Fri, 2023-03-31 at 16:04 +0800, 'David Gow' via KUnit Development wrote: > The kunit_add_action() function is much simpler and cleaner to use that > the full KUnit resource API for simple things like the > kunit_kmalloc_array() functionality. > > Replacing it allows us to get rid of a number of helper functions, and > leaves us with no uses of kunit_alloc_resource(), which has some > usability problems and is going to have its behaviour modified in an > upcoming patch. > > Note that we need to use kunit_defer_trigger_all() to implement > kunit_kfree(). Just a nitpick: kunit_defer_trigger_all does not exist in the new patch anymore. I guess this should be kunit_release_action. Benjamin > > Signed-off-by: David Gow <davidgow@google.com> > --- > lib/kunit/test.c | 48 ++++++++---------------------------------------- > 1 file changed, 8 insertions(+), 40 deletions(-) > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index e2910b261112..ec45c8863f04 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -712,58 +712,26 @@ static struct notifier_block kunit_mod_nb = { > }; > #endif > > -struct kunit_kmalloc_array_params { > - size_t n; > - size_t size; > - gfp_t gfp; > -}; > - > -static int kunit_kmalloc_array_init(struct kunit_resource *res, void *context) > +void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) > { > - struct kunit_kmalloc_array_params *params = context; > - > - res->data = kmalloc_array(params->n, params->size, params->gfp); > - if (!res->data) > - return -ENOMEM; > - > - return 0; > -} > + void *data; > > -static void kunit_kmalloc_array_free(struct kunit_resource *res) > -{ > - kfree(res->data); > -} > + data = kmalloc_array(n, size, gfp); > > -void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) > -{ > - struct kunit_kmalloc_array_params params = { > - .size = size, > - .n = n, > - .gfp = gfp > - }; > + if (!data) > + return NULL; > > - return kunit_alloc_resource(test, > - kunit_kmalloc_array_init, > - kunit_kmalloc_array_free, > - gfp, > - ¶ms); > + kunit_add_action(test, (kunit_defer_function_t)kfree, data, gfp); > + return data; > } > EXPORT_SYMBOL_GPL(kunit_kmalloc_array); > > -static inline bool kunit_kfree_match(struct kunit *test, > - struct kunit_resource *res, void *match_data) > -{ > - /* Only match resources allocated with kunit_kmalloc() and friends. */ > - return res->free == kunit_kmalloc_array_free && res->data == match_data; > -} > - > void kunit_kfree(struct kunit *test, const void *ptr) > { > if (!ptr) > return; > > - if (kunit_destroy_resource(test, kunit_kfree_match, (void *)ptr)) > - KUNIT_FAIL(test, "kunit_kfree: %px already freed or not allocated by kunit", ptr); > + kunit_release_action(test, (kunit_defer_function_t)kfree, (void *)ptr); > } > EXPORT_SYMBOL_GPL(kunit_kfree); > > -- > 2.40.0.348.gf938b09366-goog >
On Wed, 5 Apr 2023 at 01:59, Benjamin Berg <benjamin@sipsolutions.net> wrote: > > Hi, > > On Fri, 2023-03-31 at 16:04 +0800, 'David Gow' via KUnit Development wrote: > > The kunit_add_action() function is much simpler and cleaner to use that > > the full KUnit resource API for simple things like the > > kunit_kmalloc_array() functionality. > > > > Replacing it allows us to get rid of a number of helper functions, and > > leaves us with no uses of kunit_alloc_resource(), which has some > > usability problems and is going to have its behaviour modified in an > > upcoming patch. > > > > Note that we need to use kunit_defer_trigger_all() to implement > > kunit_kfree(). > > Just a nitpick: kunit_defer_trigger_all does not exist in the new patch > anymore. I guess this should be kunit_release_action. > > Benjamin > Nice catch, thanks! I'll fix it in the next revision. Cheers, -- David -- David > > > > Signed-off-by: David Gow <davidgow@google.com> > > --- > > lib/kunit/test.c | 48 ++++++++---------------------------------------- > > 1 file changed, 8 insertions(+), 40 deletions(-) > > > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > > index e2910b261112..ec45c8863f04 100644 > > --- a/lib/kunit/test.c > > +++ b/lib/kunit/test.c > > @@ -712,58 +712,26 @@ static struct notifier_block kunit_mod_nb = { > > }; > > #endif > > > > -struct kunit_kmalloc_array_params { > > - size_t n; > > - size_t size; > > - gfp_t gfp; > > -}; > > - > > -static int kunit_kmalloc_array_init(struct kunit_resource *res, void *context) > > +void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) > > { > > - struct kunit_kmalloc_array_params *params = context; > > - > > - res->data = kmalloc_array(params->n, params->size, params->gfp); > > - if (!res->data) > > - return -ENOMEM; > > - > > - return 0; > > -} > > + void *data; > > > > -static void kunit_kmalloc_array_free(struct kunit_resource *res) > > -{ > > - kfree(res->data); > > -} > > + data = kmalloc_array(n, size, gfp); > > > > -void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) > > -{ > > - struct kunit_kmalloc_array_params params = { > > - .size = size, > > - .n = n, > > - .gfp = gfp > > - }; > > + if (!data) > > + return NULL; > > > > - return kunit_alloc_resource(test, > > - kunit_kmalloc_array_init, > > - kunit_kmalloc_array_free, > > - gfp, > > - ¶ms); > > + kunit_add_action(test, (kunit_defer_function_t)kfree, data, gfp); > > + return data; > > } > > EXPORT_SYMBOL_GPL(kunit_kmalloc_array); > > > > -static inline bool kunit_kfree_match(struct kunit *test, > > - struct kunit_resource *res, void *match_data) > > -{ > > - /* Only match resources allocated with kunit_kmalloc() and friends. */ > > - return res->free == kunit_kmalloc_array_free && res->data == match_data; > > -} > > - > > void kunit_kfree(struct kunit *test, const void *ptr) > > { > > if (!ptr) > > return; > > > > - if (kunit_destroy_resource(test, kunit_kfree_match, (void *)ptr)) > > - KUNIT_FAIL(test, "kunit_kfree: %px already freed or not allocated by kunit", ptr); > > + kunit_release_action(test, (kunit_defer_function_t)kfree, (void *)ptr); > > } > > EXPORT_SYMBOL_GPL(kunit_kfree); > > > > -- > > 2.40.0.348.gf938b09366-goog > > >
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index e2910b261112..ec45c8863f04 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -712,58 +712,26 @@ static struct notifier_block kunit_mod_nb = { }; #endif -struct kunit_kmalloc_array_params { - size_t n; - size_t size; - gfp_t gfp; -}; - -static int kunit_kmalloc_array_init(struct kunit_resource *res, void *context) +void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) { - struct kunit_kmalloc_array_params *params = context; - - res->data = kmalloc_array(params->n, params->size, params->gfp); - if (!res->data) - return -ENOMEM; - - return 0; -} + void *data; -static void kunit_kmalloc_array_free(struct kunit_resource *res) -{ - kfree(res->data); -} + data = kmalloc_array(n, size, gfp); -void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) -{ - struct kunit_kmalloc_array_params params = { - .size = size, - .n = n, - .gfp = gfp - }; + if (!data) + return NULL; - return kunit_alloc_resource(test, - kunit_kmalloc_array_init, - kunit_kmalloc_array_free, - gfp, - ¶ms); + kunit_add_action(test, (kunit_defer_function_t)kfree, data, gfp); + return data; } EXPORT_SYMBOL_GPL(kunit_kmalloc_array); -static inline bool kunit_kfree_match(struct kunit *test, - struct kunit_resource *res, void *match_data) -{ - /* Only match resources allocated with kunit_kmalloc() and friends. */ - return res->free == kunit_kmalloc_array_free && res->data == match_data; -} - void kunit_kfree(struct kunit *test, const void *ptr) { if (!ptr) return; - if (kunit_destroy_resource(test, kunit_kfree_match, (void *)ptr)) - KUNIT_FAIL(test, "kunit_kfree: %px already freed or not allocated by kunit", ptr); + kunit_release_action(test, (kunit_defer_function_t)kfree, (void *)ptr); } EXPORT_SYMBOL_GPL(kunit_kfree);
The kunit_add_action() function is much simpler and cleaner to use that the full KUnit resource API for simple things like the kunit_kmalloc_array() functionality. Replacing it allows us to get rid of a number of helper functions, and leaves us with no uses of kunit_alloc_resource(), which has some usability problems and is going to have its behaviour modified in an upcoming patch. Note that we need to use kunit_defer_trigger_all() to implement kunit_kfree(). Signed-off-by: David Gow <davidgow@google.com> --- lib/kunit/test.c | 48 ++++++++---------------------------------------- 1 file changed, 8 insertions(+), 40 deletions(-)