fast-get: fix a recent regression

Recent commits 7d0621e ("fast-get: fix partition leak for
multi-thread usage") and 3c757dd ("fast-get: fix crash by
freeing buffer before removing partition") introduced a regression
when using SRC in DP mode in userspace on PTL and running two
aplay -r 44100
back to back. The reason is that SRC isn't calling mod_fast_put()
explicitly and is instead relying on automatic clean up, which then
happens in a different thread.

In fact those two commits weren't needed AFAICS, since fast_put()
shouldn't be called directly from userspace, instead only
mod_fast_put() should be called, which is a syscall, and therefore
fast_put() then will have access to all the memory.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Author: lyakh

src/audio/module_adapter/module/generic.c

@@ -341,6 +341,7 @@ EXPORT_SYMBOL(z_impl_mod_fast_get);
 static int free_contents(struct processing_module *mod, struct module_resource *container)
 {
 	struct module_resources *res = &mod->priv.resources;
+	struct k_mem_domain *mdom;
 
 	switch (container->type) {
 	case MOD_RES_HEAP:
@@ -354,7 +355,12 @@ static int free_contents(struct processing_module *mod, struct module_resource *
 #endif
 #if CONFIG_FAST_GET
 	case MOD_RES_FAST_GET:
-		fast_put(res->heap, container->sram_ptr);
+#if CONFIG_USERSPACE
+		mdom = mod->mdom;
+#else
+		mdom = NULL;
+#endif
+		fast_put(res->heap, mdom, container->sram_ptr);
 		return 0;
 #endif
 	default:

src/include/module/module/base.h

@@ -201,10 +201,8 @@ struct processing_module {
 	enum module_processing_type proc_type;
 #if CONFIG_USERSPACE
 	struct userspace_context *user_ctx;
-#endif /* CONFIG_USERSPACE */
-#if CONFIG_SOF_USERSPACE_APPLICATION
 	struct k_mem_domain *mdom;
-#endif
+#endif /* CONFIG_USERSPACE */
 #endif /* SOF_MODULE_PRIVATE */
 };
 

src/include/sof/lib/fast-get.h

@@ -11,6 +11,7 @@
 #include <stddef.h>
 
 struct k_heap;
+struct k_mem_domain;
 
 /*
  * When built for SOF, fast_get() and fast_put() are only needed when DRAM
@@ -25,13 +26,13 @@ struct k_heap;
 	(CONFIG_LLEXT_TYPE_ELF_RELOCATABLE || !defined(LL_EXTENSION_BUILD))) || \
 	!CONFIG_SOF_FULL_ZEPHYR_APPLICATION
 const void *fast_get(struct k_heap *heap, const void * const dram_ptr, size_t size);
-void fast_put(struct k_heap *heap, const void *sram_ptr);
+void fast_put(struct k_heap *heap, struct k_mem_domain *mdom, const void *sram_ptr);
 #else
 static inline const void *fast_get(struct k_heap *heap, const void * const dram_ptr, size_t size)
 {
 	return dram_ptr;
 }
-static inline void fast_put(struct k_heap *heap, const void *sram_ptr) {}
+static inline void fast_put(struct k_heap *heap, struct k_mem_domain *mdom, const void *sram_ptr) {}
 #endif
 
 #endif /* __SOF_LIB_FAST_GET_H__ */

test/cmocka/src/lib/fast-get/fast-get-tests.c

@@ -77,7 +77,7 @@ static void test_simple_fast_get_put(void **state)
 	assert(ret);
 	assert(!memcmp(ret, testdata[0], sizeof(testdata[0])));
 
-	fast_put(NULL, ret);
+	fast_put(NULL, NULL, ret);
 }
 
 static void test_fast_get_size_missmatch_test(void **state)
@@ -94,7 +94,7 @@ static void test_fast_get_size_missmatch_test(void **state)
 	ret[1] = fast_get(NULL, testdata[0], sizeof(testdata[0]) + 1);
 	assert(!ret[1]);
 
-	fast_put(NULL, ret);
+	fast_put(NULL, NULL, ret);
 }
 
 static void test_over_32_fast_gets_and_puts(void **state)
@@ -111,7 +111,7 @@ static void test_over_32_fast_gets_and_puts(void **state)
 		assert(!memcmp(copy[i], testdata[i], sizeof(testdata[0])));
 
 	for (i = 0; i < ARRAY_SIZE(copy); i++)
-		fast_put(NULL, copy[i]);
+		fast_put(NULL, NULL, copy[i]);
 }
 
 static void test_fast_get_refcounting(void **state)
@@ -133,13 +133,13 @@ static void test_fast_get_refcounting(void **state)
 		assert(!memcmp(copy[0][i], testdata[i], sizeof(testdata[0])));
 
 	for (i = 0; i < ARRAY_SIZE(copy[0]); i++)
-		fast_put(NULL, copy[0][i]);
+		fast_put(NULL, NULL, copy[0][i]);
 
 	for (i = 0; i < ARRAY_SIZE(copy[0]); i++)
 		assert(!memcmp(copy[1][i], testdata[i], sizeof(testdata[0])));
 
 	for (i = 0; i < ARRAY_SIZE(copy[0]); i++)
-		fast_put(NULL, copy[1][i]);
+		fast_put(NULL, NULL, copy[1][i]);
 }
 
 int main(void)

test/ztest/unit/fast-get/test_fast_get_ztest.c

@@ -72,7 +72,7 @@ ZTEST(fast_get_suite, test_simple_fast_get_put)
 	zassert_mem_equal(ret, testdata[0], sizeof(testdata[0]),
 			  "Returned data should match original data");
 
-	fast_put(NULL, ret);
+	fast_put(NULL, NULL, ret);
 }
 
 /**
@@ -94,7 +94,7 @@ ZTEST(fast_get_suite, test_fast_get_size_missmatch_test)
 	ret[1] = fast_get(NULL, testdata[0], sizeof(testdata[0]) + 1);
 	zassert_is_null(ret[1], "fast_get with different size should return NULL");
 
-	fast_put(NULL, ret[0]);
+	fast_put(NULL, NULL, ret[0]);
 }
 
 /**
@@ -116,7 +116,7 @@ ZTEST(fast_get_suite, test_over_32_fast_gets_and_puts)
 				  "Data at index %d should match original", i);
 
 	for (i = 0; i < ARRAY_SIZE(copy); i++)
-		fast_put(NULL, copy[i]);
+		fast_put(NULL, NULL, copy[i]);
 }
 
 /**
@@ -147,7 +147,7 @@ ZTEST(fast_get_suite, test_fast_get_refcounting)
 
 	/* Release first set of references */
 	for (i = 0; i < ARRAY_SIZE(copy[0]); i++)
-		fast_put(NULL, copy[0][i]);
+		fast_put(NULL, NULL, copy[0][i]);
 
 	/* Data should still be valid through second set of references */
 	for (i = 0; i < ARRAY_SIZE(copy[0]); i++)
@@ -156,7 +156,7 @@ ZTEST(fast_get_suite, test_fast_get_refcounting)
 
 	/* Release second set of references */
 	for (i = 0; i < ARRAY_SIZE(copy[0]); i++)
-		fast_put(NULL, copy[1][i]);
+		fast_put(NULL, NULL, copy[1][i]);
 }
 
 /**

zephyr/lib/fast-get.c

@@ -29,7 +29,7 @@ struct sof_fast_get_entry {
 	const void *dram_ptr;
 	void *sram_ptr;
 #if CONFIG_USERSPACE
-	struct k_thread *thread;
+	struct k_mem_domain *mdom;
 #endif
 	size_t size;
 	unsigned int refcount;
@@ -117,7 +117,7 @@ static bool fast_get_domain_exists(struct k_thread *thread, void *start, size_t
 	return false;
 }
 
-static int fast_get_access_grant(k_tid_t thread, void *addr, size_t size)
+static int fast_get_access_grant(struct k_mem_domain *mdom, void *addr, size_t size)
 {
 	struct k_mem_partition part = {
 		.start = (uintptr_t)addr,
@@ -126,7 +126,7 @@ static int fast_get_access_grant(k_tid_t thread, void *addr, size_t size)
 	};
 
 	LOG_DBG("add %#zx @ %p", part.size, addr);
-	return k_mem_domain_add_partition(thread->mem_domain_info.mem_domain, &part);
+	return k_mem_domain_add_partition(mdom, &part);
 }
 #endif /* CONFIG_USERSPACE */
 
@@ -183,15 +183,18 @@ const void *fast_get(struct k_heap *heap, const void *dram_ptr, size_t size)
 		ret = entry->sram_ptr;
 
 #if CONFIG_USERSPACE
+		struct k_mem_domain *mdom = k_current_get()->mem_domain_info.mem_domain;
+
 		/* We only get there for large buffers */
-		if (k_current_get()->mem_domain_info.mem_domain->num_partitions > 1) {
+		if (mdom->num_partitions > 1) {
 			/* A userspace thread makes the request */
-			if (k_current_get() != entry->thread &&
+			if (mdom != entry->mdom &&
 			    !fast_get_domain_exists(k_current_get(), ret,
 						    ALIGN_UP(size, CONFIG_MM_DRV_PAGE_SIZE))) {
-				LOG_DBG("grant access to thread %p first was %p", k_current_get(),
-					entry->thread);
-				int err = fast_get_access_grant(k_current_get(), ret, size);
+				LOG_DBG("grant access to domain %p first was %p", mdom,
+					entry->mdom);
+
+				int err = fast_get_access_grant(mdom, ret, size);
 
 				if (err < 0) {
 					LOG_ERR("failed to grant additional access err=%d", err);
@@ -228,10 +231,10 @@ const void *fast_get(struct k_heap *heap, const void *dram_ptr, size_t size)
 	dcache_writeback_region((__sparse_force void __sparse_cache *)entry->sram_ptr, size);
 
 #if CONFIG_USERSPACE
-	entry->thread = k_current_get();
+	entry->mdom = k_current_get()->mem_domain_info.mem_domain;
 	if (size > FAST_GET_MAX_COPY_SIZE) {
 		/* Otherwise we've allocated on thread's heap, so it already has access */
-		int err = fast_get_access_grant(entry->thread, ret, size);
+		int err = fast_get_access_grant(entry->mdom, ret, size);
 
 		if (err < 0) {
 			LOG_ERR("failed to grant access err=%d", err);
@@ -265,7 +268,7 @@ static struct sof_fast_get_entry *fast_put_find_entry(struct sof_fast_get_data *
 	return NULL;
 }
 
-void fast_put(struct k_heap *heap, const void *sram_ptr)
+void fast_put(struct k_heap *heap, struct k_mem_domain *mdom, const void *sram_ptr)
 {
 	struct sof_fast_get_data *data = &fast_get_data;
 	struct sof_fast_get_entry *entry;
@@ -294,17 +297,16 @@ void fast_put(struct k_heap *heap, const void *sram_ptr)
 	 * Order matters: free buffer first (needs partition for cache access),
 	 * then remove partition.
 	 */
-	if (entry->size > FAST_GET_MAX_COPY_SIZE && entry->thread) {
+	if (entry->size > FAST_GET_MAX_COPY_SIZE && entry->mdom) {
 		struct k_mem_partition part = {
 			.start = (uintptr_t)entry->sram_ptr,
 			.size = ALIGN_UP(entry->size, CONFIG_MM_DRV_PAGE_SIZE),
 			.attr = K_MEM_PARTITION_P_RO_U_RO | XTENSA_MMU_CACHED_WB,
 		};
-		struct k_mem_domain *domain = k_current_get()->mem_domain_info.mem_domain;
 
-		LOG_DBG("removing partition %p size %#zx from thread %p",
-			(void *)part.start, part.size, k_current_get());
-		int err = k_mem_domain_remove_partition(domain, &part);
+		LOG_DBG("removing partition %p size %#zx memory domain %p",
+			(void *)part.start, part.size, mdom);
+		int err = k_mem_domain_remove_partition(mdom, &part);
 
 		if (err)
 			LOG_WRN("partition removal failed: %d", err);