On Wed, Mar 23, 2022 at 2:22 AM Brendan Higgins brendanhiggins@google.com 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?