Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[Marco Pagani <marco.pagani@linux.dev>]

On 16/07/2026 14:14, Tvrtko Ursulin wrote:
> 
> 
> On 15/07/2026 17:02, Marco Pagani wrote:
> 
> 8<>
> 
>>> I first want to understand exactly what the issue is. The current design
>>> exactly cannot free the memory under the scheduler nose unless there is
>>> a test bug where test exited and has left the scheduler with unprocessed
>>> jobs. I am not saying we cannot have reference counted jobs but, again,
>>> at the moment I don't understand the problem.
>>
>> I think that the current design of the mock scheduler can indeed free
>> test objects' memory under the scheduler backend's nose. Consider the
>> example discussed above: the mock scheduler is instantiated and released
>> in the .init and .exit functions respectively, while jobs are allocated
>> in the test body using drm_mock_sched_job_new(). In this rather basic
>> use case, jobs' memory is freed before the entities and the scheduler
>> are released in the .exit function.
> 
> So at first I thought that as long as entity is freed at each test exit, 
> which includes idling, and is done by all current test cases (apart from 
> the parallel one where you did it via actions) it is all fine. The 
> entity->last_scheduled you mention is not a concern then since it is 
> cleared etc.
> 
> However, now I think that even if entity is idled and freed before test 
> exit that does nothing for the jobs not yet processed by the 
> drm_sched_free_job_work()-er.
> 
> Even though I was testing with KASAN I possibly did not manage to hit 
> that race. Strange but I guess possible.

It was also a surprise for me when Philipp reported a suspected UAF.
Definitely a subtle bug to catch. I managed to reproduce it by building
the kernel with KASAN and kunit.autorun enabled, and then simply booting
it with no rootfs using qemu with this slightly odd configuration:

qemu-system-x86_64 -enable-kvm -m 48G -smp cpus=18 -kernel [...]

> Unless I am missing something with the above analysis, one options is 
> along the lines of what you propose - forgo using kunit managed 
> allocations for anything mock scheduler owned. Second would be to stop 
> using test suite init/exit for the scheduler management and just make 
> each test create own scheduler and free it as it exits.

I think the second approach is still fragile. If we move
drm_mock_sched_fini() at the end of each test body, any KUNIT_ASSERT_*()
failure will cause the test to abort immediately, skipping the teardown
and leaving the scheduler in an inconsistent state that could cause any
sort of problem.

Moreover, initializing/tearing down the scheduler in the test's
init/exit functions is the most intuitive and possibly the best approach
(DRY) from a test code perspective. Asking test writers to not use the
standard init/exit functions and enforcing this limitation is a burden
that I think no one deserves to take.

> Regardless of the option, as long as it is safe against UAFs we are I 
> think good. Memory leaks on test failures are not that interesting that 
> it would warrant coming up with anything more complicated.

> Does that make sense? If so, question is which option is simpler? 
> Probably just making tests create and destroy schedulers since then 
> there is no need to deal with any memory freeing. It would be just two 
> lines added to a subset of tests, the ones which rely on .init/.exit.

I thought so too when developing the drm_gem_shmem_test some time ago,
but then I changed my mind after dealing with downstream CI. A single
test leaking some memory is not optimal, but not even a big deal per se.
However, consider hundreds of KUnit tests running periodically on a CI.
If all failing tests did not care about leaking memory, they could
seriously bog down the system. I think it's responsible to have tests
that do not leak memory when they fail.

I'll see if I can come up with a cleaner version of the kref approach,
taking into account your recommendations and Sashiko's bug report.

Thank you for your suggestions,
Marco
 
> Regards,
> 
> Tvrtko
>