On Mon, Jul 15, 2019 at 1:24 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Brendan Higgins (2019-07-12 01:17:28)
diff --git a/kunit/test.c b/kunit/test.c index 571e4c65deb5c..f165c9d8e10b0 100644 --- a/kunit/test.c +++ b/kunit/test.c @@ -171,6 +175,96 @@ int kunit_run_tests(struct kunit_suite *suite) return 0; }
+struct kunit_resource *kunit_alloc_resource(struct kunit *test,
kunit_resource_init_t init,kunit_resource_free_t free,void *context)+{
struct kunit_resource *res;int ret;res = kzalloc(sizeof(*res), GFP_KERNEL);This uses GFP_KERNEL.
if (!res)return NULL;ret = init(res, context);if (ret)return NULL;res->free = free;mutex_lock(&test->lock);And this can sleep.
list_add_tail(&res->node, &test->resources);mutex_unlock(&test->lock);return res;+}
+void kunit_free_resource(struct kunit *test, struct kunit_resource *res)
Should probably add a note that we assume the test lock is held here, or even add a lockdep_assert_held(&test->lock) into the function to document that and assert it at the same time.
Seems reasonable.
+{
res->free(res);list_del(&res->node);kfree(res);+}
+struct kunit_kmalloc_params {
size_t size;gfp_t gfp;+};
+static int kunit_kmalloc_init(struct kunit_resource *res, void *context) +{
struct kunit_kmalloc_params *params = context;res->allocation = kmalloc(params->size, params->gfp);if (!res->allocation)return -ENOMEM;return 0;+}
+static void kunit_kmalloc_free(struct kunit_resource *res) +{
kfree(res->allocation);+}
+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,This calls that sleeping function above...
kunit_kmalloc_init,kunit_kmalloc_free,¶ms);but this passes a GFP flags parameter through to the kunit_kmalloc_init() function. How is this going to work if some code uses GFP_ATOMIC, but then we try to allocate and sleep in kunit_alloc_resource() with GFP_KERNEL?
Yeah, that's an inconsistency. I need to fix that.
One solution would be to piggyback on all the existing devres allocation logic we already have and make each struct kunit a device that we pass into the devres functions. A far simpler solution would be to just copy/paste what devres does and use a spinlock and an allocation function that takes GFP flags.
Yeah, that's what I did originally, but I thought from the discussion on patch 01 that you thought a spinlock was overkill for struct kunit. I take it you only meant in that initial patch?
if (res)return res->allocation;return NULL;+}
Cheers