From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com [136.143.188.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 609C52D3739 for ; Mon, 3 Aug 2026 13:21:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=136.143.188.112 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785763314; cv=pass; b=F5RdSmM0+2GXtsuvO98pXHC5CSOarSbTR/b8RcGVH4OZXdf7lLgEdgjMwBFVa9LjBOufTNLWW9WM0RRd7th/c1Kkatrf3hFHYBffDbnmsyQhkDi9YM1ImuVSc34T7A1QLEZoC3Biv7XKzyV62yK2iWIw1VbYqmPtrcX1JvMtM4U= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785763314; c=relaxed/simple; bh=vQLFylI5WCLelO1ql/20b+iWvE6QuGMq/ifPZ4PcB04=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=uPK1Ww+TTekWW1tjQczjGiYxKm96aqsoghmLsBVtVv8QACpOWXfYqGQPRVedauIsX1Tgfc5DN2ls6v066zrxhcPxKyp0Ggf41BIYnLptkpaEp2gtvAd1cF0Qp7ZyCw7x8BLXZeLMXGOI6YUbbtzanbA2+PJtVIolYcGLF6KBo3Y= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (1024-bit key) header.d=collabora.com header.i=nicolas.frattaroli@collabora.com header.b=A30PE+Vf; arc=pass smtp.client-ip=136.143.188.112 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=collabora.com header.i=nicolas.frattaroli@collabora.com header.b="A30PE+Vf" ARC-Seal: i=1; a=rsa-sha256; t=1785763223; cv=none; d=zohomail.com; s=zohoarc; b=EvVRl/yXstn8NkEx27l4Z5npudKXR2KuJnjosYfUq+5qbLqHCgP5dvaXbHXd636UqUV4LnXx8XyULv1ZPkeRJea0OOqoV3vIykSzZB2ZHbm8ukCnTJJmcc7N2YaPnk1iBAkShZ/lqmaco6/t/A8GTQbHy2PvdgbzpIfqL52DwbE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1785763223; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:Subject:Subject:To:To:Message-Id:Reply-To; bh=3XDs2AL6dDQ4H+CdWU65MVevEoVYOrc0yGy3mX2p2jE=; b=CNrEOnakREEaj1lXADE8lJD5R0McKwFAf+fX7lCbj/vigvMKNNrIgT+ADDVM7Hwg4cUJobTmF2qJzqjc442TMurLFhO95dmS1LdgLauzK35YvIomR9ocAAdlijtX7FHGMgVijii0Onx3pPG5ZUhR7gi2d3a4CaLuqb7FOCz2Ous= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=collabora.com; spf=pass smtp.mailfrom=nicolas.frattaroli@collabora.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1785763223; s=zohomail; d=collabora.com; i=nicolas.frattaroli@collabora.com; h=From:From:To:To:Cc:Cc:Subject:Subject:Date:Date:Message-ID:In-Reply-To:MIME-Version:Content-Transfer-Encoding:Content-Type:Message-Id:Reply-To; bh=3XDs2AL6dDQ4H+CdWU65MVevEoVYOrc0yGy3mX2p2jE=; b=A30PE+VfdbTWRMbr585ir6P6g2kEFP6YXQt2tbkYWoggOXt1JSt5BFXQJTziwoXr v7RBojQnjt2Oe9iBXlo6yl5s56gO2l8uiprhS89qOdc+oLABFbT2mx5pprcvJFi15jW lt8naiysz7DOzGZzf6WMZXOiY701X9Z8GowP7OsI= Received: by mx.zohomail.com with SMTPS id 1785763222071159.7552701038611; Mon, 3 Aug 2026 06:20:22 -0700 (PDT) From: Nicolas Frattaroli To: Boris Brezillon Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , K Prateek Nayak , Steven Price , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Grant Likely , Heiko Stuebner , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, kernel@collabora.com Subject: Re: [PATCH v2 2/3] drm/panthor: Revisit reqs_lock handling in flush/reset paths Date: Mon, 03 Aug 2026 15:20:14 +0200 Message-ID: In-Reply-To: References: <20260730-panthor-cache-flush-fix-v2-0-28790478bfff@collabora.com> <20260803105324.478ed594@fedora1.home> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" On Monday, 3 August 2026 15:13:25 Central European Summer Time Nicolas Frattaroli wrote: > On Monday, 3 August 2026 10:53:24 Central European Summer Time Boris Brezillon wrote: > > Hello Nicolas, > > > > On Thu, 30 Jul 2026 13:45:15 +0200 > > Nicolas Frattaroli wrote: > > > > > panthor_gpu_flush_caches() and panthor_gpu_soft_reset() would read (and > > > even reset) the contents of the pending_reqs register outside of holding > > > the reqs_lock. > > > > Can you elaborate a bit on the race being fixed here? If pending_reqs bits > > are truly cleared before the wake_up_all() call (which would require a > > WRITE_ONCE() to be enforced, admittedly), there's no risk for the > > wait_event() call to do a test before the bits have been updated, > > and this holds even if the test is done without the lock held. > > > > The other race I could think of is two threads calling > > panthor_gpu_flush_caches() concurrently, and the second one stealing > > the FLUSH_COMPLETED event the first thread waits on and re-issuing a > > second flush on top, thus delaying the completion for the first thread. > > But that should be covered by the cache_flush_lock. > > panthor_gpu_flush_caches() is not the only thing that sets/gets > pending_reqs. Notably, the threaded interrupt handler does, as > well as any other functionality using the same member for reqs > tracking (e.g. the soft reset). > > Consider the following serialisation of events: > 1. T1 asks to flush caches by writing GPU_CMD and setting pending_reqs > 2. T1 drops reqs_lock. > 3. T2 enters IRQ handler for flush complete, spins lock waiting for > reqs_lock Minor correction: Imagine 2 and 3 reversed here in a non-IRQ-disabling, variant, otherwise "spins lock" does not make sense. With an IRQ-disabling variant, ignore the "spins lock waiting for reqs_lock" part. > 4. T1 sleeps at wait_event_timeout > 5. T2 updates pending_reqs and wakes up the waiter in any order, since > the effects of those two can't consistently be observed as sequential > logic without the outer reqs_lock being held by the observer > 6. T1 wakes up, checks pending_reqs, but since pending_reqs is checked > without holding any lock, so we implictly depend on the synchronisation > point that is the waitqueue's lock rather than the reqs_lock spinlock, > which says nothing about whether the pending_reqs change materialised > on T1's side yet as far as I can tell? > 7. T1 sees that pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED is still != 0, > so goes back to sleep for some future wake-up of reqs_acked or a timeout. >