From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7F63B3A63F6 for ; Fri, 6 Feb 2026 12:12:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770379934; cv=none; b=PzvMY2psMaV+kuXFO68/9LFTHlrB0qxwyPdXAsWZt8gCkIz5ZZptAgcHFXa51WTt6/atq8yEnXSY0UgKBMJ4fuSkZjmqORF5ZKVDKZcAuuCqE3aN3OdBbcAfzZIw3l2DdueTce4vvBFrUX6+WOry3aOTu0nAN1Ehq4kdaT8c508= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770379934; c=relaxed/simple; bh=RFfA8Duru7OkHxqTl27oy2IgP2/9RcM+rB4UvvLSdDc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=poQjU76QWrAbzle/1xxlr54+VCAZ+mgu5YH0C+XHV5OXzDznbcMcyZx/YN6sS5Dks7hiEWdwyjNYI+kkJDg7BeJvOJ6gjPYmQn7jU56/goBbiQ2pCMYymwSi701glLT7eUyk+QBufwGIg2ChAb5v+UqQiExg01dxHGt8SKkTOz4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ursulin.net; spf=pass smtp.mailfrom=ursulin.net; dkim=pass (2048-bit key) header.d=ursulin.net header.i=@ursulin.net header.b=Br1Jy0LZ; arc=none smtp.client-ip=209.85.128.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ursulin.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ursulin.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ursulin.net header.i=@ursulin.net header.b="Br1Jy0LZ" Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-48327b8350dso3213615e9.1 for ; Fri, 06 Feb 2026 04:12:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ursulin.net; s=google; t=1770379932; x=1770984732; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=YTnNWMKuRG+jSdrzPn8mAyMAVE6ntJcgf6I9XpbwmCg=; b=Br1Jy0LZx7LMr0ep8yYOAccIyTtHFMAizWuPNpPOLKxjcVgVPxawVpQ9KU1YMy8jcu oqUnycNMXUCZch/chv3hfom4MS4mOrPSQMB/HbLSURMR+8vauCXly9s6wEz4A4Ez6hTp gPyZV8cWhxTKcdhbKu7RBsC0gUmb31sUY550bORCUAd+UxqasxjXL+JedjY0pctKlvxS q6Dw3vk75XGSkre7EQ0DlKgvesgl3q2K8Cz6S/A6KcCmgFelU4R81EbaM0OBe8jJQaus FPSc23KISZpG57x1hq4wA7KWtaKQ++WQHkhMw02hsggjHRaDbEJjh0ZF2Efmg0oVp8+d Fpvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770379932; x=1770984732; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=YTnNWMKuRG+jSdrzPn8mAyMAVE6ntJcgf6I9XpbwmCg=; b=Vo9z//uYos0mteH6yx0TnXjVtgeyPKtwPPY2BpQXo3+mJQ+MP5vSRXvCSK/TZku21b vVmShDKmXWf/7zu2SJWFfowtb9ETU8gCTNYP0fHC5zG0xAiaIAY71bdBKWOL/FnvVK3j c8B+Xgt69V3df0ZqYBZXVpWG+c7hV4hN+oR1cs5xKPv8ehNFkdj6E9L1buubHFI7BRu3 5T/641WP60ZBMQt0zuZrZ+zA4QY3kG/PW1w1q1Ii+PYha1JxCFsSex6wVVH6MugKFepl /FMuorzkhXmCi2gBx03Hz9MNXVxA02uV4GhA0I9ML3Yyk7ELnQYh097qLjHBsR7Lw9J2 r7VA== X-Forwarded-Encrypted: i=1; AJvYcCV/yBXF9uzGphMUCchf9Wxigja0ftnhNBodrvu5yvZ/DMWbqRfvv9zc1GV0Wd2NouOsLr9nBQozb1hzoco=@vger.kernel.org X-Gm-Message-State: AOJu0Yx0VJ3+Oq1Wl2mtDiMGaydWUMdGicRLvs0Dirs9m6tMYQBArVhK Iqh1+olNsjZaLN6GhIKBBvDEWGDdD/YsSnz4CzYCn5cUsNqAhcQcTCLqiuxTD3Irr34= X-Gm-Gg: AZuq6aJ1pY8Rw+iGQQuvi51hDOsZa4M326hpNUDuPGyZtOk4UuCfgGDd+TtJHdk8sfp /C4Kvb5hhaPTdhGXxZ8Xm6MYbdYN5oNmTQrGIreY0JvxXqnQkxAyw4V0NKTvhoJ5LWpPoMKUmva Q8hU94P4I3sfMvmygWlBQvutbHkUrNdrPmPWww+sCIUB5DlINuNvw5w6XoWba4aHX2W6Murr5NW yG6i//BRHrWOnANGmQKE57jQP18ZllRxnkjiglCZvYybIxzHcxy3/531YSJWikrkR+hrFsrFLsr r33cZJKqpIM3Yip6M+yeoEds9nKfLnsEAnPJxqe67sD1yf0yAge1og9OKhJV59VpXGt8LLj9Nvr 8I5eJ91zpvECfoSFwtbfQag4fTbRh2FluY/mT97yyx/uDzgT3HOrpHGSyXmShD3FY1Nt7Rqjq2H CPLkTu3w/vHXFfU3NR8FI7Hz96XQR11f6WGkPfvJ7GSrg= X-Received: by 2002:a05:600c:c165:b0:45c:4470:271c with SMTP id 5b1f17b1804b1-48320213840mr35367385e9.18.1770379931591; Fri, 06 Feb 2026 04:12:11 -0800 (PST) Received: from [192.168.0.101] ([90.240.106.137]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48317d835f0sm122385125e9.14.2026.02.06.04.12.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 06 Feb 2026 04:12:11 -0800 (PST) Message-ID: Date: Fri, 6 Feb 2026 12:12:10 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH] drm/sched: Add new KUnit test suite for concurrent job submission To: Marco Pagani Cc: airlied@gmail.com, ckoenig.leichtzumerken@gmail.com, dakr@kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, maarten.lankhorst@linux.intel.com, marpagan@redhat.com, matthew.brost@intel.com, mripard@kernel.org, phasta@kernel.org, simona@ffwll.ch, tzimmermann@suse.de References: <79cf2013-da6b-4653-aaa8-3e29a7d1ee3a@ursulin.net> <90cdb121-7ff2-43a5-9327-2898b5e65609@linux.dev> <36b2f7b1-f38e-41b6-b7e5-107a80c028d0@ursulin.net> <07c8c588-c2d6-463d-aabc-d472b8f38be8@ursulin.net> Content-Language: en-GB From: Tvrtko Ursulin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 06/02/2026 10:36, Marco Pagani wrote: > > > On 05/02/2026 10:53, Tvrtko Ursulin wrote: >> >> On 04/02/2026 16:33, Marco Pagani wrote: >> >> 8>< >> >>>>>>> + { >>>>>>> + .description = "Concurrently submit multiple jobs in a single entity", >>>>>>> + .job_base_us = 1000, >>>>>>> + .num_jobs = 10, >>>>>>> + .num_subs = 64, >>>>>>> + }, >>>>>>> +}; >>>>>>> + >>>>>>> +static void >>>>>>> +drm_sched_concurrent_desc(const struct drm_sched_concurrent_params *params, char *desc) >>>>>>> +{ >>>>>>> + strscpy(desc, params->description, KUNIT_PARAM_DESC_SIZE); >>>>>>> +} >>>>>>> + >>>>>>> +KUNIT_ARRAY_PARAM(drm_sched_concurrent, drm_sched_concurrent_cases, drm_sched_concurrent_desc); >>>>>>> + >>>>>>> +struct submitter_data { >>>>>>> + struct work_struct work; >>>>>>> + struct sched_concurrent_test_context *ctx; >>>>>>> + struct drm_mock_sched_entity *entity; >>>>>>> + struct drm_mock_sched_job **jobs; >>>>>>> + struct kunit *test; >>>>>>> + unsigned int id; >>>>>>> + bool timedout; >>>>>>> +}; >>>>>>> + >>>>>>> +static void drm_sched_submitter_worker(struct work_struct *work) >>>>>>> +{ >>>>>>> + const struct drm_sched_concurrent_params *params; >>>>>>> + struct sched_concurrent_test_context *ctx; >>>>>>> + struct submitter_data *sub_data; >>>>>>> + unsigned int i, duration_us; >>>>>>> + unsigned long timeout_jiffies; >>>>>>> + bool done; >>>>>>> + >>>>>>> + sub_data = container_of(work, struct submitter_data, work); >>>>>>> + ctx = sub_data->ctx; >>>>>>> + params = sub_data->test->param_value; >>>>>>> + >>>>>>> + wait_for_completion(&ctx->wait_go); >>>>>>> + >>>>>>> + for (i = 0; i < params->num_jobs; i++) { >>>>>>> + duration_us = params->job_base_us + (sub_data->id * 10); >>>>>> >>>>>> Why is job duration dependent by the submitter id? >>>>> >>>>> Just a simple way to have a deterministic distribution of durations. >>>>> I can change it if it doesn't fit. >>>>> >>>>>> Would it be feasiable to not use auto-completing jobs and instead >>>>>> advance the timeline manually? Given how the premise of the test seems >>>>>> to be about concurrent submission it sounds plausible that what happens >>>>>> after submission maybe isn't very relevant. >>>>> >>>>> Good idea! I'll run some experiments and see if it works. >>>> >>>> Cool, I will await your findings in v2. :) >>> >>> >>> After fiddling a bit with the code, I came to the conclusion that >>> changing the design to use manual timeline advancement is doable, but >>> not beneficial, since it would require serializing job submission into >>> "batches" using a two-step process, i.e., (i) workers submit jobs, and >>> (ii) the main thread waits for all submissions, advances the timeline, >>> and then releases the workers for the next iteration. >> >> What do you mean by next iteration? >> >> In the patch you have each worker submit all jobs in one go. > > I mean, if I change the code to use manual timeline advancement, then I > must introduce some synchronization logic that makes the main thread > advance the timeline only after the workers have submitted their jobs. Oh that, I was thinking advancing after flushing the workqueue would be enough for this use case. Since that one does not care about when completions happens they can just be cleaned up at the exit. > Since workers submit multiple jobs, I was thinking it would be better to > have the workers submit jobs in batches instead of all in one go. No strong opinion from me, as long as it is clear what is being tested. >>> This approach would partially defeat the purpose of a concurrency test >>> as it would not allow job submission (KUnit process context) to overlap >>> with job completion (hrtimer callback interrupt context) that models >>> asynchronous hardware in the mock scheduler, limiting contention on the >>> DRM scheduler internal locking. Moreover, while manual advancement might >>> appear to make the test deterministic, it does not since the order in >>> which the workers are scheduled will still be non-deterministic. >> >> Ah, so it depends what is the test actually wanting to test. In my view >> exercising parallel submit is one thing, while interleaving submission >> with completion is something else. >> >> In the test as written I think there is very little of the latter. Each >> worker submits all their jobs in one tight loop. Jobs you set to be 1ms >> so first job completion is 1ms away from when workers are released. A >> lot of the jobs can be submitted in 1ms and it would be interesting to >> see exactly how much, from how many workers. >> >> If desire is to interleave completion and submission I think that should >> be made more explicit (less depending on how fast is the underlying >> machine). For example adding delays into the submit loop to actually >> guarantee that. > > Fair point. > >> But I would also recommend parallel submit and parallel submit vs >> completions are tested in separate test cases. It should be easy to do >> with some flags and almost no new code. I was suggesting flags for some >> other thing in the initial review as well. Right, for auto-complete. So >> flag could be something like: >> >> +static const struct drm_sched_concurrent_params >> drm_sched_concurrent_cases[] = { >> + { >> + .description = "Concurrently submit a single job in a single entity", >> + .job_base_us = 1000, >> + .num_jobs = 1, >> + .num_subs = 32, >> .flags = INTERLEAVE_SUBMIT_AND_COMPLETE, >> + }, >> >> In the submit loop: >> >> + for (i = 0; i < params->num_jobs; i++) { >> + duration_us = params->job_base_us + (sub_data->id * 10); >> if (flags & INTERLEAVE_SUBMIT_AND_COMPLETE) { >> drm_mock_sched_job_set_duration_us(sub_data->jobs[i], duration_us); >> // Add a delay based on time elapse and job duration to guarantee job >> completions start arriving >> } >> + drm_mock_sched_job_submit(sub_data->jobs[i]); >> + } >> >> >> And of course handle the job waiting stage appropriately depending on >> auto-complete or not. >> >> Anyway, testing as little as possible at a time is a best practice I >> recommend, but if you insist, there is also nothing fundamentally wrong >> with the test as you have it so I won't block it. > > Agreed. Unit tests should test one functionality at a time and be clear > about which one. I'll follow your suggestions and have two separate test > cases: a basic one for concurrent submissions with manual timeline > advancement (no batches, workers submit all jobs at once) and then a > second one with automatic timeline advancement for testing interleaving > submissions with completions. > > At this point though, I think it would be better to move the tests to a > separate source file, as the partial similarity of the first concurrent > test case with drm_sched_basic_submit could create some confusion. Works for me, thank you! Regards, Tvrtko