Re: [PATCH v7 07/16] rust: add `io::{Io, IoRaw}` base types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 25, 2025 at 04:50:05PM +1100, Alistair Popple wrote:
> Kind of, but given the current state of build_assert's and the impossiblity of
> debugging them should we avoid adding them until they can be fixed?

If you build the module as built-in the linker gives you some more hints, but
I agree it's still not great.

I think build_assert() is not widely used yet and, until the situation improves,
we could also keep a list of common pitfalls if that helps?

> Unless the code absolutely cannot compile without them I think it would be
> better to turn them into runtime errors that can at least hint at what might
> have gone wrong. For example I think a run-time check would have been much more
> appropriate and easy to debug here, rather than having to bisect my changes.

No, especially for I/O the whole purpose of the non-try APIs is to ensure that
boundary checks happen at compile time.

> I was hoping I could suggest CONFIG_RUST_BUILD_ASSERT_ALLOW be made default yes,
> but testing with that also didn't yeild great results - it creates a backtrace
> but that doesn't seem to point anywhere terribly close to where the bad access
> was, I'm guessing maybe due to inlining and other optimisations - or is
> decode_stacktrace.sh not the right tool for this job?

I was about to suggest CONFIG_RUST_BUILD_ASSERT_ALLOW=y to you, since this will
make the kernel panic when hitting a build_assert().

I gave this a quick try with [1] in qemu and it lead to the following hint,
right before the oops:

[    0.957932] rust_kernel: panicked at /home/danilo/projects/linux/nova/nova-next/rust/kernel/io.rs:216:9:

Seeing this immediately tells me that I'm trying to do out of bound I/O accesses
in my driver, which indeed doesn't tell me the exact line (in case things are
inlined too much to gather it from the backtrace of the oops), but it should be
good enough, no?

[1]

diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 1fb6e44f3395..2ff3af11d711 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -13,7 +13,7 @@ impl Regs {
     const OFFSET: usize = 0x4;
     const DATA: usize = 0x8;
     const COUNT: usize = 0xC;
-    const END: usize = 0x10;
+    const END: usize = 0x2;
 }

 type Bar0 = pci::Bar<{ Regs::END }>;




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux