From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (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 B56FD3451C1 for ; Sat, 15 Aug 2026 20:25:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786825533; cv=none; b=CRpXywMSm/qcmaX0heaHQHuN4Dmfxb8R+ybBWNyLMLvmZAYgRkeUnydEKK26R8aF+XCuCD1tyArOs+tjWOKpxAgR6EMYDu/63gW4alJpmpgc6luCA92/RMKWj+5PGVKEJ4j5rh4Bwwun1I6pYK/nBolbIIS5cT4cD3LHx1h4FZw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786825533; c=relaxed/simple; bh=wo0QDV7tc8hQuUHblBQdWfiahU+O+7k91n/3wFhBqHw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: Content-Type:MIME-Version; b=haHOYxYRS2D6RTAP/Oh8WmletHYb94E2nqPC4qj5eR+EncIMAyvsxuiL6+n5mEqoWHdRDuWASomO45a7/w6oJo377yH6PzP0OOixHFJJcIn39Bn7UsoM0lSh8suIOnxDkKxteQ9nmgRQmkvjJ1wssJBLD7v6KmKoY7zHpafpurA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=HpKkI+T2; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="HpKkI+T2" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-4994d41ceb9so2973825e9.2 for ; Sat, 15 Aug 2026 13:25:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786825530; x=1787430330; darn=vger.kernel.org; h=mime-version:content-transfer-encoding:content-type:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to:content-type; bh=+ZlAyzPLsV68fOC9YLcWPEXPgYOdEPUukx8GNRQ7w8g=; b=HpKkI+T2LuFte8Z58u8F8ULahzaZCwQpGTyC+Sfsbb5yjc6SETgeys51leFBcU7OlV ocgseGO6H47Ta3Ut7IhSEAPxyDMx3DTscye7Lw8sXq1wJmNVpvG2E1RhbYnS/sJnTej3 TtUQeq9a8LMtamj/OmnvZL3P7MnKPMcHpWD2Iriu/TNrx8+4Kbz6eOCUYbRBrnqpcEVi x2FWLmAp/5w1ZgU7GCVIhasFnN/vCFbOOerK75myhrFMr8GHgw7IotkKUiPYMR15Mt1g r1r7ajm+LR7IW4ah/5oNoJqrxH0ZkyrTmuVkauHVFng9N4UqaqOfq4fGyoLD55/0qlJ5 zhnw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786825530; x=1787430330; h=mime-version:content-transfer-encoding:content-type:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=+ZlAyzPLsV68fOC9YLcWPEXPgYOdEPUukx8GNRQ7w8g=; b=i4Q5EE4bNuXQ+1hSxDRWRPFMb8mOtGkmTUvbVGk3BhyBpgIPbQtEfLhQgREXP1+vKd bmgSnQKF9rgBmGH6JLCgtSg43Hk8AHrHJGQvLgBnBxQVhGRyMLLQVv47RoLm2w6yu7qk qxxv9gsfNJigXXmLyvux7pp+TdKk7DhxSN9r76RTsqLl5vytRxUXeirsHMB746vFb9bh /FGrLMPq4oX7Fk5VI0uNIlENREWu/2TVVCYFEmJCNNO6kEtkKktoOUTBcivvv4L9fcEU hYSIonFtFwEoyU5nvEp/11smt2jpeWVeEKEAT2WeITT1ar8rTamj5yHMnXwj+uC/1CAc E/uw== X-Forwarded-Encrypted: i=1; AHgh+RpytjHNLBgoR/8MBXXxNit4u67bRSrHfGcAmr3/Lu3+HfB9/All3uh2obTIc0sFqL8zoif6sUhO0Z1ydDA=@vger.kernel.org X-Gm-Message-State: AOJu0YyCmy1f0GHYy1Nv8wK0hy1rW7bu5oomNGrkMox6PebAgYF1lFfB YBurcRzhv0YKozHYCfYjGioc0ZMlkX3fYrXHhnGaSOwgl89vg8wfIWIi X-Gm-Gg: AR+sD10bjq4VQC4+vx8pe4+EXEBedtq/re19QTnbhbw1iPmkbczLZ4qeIhxuUYTN4eU SRxiuubdALm6JHqpU0esxCg+Td7g3UUAM+6QLhPeSjr1TOERYPIfq3LdzJQwY2iPYQ/tQnZK0rl 7YKetM9aaxLKQCH4qgerG1dy1kbam1JmCCQ3bJbqIjixqJi7Y2bqoeW+FcQLq2XdH8NxxDFfwn4 AE+tWYCFrdIA2yGK8blnDFBwfQCze8UjOQYRXM6sxnNiwYBwzojzQn4Dsg+3EpXfsphegi0/YDq TVO1D0UjPNeikIaTvxNojqVnrEAbYktJYPsOHNwBQvXAEuaZiBTwWIV1QKKmSNzVrrWGLRmHqdi KyQAIRNFsg9VmkUND/RNuQqzs8eag+SqqZdgG7QPIzk5lbTXbzGGt5E/CSF9OaBCSYGSjsR+cwd JssaYKiWH7GZQXajgwJoMREEkprJY/KTi1+dYsIa7YgX2TaPc+C056h1ejichsWFlZsivyJ/3l1 EKBXfmTNndiyEQ1ZkwPI9EGo57VJlE0R7T8xvw5DtxlDcchregr X-Received: by 2002:a05:6000:60b:b0:47f:946b:d3fc with SMTP id ffacd0b85a97d-481607a9661mr9883130f8f.2.1786825529846; Sat, 15 Aug 2026 13:25:29 -0700 (PDT) Received: from [127.0.0.1] (ip-109-193-028-127.um39.pools.vodafone-ip.de. [109.193.28.127]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4815f2b1fc2sm19576914f8f.20.2026.08.15.13.25.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 15 Aug 2026 13:25:29 -0700 (PDT) From: Marek Czernohous To: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: sashiko-bot@kernel.org, linux-kernel@vger.kernel.org, Danilo Krummrich , Lyude Paul , David Airlie , Simona Vetter Subject: Re: [PATCH 1/3] drm/nouveau: destroy the fence event before cancelling its work Date: Sat, 15 Aug 2026 22:25:28 +0200 Message-ID: <178682552848.3774290.16460050233438707000@gmail.com> In-Reply-To: <20260815200914.8A1131F000E9@smtp.kernel.org> References: <178682366001.3748010.7798811159846779765@gmail.com> <178682366002.3748010.12779628082366287968@gmail.com> <20260815200914.8A1131F000E9@smtp.kernel.org> X-Mailer: python-smtplib Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The bot is right, and this is worse than a wording problem: 1/3 does introduce the race, it does not merely fail to rule it out. Please do not apply 1/3. 2/3 and 3/3 are independent of it and unaffected. What I missed is why the old order was safe in the first place. It was not an accident of ordering, it was load-bearing: nouveau_fence_context_kill() signals every fence on fctx->pending, and dma_fence_add_callback() returns -ENOENT for an already signalled fence before it ever reaches __dma_fence_enable_signaling() (drivers/dma-buf/dma-fence.c:707-710). So once the kill has run, nouveau_fence_enable_signaling() is no longer reachable for those fences, and nvif_event_dtor() afterwards has nobody left to race with. Moving the dtor to the front puts it exactly where those fences are still live, so nvif_event_allow() can be in flight on another CPU with nvif_event_constructed() already evaluated to true. There is nothing to serialise the two: nouveau_fence_context_del() takes no lock at all, enable_signaling() runs under fence->lock, which for nouveau is fctx->lock (nouveau_fence.c:218-219), and the dtor cannot take that, since the nvif ioctl may sleep and fctx->lock is taken with interrupts off. The window is then held open for the whole of cancel_work_sync(), which can block arbitrarily long. So my patch traded a narrow re-arm window for a wider NULL-deref window. That is a bad trade and my commit message argued for it with a "guard" that is a plain unsynchronised read of object->client. The re-arm problem the patch was aimed at is real, but the fix has to keep the kill in front of the dtor. The obvious shape is to move the drain to the back instead of the dtor to the front: nouveau_fence_context_kill(fctx, 0); nvif_event_dtor(&fctx->event); cancel_work_sync(&fctx->uevent_work); The kill closes enable_signaling(), the dtor then stops the handler, and the drain last picks up anything the handler queued on its way out. I want to convince myself properly that kill-before-drain is safe, rather than send a second version tonight on the strength of it looking right, so I will post a v2 once I have. Thanks to the bot for catching this before anyone applied it. For what it is worth, my own review pass had found the same mechanism a few hours earlier and I mis-filed it as a wording problem in the commit message instead of asking whether the patch itself was wrong. That one is on me.