On Wed, Mar 23, 2022 at 2:22 AM Brendan Higgins
<brendanhiggins@xxxxxxxxxx> wrote:
>
<snip>
>
> > The latter ~never need to get "found" (e.g. kunit_kmalloc() users).
> > The one exception: when people use kunit_kfree() to free things early,
> > which requires us to "find" these resources we otherwise wouldn't care
> > about.
> >
> > So I don't know how we can split the API unless we get rid of kunit_kfree().
> > Its presence means kunit_kmalloc() and friends need refcounting.
>
> Do we need to choose between dropping kunit_kfree() and refcounting? I
> think this is semantically different from other findable resources,
> and I think it fairly obviously entails the complexity of using it.
Yes, they're different.
We could do something different and just have a atomic bool "is_freed"
for the kunit_kmalloc() style resources.
But is it worth it?
Currently kunit_kfree() is defined as
697:void kunit_kfree(struct kunit *test, const void *ptr)
698-{
699- struct kunit_resource *res;
700-
701- res = kunit_find_resource(test, kunit_resource_instance_match,
702- (void *)ptr);
703-
704- /*
705- * Removing the resource from the list of resources drops the
706- * reference count to 1; the final put will trigger the free.
707- */
708- kunit_remove_resource(test, res);
709-
710- kunit_put_resource(res);
711-
712-}
i.e. the overhead of using a refcount is that we need to call
kunit_put_resource() bc we called kunit_find_resource().
IMO, this less semantic overhead than adding a different mechanism
specifically for kunit_kfree().
Tangent:
Huh, it segfaults if you call kunit_kfree() on a non-kunit allocated ptr.
res == NULL on 701 in that case, but kunit_remove_resource() doesn't
guard against that.
It also happens if you call kunit_free() twice.
That's analogous to how kfree() works, so I guess that's fine.
A difference though is
kfree(NULL); // is fine
kunit_free(test, NULL); // segfaults, res == NULL above
But thinking on it more, someone could register a resource w/ data == NULL.
I.e. a named resource which just acts as a flag via presence/absence.
kunit_kfree(test, NULL) would the most recent such resource though.
Should we do the trick/hack where we check the free function first in
kunit_kfree() to avoid such confusion?
