diff --git a/scripts/kernel/generate-kernel-patch b/scripts/kernel/generate-kernel-patch index 4a3c0dfd9..12374e86b 100755 --- a/scripts/kernel/generate-kernel-patch +++ b/scripts/kernel/generate-kernel-patch @@ -325,7 +325,8 @@ scst_07_pres="scst/src/scst_pres.h scst/src/scst_pres.c" scst_08_sysfs="scst/src/scst_sysfs.c \ scst/src/scst_sysfs_test.h" scst_09_debug="scst/include/scst_debug.h scst/src/scst_debug.c" -scst_10_sgv="scst/include/scst_sgv.h scst/src/scst_mem.h scst/src/scst_mem.c doc/scst_pg.sgml" +scst_10_sgv="scst/include/scst_sgv.h scst/src/scst_mem.h scst/src/scst_mem.c \ +scst/src/scst_mem_test.h doc/scst_pg.sgml" scst_user="scst/include/scst_user.h scst/src/dev_handlers/scst_user_main.c \ scst/src/dev_handlers/scst_user_test.h" scst_13_vdisk="scst/src/dev_handlers/scst_vdisk_main.c \ diff --git a/scst/src/kunit/lib/suites/scst_kunit_sgv.c b/scst/src/kunit/lib/suites/scst_kunit_sgv.c new file mode 100644 index 000000000..709fcf1ef --- /dev/null +++ b/scst/src/kunit/lib/suites/scst_kunit_sgv.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include "../../scst_kunit.h" +#include +#include +#ifdef INSIDE_KERNEL_TREE +#include +#else +#include "scst.h" +#endif +#include "../../../scst_mem.h" +#include "../../../scst_mem_test.h" + +static void sgv_shared_rejects_other_mm(struct kunit *test) +{ + struct sgv_pool *pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + struct mm_struct *other_mm = kunit_kzalloc(test, sizeof(*other_mm), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, pool); + KUNIT_ASSERT_NOT_NULL(test, other_mm); + pool->owner_mm = other_mm; + kref_init(&pool->sgv_pool_ref); + + /* A failed lookup must not return the original owner's reference. */ + KUNIT_EXPECT_PTR_EQ(test, sgv_pool_get_shared(pool), NULL); + KUNIT_EXPECT_EQ(test, kref_read(&pool->sgv_pool_ref), 1); +} + +static void sgv_shared_gets_same_mm(struct kunit *test) +{ + struct sgv_pool *pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, pool); + pool->owner_mm = current->mm; + kref_init(&pool->sgv_pool_ref); + + KUNIT_EXPECT_PTR_EQ(test, sgv_pool_get_shared(pool), pool); + KUNIT_EXPECT_EQ(test, kref_read(&pool->sgv_pool_ref), 2); +} + +static void sgv_defer_release(struct kref *ref) +{ + /* Keep the fixture alive to model a destructor waiting to unlink it. */ +} + +static void sgv_shared_rejects_destroying_pool(struct kunit *test) +{ + struct sgv_pool *pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, pool); + pool->owner_mm = current->mm; + /* Last put has reached zero; the destructor has not unlinked it yet. */ + kref_init(&pool->sgv_pool_ref); + kref_put(&pool->sgv_pool_ref, sgv_defer_release); + + KUNIT_EXPECT_PTR_EQ(test, sgv_pool_get_shared(pool), NULL); + KUNIT_EXPECT_EQ(test, kref_read(&pool->sgv_pool_ref), 0); +} + +static struct kunit_case scst_sgv_cases[] = { + KUNIT_CASE(sgv_shared_rejects_other_mm), + KUNIT_CASE(sgv_shared_gets_same_mm), + KUNIT_CASE(sgv_shared_rejects_destroying_pool), + {} +}; + +scst_kunit_suite(scst_sgv_suite, "LIB", "-sgv", + .test_cases = scst_sgv_cases, +); diff --git a/scst/src/kunit/objects.mk b/scst/src/kunit/objects.mk index 0ef838000..8358279c3 100644 --- a/scst/src/kunit/objects.mk +++ b/scst/src/kunit/objects.mk @@ -11,6 +11,7 @@ scst-kunit-scst := \ kunit/event/suites/scst_kunit_event_delivery.o \ kunit/lib/suites/scst_kunit_capacity.o \ kunit/lib/suites/scst_kunit_lib.o \ + kunit/lib/suites/scst_kunit_sgv.o \ kunit/lib/suites/scst_kunit_write_same.o \ kunit/lifecycle/suites/scst_kunit_lifecycle.o \ kunit/lifecycle/suites/scst_kunit_blocking.o \ diff --git a/scst/src/scst_mem.c b/scst/src/scst_mem.c index 94adc47a0..69a2d3a18 100644 --- a/scst/src/scst_mem.c +++ b/scst/src/scst_mem.c @@ -33,6 +33,7 @@ #endif #include "scst_priv.h" #include "scst_mem.h" +#include "scst_mem_test.h" #undef DEFAULT_SYMBOL_NAMESPACE #define DEFAULT_SYMBOL_NAMESPACE SCST_NAMESPACE @@ -1386,7 +1387,7 @@ static int sgv_pool_init(struct sgv_pool *pool, const char *name, } } - atomic_set(&pool->sgv_pool_ref, 1); + kref_init(&pool->sgv_pool_ref); spin_lock_init(&pool->sgv_pool_lock); INIT_LIST_HEAD(&pool->sorted_recycling_list); for (i = 0; i < pool->max_caches; i++) @@ -1506,6 +1507,11 @@ out: TRACE_EXIT(); } +static void sgv_pool_release(struct kref *ref) +{ + sgv_pool_destroy(container_of(ref, struct sgv_pool, sgv_pool_ref)); +} + /** * sgv_pool_set_allocator - set custom pages allocator * @pool: the cache @@ -1525,6 +1531,23 @@ void sgv_pool_set_allocator(struct sgv_pool *pool, } EXPORT_SYMBOL_GPL(sgv_pool_set_allocator); +/* The caller holds sgv_pools_mutex to protect the lookup result. */ +SCST_KUNIT_VISIBLE +struct sgv_pool *sgv_pool_get_shared(struct sgv_pool *pool) +{ + if (pool->owner_mm != current->mm) { + PRINT_ERROR("Attempt of a shared use of SGV pool %s with different MM", + pool->name); + return NULL; + } + + /* Last put can have committed to destruction before taking the mutex. */ + if (!kref_get_unless_zero(&pool->sgv_pool_ref)) + return NULL; + + return pool; +} + /** * sgv_pool_create_node - creates and initializes an SGV pool * @name: the name of the SGV pool @@ -1581,12 +1604,7 @@ struct sgv_pool *sgv_pool_create_node(const char *name, enum sgv_clustering_type list_for_each_entry(tp, &sgv_pools_list, sgv_pools_list_entry) { if (strcmp(tp->name, name) == 0) { if (shared) { - if (tp->owner_mm != current->mm) { - PRINT_ERROR("Attempt of a shared use of SGV pool %s with different MM", - name); - goto out_free; - } - sgv_pool_get(tp); + tp = sgv_pool_get_shared(tp); goto out_free; } else { PRINT_ERROR("SGV pool %s already exists", name); @@ -1623,9 +1641,9 @@ EXPORT_SYMBOL_GPL(sgv_pool_create_node); */ void sgv_pool_get(struct sgv_pool *pool) { - atomic_inc(&pool->sgv_pool_ref); - TRACE_MEM("Incrementing sgv pool %p ref (new value %d)", - pool, atomic_read(&pool->sgv_pool_ref)); + kref_get(&pool->sgv_pool_ref); + TRACE_MEM("Incrementing sgv pool %p ref (new value %u)", + pool, kref_read(&pool->sgv_pool_ref)); } EXPORT_SYMBOL_GPL(sgv_pool_get); @@ -1637,10 +1655,9 @@ EXPORT_SYMBOL_GPL(sgv_pool_get); */ void sgv_pool_put(struct sgv_pool *pool) { - TRACE_MEM("Decrementing sgv pool %p ref (new value %d)", - pool, atomic_read(&pool->sgv_pool_ref) - 1); - if (atomic_dec_and_test(&pool->sgv_pool_ref)) - sgv_pool_destroy(pool); + TRACE_MEM("Decrementing sgv pool %p ref (new value %u)", + pool, kref_read(&pool->sgv_pool_ref) - 1); + kref_put(&pool->sgv_pool_ref, sgv_pool_release); } EXPORT_SYMBOL_GPL(sgv_pool_put); diff --git a/scst/src/scst_mem.h b/scst/src/scst_mem.h index cfffb6986..fee8950b2 100644 --- a/scst/src/scst_mem.h +++ b/scst/src/scst_mem.h @@ -15,6 +15,7 @@ * GNU General Public License for more details. */ +#include #include #include @@ -113,7 +114,7 @@ struct sgv_pool { atomic_t big_alloc, big_pages, big_merged; atomic_t other_alloc, other_pages, other_merged; - atomic_t sgv_pool_ref; + struct kref sgv_pool_ref; int max_caches; diff --git a/scst/src/scst_mem_test.h b/scst/src/scst_mem_test.h new file mode 100644 index 000000000..f17837e46 --- /dev/null +++ b/scst/src/scst_mem_test.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef SCST_MEM_TEST_H +#define SCST_MEM_TEST_H + +#include "scst_test.h" + +#ifdef SCST_KUNIT_ENABLED +struct sgv_pool; + +struct sgv_pool *sgv_pool_get_shared(struct sgv_pool *pool); +#endif + +#endif /* SCST_MEM_TEST_H */