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 <[email protected]> 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/[email protected]?part=11
