Description
Looking at this patch that Christoph Hellwig NAKed
https://lore.kernel.org/linux-kernel/[email protected]/
and comparing to
https://github.com/torvalds/linux/blob/master/include/linux/dma-mapping.h
Maybe ask question why is there so much duplication. Then going hang on there is not enough information in the C header file. Then also remember sparse and other tools like it add bits to C code and Headers so they can function. So why cannot rust bindgen do the same.
The section of code that drew my attention in the NAK submit is this.
dma .rs
+pub mod attrs {
+ use super::Attrs;
+
+ /// Specifies that reads and writes to the mapping may be weakly ordered, that is that reads
+ /// and writes may pass each other.
+ pub const DMA_ATTR_WEAK_ORDERING: Attrs = Attrs(bindings::DMA_ATTR_WEAK_ORDERING);
+
+ /// Specifies that writes to the mapping may be buffered to improve performance.
+ pub const DMA_ATTR_WRITE_COMBINE: Attrs = Attrs(bindings::DMA_ATTR_WRITE_COMBINE);
+
+ /// Lets the platform to avoid creating a kernel virtual mapping for the allocated buffer.
+ pub const DMA_ATTR_NO_KERNEL_MAPPING: Attrs = Attrs(bindings::DMA_ATTR_NO_KERNEL_MAPPING);
+
+ /// Allows platform code to skip synchronization of the CPU cache for the given buffer assuming
+ /// that it has been already transferred to 'device' domain.
+ pub const DMA_ATTR_SKIP_CPU_SYNC: Attrs = Attrs(bindings::DMA_ATTR_SKIP_CPU_SYNC);
+
+ /// Forces contiguous allocation of the buffer in physical memory.
+ pub const DMA_ATTR_FORCE_CONTIGUOUS: Attrs = Attrs(bindings::DMA_ATTR_FORCE_CONTIGUOUS);
+
+ /// This is a hint to the DMA-mapping subsystem that it's probably not worth the time to try
+ /// to allocate memory to in a way that gives better TLB efficiency.
+ pub const DMA_ATTR_ALLOC_SINGLE_PAGES: Attrs = Attrs(bindings::DMA_ATTR_ALLOC_SINGLE_PAGES);
+
+ /// This tells the DMA-mapping subsystem to suppress allocation failure reports (similarly to
+ /// __GFP_NOWARN).
+ pub const DMA_ATTR_NO_WARN: Attrs = Attrs(bindings::DMA_ATTR_NO_WARN);
+
+ /// Used to indicate that the buffer is fully accessible at an elevated privilege level (and
+ /// ideally inaccessible or at least read-only at lesser-privileged levels).
+ pub const DMA_ATTR_PRIVILEGED: Attrs = Attrs(bindings::DMA_ATTR_PRIVILEGED);
+}
In my eyes this could be generated from the following from following section out of dma-mapping.h
/**
* List of possible attributes associated with a DMA mapping. The semantics
* of each attribute should be defined in Documentation/core-api/dma-attributes.rst.
*/
/*
* DMA_ATTR_WEAK_ORDERING: Specifies that reads and writes to the mapping
* may be weakly ordered, that is that reads and writes may pass each other.
*/
#define DMA_ATTR_WEAK_ORDERING (1UL << 1)
/*
* DMA_ATTR_WRITE_COMBINE: Specifies that writes to the mapping may be
* buffered to improve performance.
*/
#define DMA_ATTR_WRITE_COMBINE (1UL << 2)
/*
* DMA_ATTR_NO_KERNEL_MAPPING: Lets the platform to avoid creating a kernel
* virtual mapping for the allocated buffer.
*/
#define DMA_ATTR_NO_KERNEL_MAPPING (1UL << 4)
/*
* DMA_ATTR_SKIP_CPU_SYNC: Allows platform code to skip synchronization of
* the CPU cache for the given buffer assuming that it has been already
* transferred to 'device' domain.
*/
#define DMA_ATTR_SKIP_CPU_SYNC (1UL << 5)
/*
* DMA_ATTR_FORCE_CONTIGUOUS: Forces contiguous allocation of the buffer
* in physical memory.
*/
#define DMA_ATTR_FORCE_CONTIGUOUS (1UL << 6)
/*
* DMA_ATTR_ALLOC_SINGLE_PAGES: This is a hint to the DMA-mapping subsystem
* that it's probably not worth the time to try to allocate memory to in a way
* that gives better TLB efficiency.
*/
#define DMA_ATTR_ALLOC_SINGLE_PAGES (1UL << 7)
/*
* DMA_ATTR_NO_WARN: This tells the DMA-mapping subsystem to suppress
* allocation failure reports (similarly to __GFP_NOWARN).
*/
#define DMA_ATTR_NO_WARN (1UL << 8)
/*
* DMA_ATTR_PRIVILEGED: used to indicate that the buffer is fully
* accessible at an elevated privilege level (and ideally inaccessible or
* at least read-only at lesser-privileged levels).
*/
#define DMA_ATTR_PRIVILEGED (1UL << 9)
Yes I don't know enough Rust or rust bindgen to have to this 100 percent correct.
Something like the following inserted into dma-mapping.h just before the section I quoted out of dma-mapping.h
#ifdef RUST_BINDGEN
pub mod attrs {
use super::Attrs;
/* possible some transformation direction*./
#end
And this just after the section I quoted out of dma-mapping.h.
#ifdef RUST_BINDGEN
}
#end
Yes so that BINDGEN can get the information to generate what was in dma.rs so avoiding the sync issue.
I see this is the big source of friction in the Linux kernel. Not being able to put directions in the C header file means developers and up duplicating code/comments so causing a long term code maintenance problems by creating more areas in a code base to go out of sync that then results in the code not building so causing maintainers issues.
I also wonder how many hours are the rust Linux kernel developers putting into maintaining these abstractions every time something changes that should have been generated code that would have updated without human labor.