linux-kernel - Re: [PATCH v7 3/6] rust: irq: add support for non-threaded IRQs and handlers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DBHU8JNG1P7I.NNDX9ZDT9DNU@kernel.org>
Date: Mon, 21 Jul 2025 17:28:35 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Daniel Almeida" <daniel.almeida@...labora.com>
Cc: "Alice Ryhl" <aliceryhl@...gle.com>, "Miguel Ojeda" <ojeda@...nel.org>,
 "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>,
 "Gary Guo" <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Trevor Gross" <tmgross@...ch.edu>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 "Thomas Gleixner" <tglx@...utronix.de>, "Bjorn Helgaas"
 <bhelgaas@...gle.com>, Krzysztof Wilczyński
 <kwilczynski@...nel.org>, "Benno Lossin" <lossin@...nel.org>,
 <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>,
 <linux-pci@...r.kernel.org>
Subject: Re: [PATCH v7 3/6] rust: irq: add support for non-threaded IRQs and
 handlers

On Mon Jul 21, 2025 at 5:10 PM CEST, Daniel Almeida wrote:
> Hi Alice, thanks for looking into this again :)
>
>
> […]
>
>>> diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..2f4637d8bc4c9fda23cbc8307687035957b0042a
>>> --- /dev/null
>>> +++ b/rust/kernel/irq/request.rs
>>> @@ -0,0 +1,267 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +// SPDX-FileCopyrightText: Copyright 2025 Collabora ltd.
>>> +
>>> +//! This module provides types like [`Registration`] which allow users to
>>> +//! register handlers for a given IRQ line.
>>> +
>>> +use core::marker::PhantomPinned;
>>> +
>>> +use crate::alloc::Allocator;
>>> +use crate::device::Bound;
>>> +use crate::device::Device;
>> 
>> The usual style is to write this as:
>> 
>> use crate::device::{Bound, Device};
>
> I dislike this syntax because I think it is a conflict magnet. Moreover, when
> you get conflicts, they are harder to solve than they are when each import
> is in its own line, at least IMHO.  

Intuitively, I would agree. However, I think practically it's not that bad.

While it's true that Rust has generally more conflict potential - especially in
the current phase - my feeling hasn't been that includes produce significantly
more conflicts then any other code so far.

> In any case, I don't think we have a guideline for imports at the moment?

No, but I think we should try to be as consistent as possible (at least within a
a certain logical unit, e.g. subsystem, module, etc.). Not sure where exactly
the IRQ stuff will end up yet. :)

>>> +/// A registration of an IRQ handler for a given IRQ line.
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// The following is an example of using `Registration`. It uses a
>>> +/// [`AtomicU32`](core::sync::AtomicU32) to provide the interior mutability.
>>> +///
>>> +/// ```
>>> +/// use core::sync::atomic::AtomicU32;
>>> +/// use core::sync::atomic::Ordering;
>>> +///
>>> +/// use kernel::prelude::*;
>>> +/// use kernel::device::Bound;
>>> +/// use kernel::irq::flags;
>>> +/// use kernel::irq::Registration;
>>> +/// use kernel::irq::IrqRequest;
>>> +/// use kernel::irq::IrqReturn;
>> 
>> /// use kernel::irq::{Flags, IrqRequest, IrqReturn, Registration};
>
> Same here. I’d rather not do this, if it’s ok with others.
>
> — Daniel