Re: [PATCH v2 11/14] hv: vmbus: vmalloc requestor metadata

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] hv: memory leak in vmbus KUnit tests on KUNIT_ASSERT_EQ failure
--

commit 691cb947fd6eb3bbd5abcb1f0c1c8a97f9b7e21e
Author: Emerson Busson <emersonbusson@gmail.com>

hv: vmbus: vmalloc requestor metadata

This commit changes the channel open requestor array and bitmap allocations
from kvcalloc and bitmap_zalloc to vzalloc to avoid high-order allocations
under memory fragmentation, and adds KUnit tests for these structures.

> diff --git a/drivers/hv/vmbus_buffer_test.c b/drivers/hv/vmbus_buffer_test.c
> index 5c8e70d861adb..d0102dabef737 100644
> --- a/drivers/hv/vmbus_buffer_test.c
> +++ b/drivers/hv/vmbus_buffer_test.c
[ ... ]
> +static void vmbus_requestor_invalid_ids_test(struct kunit *test)
> +{
> +	struct vmbus_channel channel = { .rqstor_size = 4 };
> +	struct vmbus_requestor *rqstor = &channel.requestor;
> +	u64 id, addr;
> +
> +	KUNIT_ASSERT_EQ(test, vmbus_alloc_requestor(rqstor, 4), 0);
> +
> +	/* ID 0 is the unsolicited-message sentinel and is never in the set. */
> +	KUNIT_EXPECT_EQ(test, vmbus_request_addr_match(&channel, 0, 0),
> +			VMBUS_RQST_ERROR);
> +
> +	/* Out-of-range IDs are refused, not wrapped. */
> +	KUNIT_EXPECT_EQ(test, vmbus_request_addr_match(&channel, 5, 0),
> +			VMBUS_RQST_ERROR);
> +	KUNIT_EXPECT_EQ(test, vmbus_request_addr_match(&channel, U64_MAX, 0),
> +			VMBUS_RQST_ERROR);
> +
> +	id = vmbus_next_request_id(&channel, 0xABCD);
> +	KUNIT_ASSERT_EQ(test, id, 1U);

[Severity: Low]
Does this abort the test and skip the cleanup on failure?

In vmbus_requestor_invalid_ids_test(), vmbus_alloc_requestor() is called 
earlier to allocate the vmalloc-backed metadata arrays. If this 
KUNIT_ASSERT_EQ() fails, KUnit immediately aborts the execution of the test 
function.

This would skip the manual cleanup call to vmbus_free_requestor() at the 
end of the function, resulting in a memory leak of the requestor arrays.

Could this be changed to use KUNIT_EXPECT_EQ(), which reports the failure 
but continues execution, or could a deferred cleanup action be registered 
with KUnit?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007190752.336426-1-emersonbusson@gmail.com?part=11