From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from CH4PR04CU002.outbound.protection.outlook.com (mail-northcentralusazon11013033.outbound.protection.outlook.com [40.107.201.33]) (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 6F19A3FADED; Tue, 9 Jun 2026 10:54:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=fail smtp.client-ip=40.107.201.33 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781002444; cv=fail; b=m2dWfV6jF2nwUS1XpMTjpiW55yg8H5QeGqMELpC+0Sb67HQE4QqLcDo3rUwa8eNRh32SX4Qr7aLa79SJkrwqvAp9Sekb6ylFCP365y0Mr6Pxes8NVRFNj3yFBAQk8U9vdSSK0CekMQxWbCFeeq8v5h7MhcoQZXFn6osk5LKvHKw= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781002444; c=relaxed/simple; bh=9c8BP74mrLmuWrEVpmUeAnvzu0FaMNJADGiATilP/7c=; h=Message-ID:Date:Subject:To:Cc:References:From:In-Reply-To: Content-Type:MIME-Version; b=KpdI46Z8fHU98Jx7B0s3QRzdTjXTOu9LV3vw2wV99RL52xLIekxV/X2VlnEs9cdgzcd9nhYobW8vuq7P4WIkyAEgVeUe+lSHyx+3REj92G8EC2gWtVrZQFghDmFJ2pM7/9a1RrPTPq9mBTZUMIsOT+CjlFvpDXHjHGlLZrs778Q= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amd.com; spf=fail smtp.mailfrom=amd.com; dkim=pass (1024-bit key) header.d=amd.com header.i=@amd.com header.b=q/oWqugt; arc=fail smtp.client-ip=40.107.201.33 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amd.com Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=amd.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=amd.com header.i=@amd.com header.b="q/oWqugt" ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=gARJJqx5R3fci53P/4LJmT21rE3dfDO5Rc8lismWOyU7mq/2cWm9r+FOiW2iSULrwTbIxXSPe3F8QBEpduXiTUHc63Ef5r2Jumu8wQPQOWRSlubf+SJ45gq5vZwMPfs0MmKHGc8aTWwJe74PdiPyWuls0NrsdtLAeiqBKubqeAo4hAKZMJX/mqXjxwgrWFCt4XJeFxxbMIQTAkMoVHcHG7ySNr4TWyuYPr4gJ6S3EDzScsbJvkBnkTZAVQDKjrZ7PWJzy0RWoxipqdP2kGD0G2LCP1BJ9IEYDQ50qzS2aUdtj1IQzFo2DB/NSTCPvjGBdAIP4UwVbLDk4R48NT/NsQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=cZBPAxLXefDlMlbobriMFU0V+abEItqwn6kuAYAfHJY=; b=KcQnWvsHrzwWqbyIussZ9Eo1zbfnypMgelOkfBoTPmHJgj0Ym5VoC9AYnTFfmARa0cOIFZFVPAZOpLa8Akgl/E8peaQ/IdTYPDKNimLmoGn+zH0TWdJO4VFb0rpeGkj4sH8dzIDvY+6sz6Plb26btN0g7uRVNN3ZYV/2Mmuisrg52hG39iGg43wGd8tcY4lswA4kQW5cJNf1LbzFM6u7ycuN9x1WpHwmtHMkdyeQun3zZhK/xeW+QodQlCXDhA14NfW3RyqHD8XMF5Jtr43yQhRpRfXl8bIMCe13r4bQRLZgpAUx3gIPzn0JsOFfNUqcJh0OQUMUpCTgEf8F4QGu9Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cZBPAxLXefDlMlbobriMFU0V+abEItqwn6kuAYAfHJY=; b=q/oWqugtgZWfzddP8fWJFuUvfzBw0ZukglpO1awK5+VsIkddNHNKpMnYOzJkudRIYQ8gA4dEQXB74Hdc6YMh88Y8eBiMHi5DTsQWK0wMsDdlnhYwKldhcT0KNse6qq4c6sROVGc0/pkl2r7Pa8+q+DdDoqGWfXNLK7NzLCLq04o= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from PH7PR12MB5685.namprd12.prod.outlook.com (2603:10b6:510:13c::22) by SJ2PR12MB8718.namprd12.prod.outlook.com (2603:10b6:a03:540::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.92.12; Tue, 9 Jun 2026 10:53:58 +0000 Received: from PH7PR12MB5685.namprd12.prod.outlook.com ([fe80::ce69:cfae:774d:a65c]) by PH7PR12MB5685.namprd12.prod.outlook.com ([fe80::ce69:cfae:774d:a65c%5]) with mapi id 15.21.0092.006; Tue, 9 Jun 2026 10:53:57 +0000 Message-ID: <1bb5efeb-a5d3-4d0b-ae69-8dc8620604d4@amd.com> Date: Tue, 9 Jun 2026 12:53:50 +0200 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH] dma-fence: Fix races of fence callbacks versus destructors by locking To: phasta@kernel.org, Danilo Krummrich Cc: Sumit Semwal , Boris Brezillon , Alice Ryhl , Daniel Almeida , Gary Guo , Tvrtko Ursulin , linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20260608142436.265820-2-phasta@kernel.org> <95f4ae6b-9dec-4122-84e0-fbb0cdee9cb5@amd.com> <9d49c901-fcdf-487a-a733-0320d0bdf94c@amd.com> <74bd33a06b75c291c3e2eda19e0250fbd280c49b.camel@mailbox.org> <66349a9f-d9dd-498b-b118-1c79d3aa3cca@amd.com> <11d7c83185a18d13760b6e77275e97c110dcddcc.camel@mailbox.org> Content-Language: en-US From: =?UTF-8?Q?Christian_K=C3=B6nig?= In-Reply-To: <11d7c83185a18d13760b6e77275e97c110dcddcc.camel@mailbox.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: BL0PR05CA0015.namprd05.prod.outlook.com (2603:10b6:208:91::25) To PH7PR12MB5685.namprd12.prod.outlook.com (2603:10b6:510:13c::22) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: PH7PR12MB5685:EE_|SJ2PR12MB8718:EE_ X-MS-Office365-Filtering-Correlation-Id: 78cc45c3-9c54-434d-51be-08dec61567aa X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|7416014|376014|1800799024|366016|11063799006|56012099006|4143699003|22082099003|18002099003; X-Microsoft-Antispam-Message-Info: rqyPXuCWpdPY8+GCZq9Gh/u0gPL50xwd5mWdPlEWc5mFzq2+Q4h8cbXSykbciir9s0MUpLzhKli87CSvm7ZtXusavxaTjn57b29HbPBLmDGNpON0UCjXDbucZkjOvZBaJu7RfT2/a+w1B48f1dd5MgUvci0udCxjk8Pukppxwx0VIkcuIRLYBoVTWxEA5rHekqZMKzuNNzpvt21i1T9n+hub4GRQjVZW7WZike16S4bbLMBlBq9vTvoxGpHF7GiA82/egijPS2HHH1ZeD5zfw5BMnGtziwqsjrb5ydLhFCkggWu1YIdumUKxTbrW/TFFxbIF2kKQVmw+i+kcGZot1DYRTDxn5xh84ySK7NAJoYUvUwWUZSq4ho7aqIqZuKsA+GOPHAMyU1XXKRYll1BkLeeMps9eVIofC0gUgckx5qrSEWzRug4Wzsm4v+awo6CFTDkzwH3xRaIh98AyfQMrFBPYwK+uqhSdjsxtLDNlbsGOd10z7za8vSu0VoqeBoMD/UEoGH6olcqTVRnUAmWj0l0aSSYbx8OZSvd4A/Pw7f+CgnXTV+kf9dd7ISq1vWJQcugIW9uQIrnuAqfKAvhW0P+fqCbKLHTni8v55GP8a06QHY5Nn13bFPfoOsC3jWFTyxPz6+VNXqtxWUUf4cDA7A== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PH7PR12MB5685.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230040)(7416014)(376014)(1800799024)(366016)(11063799006)(56012099006)(4143699003)(22082099003)(18002099003);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dGdqU2NORTJOTjBTdTNVbjFPMFh1R0JrODRTQnQ1ZGVKbytMN3UyNUdjWFdz?= =?utf-8?B?akJzdTUwTzNDVThtV2c4TlRTWU5YU2xuMzR5eFh0WWx1S3pDdVBCOCtWRWZM?= =?utf-8?B?bjB6WXUzMFJoTHpVK2NDM09MREhRQUcraWw4d1BtQ3JmRmVLdFZOaFd1N0Nz?= =?utf-8?B?R0N5Q0ZOMzBjblR2UGJLcDZiZXpxeW1xU0xneDQ3SG85TlZ3d1RNMHBJSlpy?= =?utf-8?B?TUIwM081QzBWeGwrY0VmbW94ejVGU0lkeVFmcU9UUFI0VGpGLzQ2ZEJwTkNm?= =?utf-8?B?UHdnOXFVeDZ4aGpoZkJ6T2thSU5JK093MEFoaXdpTjdRZ29vdHZ5Z1FWY0tm?= =?utf-8?B?aFJsWkI2cGFxUU0xeElhbXVSZjVuVDJIeUFrNFlYaEtKMGVvdnZ5VE5RRFRo?= =?utf-8?B?R1kxbGhWMjBDRmZGK2gwVGFGSkF2ZkxTYVBOMzJKVTFSRGRZZVRPKy80ZTEr?= =?utf-8?B?QldrOFJ3U2tXeTdEZ3lkN2VvRG5xUUg5ZUEveFJSdi9jdkwyQ01lVjRNaHky?= =?utf-8?B?YWd1cFEyMnd2RW9DdFhSZW9pcTkzRFBLWE5zUXdXajA0dVcwL1pjL3N1OXVY?= =?utf-8?B?TWgwWEQzM0NKRlpEbVNaVk50MW1ORjFmZVV6cDNjU3A0MnpaaDhEVldoMWxW?= =?utf-8?B?NXdIejloQnpCSlQyZGpXaHZBT3QvWnVBclRmcVhrb1JCMzV1a01GRW5XUXh5?= =?utf-8?B?N1pzeXRCUE5sdzdWZlRUOTBaNWR2cmJiT0x4OFU3bVBPSm1panJmNmNKVGJw?= =?utf-8?B?eDhKT1FxSjZnS29YRUlIOWdLeXhaN1hOWTVCVi9hek42dGVEZnhPYmVlWFlM?= =?utf-8?B?ZXBIVmN2ZkpsTytLV0NLSkJpdFR0MW1pTVVSOFVueG5VYUxyeGhtemZpaDJH?= =?utf-8?B?VmtLN2o0NUJVUy9Lb1UrSEI3YzZyQlppQklxNlY1OWZ6NEU0WXJSNFVNUTBB?= =?utf-8?B?eGdXQTBkM0lqRWtna2Q2Q1JDZmxsWDZzTit5OVJxRTJOVDJtNGNWVjdoaXRW?= =?utf-8?B?ZkZqMHVVbDlTQlpKZ083OWVPNWM1VFdVU2tHZURqRWtqMVdhN2xKNEU1NGRs?= =?utf-8?B?WGdQZVNUQThpRTU1K2xFUFZVSHkySWJzRjM5UkZ1SjMwNThjR0w3ZEh0aFAy?= =?utf-8?B?dktQdWxqVGJRRjlVQ3BtWDJDNFA3Rk5mTnZ6L0cvNzFxKzN2UEJXbVRPWGJN?= =?utf-8?B?ZzVGTjdmYWRXZHI1MlI3WnNXOHBpMTdVMTAzYytuRis5NEhBWHBKRWdSL00z?= =?utf-8?B?Y3d6V3pyTFFCVjZYYkY1NXdyV3lRUGh3ZmNXUHNPWkFDa2NHOTcycXRnbWk1?= =?utf-8?B?d2N5OFI0bFlFTVdCdTFpUVNONEk2M2dsUmQ5Y1F4TlNmdW5pVGM1bUxMSVNN?= =?utf-8?B?NUdQTzNEYXhBQXVFa2YwWE1MeFBwRzlqbjBPaldvNVI0dFVCZTlJY1JzRnk5?= =?utf-8?B?VHFlQzdiNEoyQkVwMmFTNzRLY1lyR0c1NjhJUGFYSDQxWC9aNDJnVVFEeVNU?= =?utf-8?B?bElVRjBJRVFVdFo1YjJES3gzYUxkdUVuMVczK3Q5UWpFRHB5Y1ZQWUVZbUwr?= =?utf-8?B?QWVEY3hCdUpRdnJpaEhSZjY0Z3ZGZ1JhbS9EVEZ6dXFWNjk1R0xOUDlGZjQx?= =?utf-8?B?UDMyWmJ5b2x0WU5CVDFJRCtlV1NNZU9tOHdlUE9ZWGpJdytSZmc2ZUoxbDBB?= =?utf-8?B?Ty9wbmk5MHo2ZEVoSWNFMTU2MjY5WEMyWndLWWhMN3RWejhIelFNQmJCVC92?= =?utf-8?B?K3pFaHFrSGtzRnlsZ3FTNXN4bEJ3NVRsMDNROForMDdlQS9lbkFMZHU2a0No?= =?utf-8?B?QVFzOFc5NDAzeGN4V3hLT2F6SGU1Q0RBM0dkeVhJRzFsQUhObGlvcmpDRGVk?= =?utf-8?B?cnV2aFViRUJnUEtJU2RqVE9IRWJtMEJMcHhFbXFtTC90R2wzbjdoMU80S1g3?= =?utf-8?B?UHR6V0M2WGFHVnhKUUMzMzAxMk1XTEkxNVV5ZUdCd1lvRWN2NTBLTHZyQVNM?= =?utf-8?B?dXRDZ3hRUEl2RmNYMytGNUdrMzFTa1FMNGU5VzVOZGw0d1hiVkpETVNHelZ1?= =?utf-8?B?OW1LUmFvaTl5K1VQeTk0MGJQVUQrSWRQNDQyZTNxazFHMlBXVHRrMGt1dmdu?= =?utf-8?B?VnZTWGovc3VZclNDanhiQlUxZ0dXL1IwdFlZNE4zUEJabWRzUVQ5WFFlZGJ6?= =?utf-8?B?M0NDY1ZMeWY0a05KOWNmKzRkKzZ1Yy91MzRScERJbzRzY0JJeE9nTkVGNlpn?= =?utf-8?B?NGd5T1RpT0g2TXRzZExLY1lTWXZmWWtUaXJhRExNcEZQRUVPMVNpUU13Smtp?= =?utf-8?Q?A8G8Ao5VwrXambQSsu?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 78cc45c3-9c54-434d-51be-08dec61567aa X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB5685.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jun 2026 10:53:57.0614 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: bIMsw1/gbOjXr4drKMUVOUeIpnwvxfXhRLQc+FovP47sfBjkIPH9c8vJw0a5qVuG X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB8718 On 6/9/26 12:42, Philipp Stanner wrote: > On Tue, 2026-06-09 at 12:26 +0200, Christian König wrote: >> On 6/9/26 07:52, Philipp Stanner wrote: >> ... >>>> >>>> In detail calling the callbacks without holding locks allows all implementations who need it to explicitly take locks in the order they want. >>> >>> Didn't you say a few mails above that the implementation should not use >>> the fence lock for its own purposes? >> >> The usual use case the drivers have is this here: >> >> dma_fence_lock_irqsave(fence, flags); >> was_signaled = dma_fence_test_signaled(fence); >> if (!was_signaled) >> dma_fence_signal(fence); >> dma_fence_unlock_irqrestore(fence, flags); >> >> if (!was_signaled) > > If cleanup() touches fence data, this is now exactly the race that I am > concerned with. > > With the current design, you'd actually need a synchronize_rcu() here, > which you definitely do not want in a hot path :( > >> cleanup(); >> >> This is actually what you and me came up with for the KFD when we >> removed the return code for dma_fence_signal(). > > Well, what we came up with for that rare case was > > was_signaled = dma_fence_check_and_signal(fence); > if (!was_signaled) > cleanup(); > > It seems that many of the other use cases where the fence lock is taken > by drivers ultimately are rooted in the (IMO) design mistake that > dma_fence_set_error() exists. > > There should just be > > void > dma_fence_signal(struct dma_fence *f, int err); > > which also conveniently forces all signalers to really carefully think > about whether the fence's associated operations succeeded. Correctly > representing the "true fence error state" to all consumers is vital for > sane system behavior, as you continuously have to point out. Yeah that came to my mind before as well. >> >> Taking the lock around the enable_signaling() callback has the exact >> same reason, preventing the fence from signaling between testing and >> calling dma_fence_signal(). The problem with that approach is that >> cleanup() now suddenly runs under the fence lock as well. > > That problem does not exist if we re-design dma_fence like that: > > // driver > dma_fence_signal(f); // revokes all accesses to our driver through backend_ops > // synchronize_rcu() now unnecessary \o/ > cleanup(f); // We know that all accessors are gone > dma_fence_put(f); Yeah and exactly that doesn't work. Just think about the Nouveau case when you have your fences on a double linked list. When the fence lock is independent, e.g. have a separate lock for each fence then this lock can't protect this double linked list. So your cleanup path needs to take a lock which protects the list, but you then run into lock inversion. >> >> So you are left with few options: Either the fence lock is external, >> which we don't want because that make the fence non-independent, or >> cleanup() defers work to irq_work or work_structs, which creates >> numerous lifetime issues. > > Yup, this is uncool and we want to avoid that. > > But these seem to be the options > > 1. Ensure proper synchronization > 2. Wait for a grace period in a hot path > 3. Defer cleanup() with some delay mechanism > > #1 is by far the cleanest approach. I still cannot see any downside, > and quite a few upsides. > > https://elixir.bootlin.com/linux/v7.1-rc6/source/drivers/dma-buf/dma-fence.c#L1025 > > ^ is already racing with the signaled check. Yeah so what? That is just an opportunistic check. > I'm going through the users right now, and it seems all we need to > ensure to implement this change is to audit all ops->signaled() for > usage of the fence_lock and see if we can port them. > >> >> When enable_signaling would be independent of the fence lock we could >> avoid all of this and just use a normal spin_lock() for the cleanup. > > How's that related to our problem? See the explanation above for the Nouveau example. As far as I can see the approach you want to implement here is a NO-GO from my side. Regards, Christian. > > P.