Commit Graph

3 Commits

Author SHA1 Message Date
David Gow
ad69172ec9 kunit: Rework kunit_resource allocation policy
KUnit's test-managed resources can be created in two ways:
- Using the kunit_add_resource() family of functions, which accept a
  struct kunit_resource pointer, typically allocated statically or on
  the stack during the test.
- Using the kunit_alloc_resource() family of functions, which allocate a
  struct kunit_resource using kzalloc() behind the scenes.

Both of these families of functions accept a 'free' function to be
called when the resource is finally disposed of.

At present, KUnit will kfree() the resource if this 'free' function is
specified, and will not if it is NULL. However, this can lead
kunit_alloc_resource() to leak memory (if no 'free' function is passed
in), or kunit_add_resource() to incorrectly kfree() memory which was
allocated by some other means (on the stack, as part of a larger
allocation, etc), if a 'free' function is provided.

Instead, always kfree() if the resource was allocated with
kunit_alloc_resource(), and never kfree() if it was passed into
kunit_add_resource() by the user. (If the user of kunit_add_resource()
wishes the resource be kfree()ed, they can call kfree() on the resource
from within the 'free' function.

This is implemented by adding a 'should_free' member to
struct kunit_resource and setting it appropriately. To facilitate this,
the various resource add/alloc functions have been refactored somewhat,
making them all call a __kunit_add_resource() helper after setting the
'should_free' member appropriately. In the process, all other functions
have been made static inline functions.

Signed-off-by: David Gow <davidgow@google.com>
Tested-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
2022-05-12 11:14:39 -06:00
David Gow
59729170af kunit: Make kunit_remove_resource() idempotent
The kunit_remove_resource() function is used to unlink a resource from
the list of resources in the test, making it no longer show up in
kunit_find_resource().

However, this could lead to a race condition if two threads called
kunit_remove_resource() on the same resource at the same time: the
resource would be removed from the list twice (causing a crash at the
second list_del()), and the refcount for the resource would be
decremented twice (instead of once, for the reference held by the
resource list).

Fix both problems, the first by using list_del_init(), and the second by
checking if the resource has already been removed using list_empty(),
and only decrementing its refcount if it has not.

Also add a KUnit test for the kunit_remove_resource() function which
tests this behaviour.

Reported-by: Daniel Latypov <dlatypov@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
2022-04-05 13:32:50 -06:00
Daniel Latypov
cdebea6968 kunit: split resource API impl from test.c into new resource.c
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: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
2022-04-04 16:23:14 -06:00