From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f74.google.com (mail-wm1-f74.google.com [209.85.128.74]) (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 43DB221C190 for ; Mon, 2 Jun 2025 16:19:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.74 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748881167; cv=none; b=OAUOHadIAGQK49+HLbisfYCQyrREnF2O5rZkUi521iRq++QniprfASDejDkxi5x8DlTNC6ayzJmUVDU2/XFcR8w+Dm4Xs+D07vO9rFl0J/SsZEL2JyBhh+aru5Of4zaIE0cJGgn+bNGF60aL/t1carq1UkNl3hUhdJSfSjx+oN0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748881167; c=relaxed/simple; bh=ljIrxYiJzxGe7Czc47ScoMraWr5MaF89zkDh0vKT6Tw=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=iGlLL3Gpn6BFYiN0wzW+DPpT/R/nRibkh4KVfEAA8pnJbSPz1x2VOxrNyhjcplz0BiibanXm+r5F/88blohbXESByDe6xLjITU5byzc9w9EVmmwI+CJ7b5ntBbOkKC/wfN+rEwJ/4nx4O6rCwMckAtm4RZtaEK3J+/r7FDtRkkw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=1mXTW4nB; arc=none smtp.client-ip=209.85.128.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="1mXTW4nB" Received: by mail-wm1-f74.google.com with SMTP id 5b1f17b1804b1-442fda1cba7so21732755e9.1 for ; Mon, 02 Jun 2025 09:19:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1748881163; x=1749485963; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=XuOsHwV8/b0lID06SIdHpZB8Xl8LCaN/OUfGvnqpXk8=; b=1mXTW4nBf73BVQjx9mCDmB6cLvSEk4nC74OM3gj6lEtfFzojAhPv7EcVMy2Rf15e/6 q7uvEDZlKGc40DLRVIFooTxW3+Bkd+SIVigz6E1ia+53tdsKlp1Pw+7h4unQp9aLvHTB Okw3Lq5BbbFbTm/ipslLxGq2QDsEIIbspqS3GLAUO06pOO4RXlXOA/o8J7E2yb5nNsye Om/m6SkY2ydczEOYMnUIFNmeFVbLeHTAD4k6LDfmSu4g2bs1ua+p7rSf5cKg466W5iW4 5r1Bx+4bu33GDzCdkvy7ztdN5ZK36dJ/vuu66ZGlikpIw4M55DwzFkM8fCBBonja1HDv mNZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1748881163; x=1749485963; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=XuOsHwV8/b0lID06SIdHpZB8Xl8LCaN/OUfGvnqpXk8=; b=sDkBAhyhHPsd2Wca5O7WnheZNshYuPCwvVYkjEZiGNDpqKqD6jirBo9kknxRR9x8KM /HDn+EWfA6wOh9Y4WK4OQFxyTQ0QwgmFHhepCL+0QpsriL3TrFv3t3MQJdi6Sj+3MCB5 CqUlkawQaDIZJZaL+JJvVAdnzQT6tW4eE5N6n705dX9A4+0SUvqkY4/K7W5tEiM+s6lN wcjS6FMxkzx4SFMtRRsnhmQlgrf4QXvMAWCVSN/E0YZ9gAQfSjx0Z4t6BnWeR0DhLq+X o10ssOg67rSZM9G/m0FlU6i0NFKswxur0pLGF5BFusf+KqJdaXPKDrSzkio7tDz4cDvl 6igg== X-Forwarded-Encrypted: i=1; AJvYcCXWkW5Q2X4e89+fafmSoUn4MfM10IucYIwWiZy+MnG4vDB2y3gzC6J9SJj4GvR00T2vdxeTpnoDnk63Q0DMFg==@vger.kernel.org X-Gm-Message-State: AOJu0Yx9C0rzxivfjmCbERoqO0o3kJ1Zfj8SzvyI+PcUwxNJ1GSIuI4r lf5iHYGPvUAw85218lje1+QzvDe/6hsiRiVEuJXMd1+yu813bvv+SxWkZP/2fasfuHWr67tQWmQ PAdvyX2tiIjYYyOQJAQ== X-Google-Smtp-Source: AGHT+IHSoq9Hg+bK+/GeaKnDjhYI/ippomoPHfGqVG5kdYjQ3Y/8lR8OvZ7OHaRyk7G5A2dqSYUHiV9mzFamY18= X-Received: from wmbbi20.prod.google.com ([2002:a05:600c:3d94:b0:451:d70f:eb87]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:600c:46cb:b0:442:d9fb:d9f1 with SMTP id 5b1f17b1804b1-450d882b051mr118531105e9.4.1748881163711; Mon, 02 Jun 2025 09:19:23 -0700 (PDT) Date: Mon, 2 Jun 2025 16:19:21 +0000 In-Reply-To: Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250514-topics-tyr-request_irq-v3-0-d6fcc2591a88@collabora.com> <20250514-topics-tyr-request_irq-v3-1-d6fcc2591a88@collabora.com> Message-ID: Subject: Re: [PATCH v3 1/2] rust: irq: add support for request_irq() From: Alice Ryhl To: Danilo Krummrich Cc: Daniel Almeida , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , "=?utf-8?B?QmrDtnJu?= Roy Baron" , Benno Lossin , Andreas Hindborg , Trevor Gross , Greg Kroah-Hartman , "Rafael J. Wysocki" , Thomas Gleixner , linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org Content-Type: text/plain; charset="utf-8" On Wed, May 14, 2025 at 11:53:21PM +0200, Danilo Krummrich wrote: > On Wed, May 14, 2025 at 04:20:51PM -0300, Daniel Almeida wrote: > > +/// // This is running in process context. > > +/// fn register_irq(irq: u32, handler: Handler) -> Result>> { > > +/// let registration = Registration::register(irq, flags::SHARED, c_str!("my-device"), handler); > > +/// > > +/// // You can have as many references to the registration as you want, so > > +/// // multiple parts of the driver can access it. > > +/// let registration = Arc::pin_init(registration, GFP_KERNEL)?; > > This makes it possible to arbitrarily extend the lifetime of an IRQ > registration. However, we must guarantee that the IRQ is unregistered when the > corresponding device is unbound. We can't allow drivers to hold on to device > resources after the corresponding device has been unbound. > > Why does the data need to be part of the IRQ registration itself? Why can't we > pass in an Arc instance already when we register the IRQ? > > This way we'd never have a reason to ever access the Registration instance > itself ever again and we can easily wrap it as Devres - > analogously to devm_request_irq() on the C side - without any penalties. If we step away from the various Rust abstractions for a moment, then it sounds like the request_irq API must follow these rules: 1. Ensure that free_irq is called before the device is unbound. 2. Ensure that associated data remains valid until after free_irq is called. We don't necessarily need to ensure that the Registration object itself is dropped before the device is unbound - as long as free_irq is called in time, it's okay. Now, if we use Devres, the way this is enforced is that during cleanup of a device, we call free_irq *and* we destroy the associated data right afterwards. By also destroying the associated data at that moment, it becomes necessary to use rcu_read_lock() to access the associated data. But if we just don't destroy the associated data during device cleanup, then that requirement goes away. Based on this, we could imagine something along these lines: struct RegistrationInner(i32); impl Drop for RegistrationInner { fn drop(&mut self) { free_irq(...); } } struct Registration { reg: Devres, data: T, } Here you can access the `data` on the registration at any time without synchronization. Note that with this, I don't think the rcu-based devres is really the right choice. It would make more sense to have a mutex along these lines: // drop Registration if devm_remove_callback() { free_irq(); } else { mutex_lock(); free_irq(); mutex_unlock(); } // devm callback mutex_lock(); free_irq(); mutex_unlock(); This avoids that really expensive call to synchronize_rcu() in the devm callback. Of course, for cases where the callback is only removed in the devm callback, you could also do away with the registration, take a ForeignOwnable that you can turn into the void pointer, and have the devm callback call free_irq followed by dropping the ForeignOwnable pointer. Alice