In the structs dma_fence_array and dma_fence_chain, the field 'lock'
has been removed, but its documentation comment remained. Remove the
stale descriptions to clear up the following kernel-doc warnings:
WARNING: ./include/linux/dma-fence-array.h:47 Excess struct member 'lock' description in 'dma_fence_array'
WARNING: ./include/linux/dma-fence-array.h:47 Excess struct member 'lock' description in 'dma_fence_array'
WARNING: ./include/linux/dma-fence-chain.h:48 Excess struct member 'lock' description in 'dma_fence_chain'
WARNING: ./include/linux/dma-fence-chain.h:48 Excess struct member 'lock' description in 'dma_fence_chain'
Fixes: 5943243914b9 ("dma-buf: use inline lock for the dma-fence-array")
Fixes: a408c0ca0c41 ("dma-buf: use inline lock for the dma-fence-chain")
Signed-off-by: Nicolás Antinori <nico.antinori.7(a)gmail.com>
---
include/linux/dma-fence-array.h | 1 -
include/linux/dma-fence-chain.h | 1 -
2 files changed, 2 deletions(-)
diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
index 1b1d87579c38..0c49d7ccefb6 100644
--- a/include/linux/dma-fence-array.h
+++ b/include/linux/dma-fence-array.h
@@ -28,7 +28,6 @@ struct dma_fence_array_cb {
/**
* struct dma_fence_array - fence to represent an array of fences
* @base: fence base class
- * @lock: spinlock for fence handling
* @num_fences: number of fences in the array
* @num_pending: fences in the array still pending
* @fences: array of the fences
diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h
index df3beadf1515..42289f505164 100644
--- a/include/linux/dma-fence-chain.h
+++ b/include/linux/dma-fence-chain.h
@@ -20,7 +20,6 @@
* @prev: previous fence of the chain
* @prev_seqno: original previous seqno before garbage collection
* @fence: encapsulated fence
- * @lock: spinlock for fence handling
*/
struct dma_fence_chain {
struct dma_fence base;
--
2.47.3
Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`.
This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the
first entry, resulting in non-page-aligned DMA addresses for all
subsequent entries.
While the underlying IOMMU mapping may be contiguous, hardware
DMA engines often require explicit address alignment (e.g., page,
cacheline, or storage sector boundaries). Passing unaligned
addresses and lengths can cause explicit failures in DMA descriptor
creation or silent data corruption if lower unaligned bits are
truncated.
Fix this by splitting the scatterlist by the largest possible page
aligned chunk within `UINT_MAX` (`ALIGN_DOWN(UINT_MAX, PAGE_SIZE)`).
This ensures all scatterlist DMA addresses and lengths remain page
aligned and satisfy hardware constraints.
Page-aligned entries allow the system to cleanly chunk payloads into
PCIe MaxPayloadSize (MPS) (e.g., 128 bytes, 256 bytes, 512 bytes).
As a result, this may help reduce TLP fragmentation in P2P transfers
and alleviate potential congestion within a logical PCIe switch
partition, especially when Relaxed Ordering is not possible due to
hardware constraints.
Reported-by: sashiko-bot <sashiko-bot(a)kernel.org>
Closes: https://lore.kernel.org/all/20260609165431.778061F00893@smtp.kernel.org/
Fixes: 3aa31a8bb11e ("dma-buf: provide phys_vec to scatter-gather mapping routine")
Cc: stable(a)vger.kernel.org
Signed-off-by: David Hu <xuehaohu(a)google.com>
---
drivers/dma-buf/dma-buf-mapping.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
index 794acff2546a..f2bde38fdb1f 100644
--- a/drivers/dma-buf/dma-buf-mapping.c
+++ b/drivers/dma-buf/dma-buf-mapping.c
@@ -5,6 +5,9 @@
*/
#include <linux/dma-buf-mapping.h>
#include <linux/dma-resv.h>
+#include <linux/align.h>
+
+#define MAX_ENT_SZ ALIGN_DOWN(UINT_MAX, PAGE_SIZE)
static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
dma_addr_t addr)
@@ -12,9 +15,9 @@ static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
unsigned int len, nents;
int i;
- nents = DIV_ROUND_UP(length, UINT_MAX);
+ nents = DIV_ROUND_UP(length, MAX_ENT_SZ);
for (i = 0; i < nents; i++) {
- len = min_t(size_t, length, UINT_MAX);
+ len = min_t(size_t, length, MAX_ENT_SZ);
length -= len;
/*
* DMABUF abuses scatterlist to create a scatterlist
@@ -24,7 +27,7 @@ static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
* does not require the CPU list for mapping or unmapping.
*/
sg_set_page(sgl, NULL, 0, 0);
- sg_dma_address(sgl) = addr + (dma_addr_t)i * UINT_MAX;
+ sg_dma_address(sgl) = addr + (dma_addr_t)i * MAX_ENT_SZ;
sg_dma_len(sgl) = len;
sgl = sg_next(sgl);
}
@@ -41,14 +44,14 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
if (!state || !dma_use_iova(state)) {
for (i = 0; i < nr_ranges; i++)
- nents += DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);
+ nents += DIV_ROUND_UP(phys_vec[i].len, MAX_ENT_SZ);
} else {
/*
* In IOVA case, there is only one SG entry which spans
* for whole IOVA address space, but we need to make sure
* that it fits sg->length, maybe we need more.
*/
- nents = DIV_ROUND_UP(size, UINT_MAX);
+ nents = DIV_ROUND_UP(size, MAX_ENT_SZ);
}
return nents;
--
2.55.0.rc0.738.g0c8ab3ebcc-goog
The current name of `as_ptr` is very generic, and if you attempt to
invoke `foo.as_ptr()` on a type for which this method is missing, then
an error along these lines will be printed:
error[E0599]: no method named `as_ptr` found for reference `&DmaBuf` in the current scope
--> linux/rust/kernel/dma_buf/buf.rs:54:38
|
54 | ptr::eq(self.as_ptr(), other.as_ptr())
| ^^^^^^ method not found in `&DmaBuf`
|
= help: items from traits can only be used if the trait is implemented and in scope
note: `device_id::IdTable` defines an item `as_ptr`, perhaps you need to implement it
--> linux/rust/kernel/device_id.rs:165:1
|
165 | pub trait IdTable<T: RawDeviceId, U> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Suggesting the IdTable trait when an as_ptr() method is missing is not
useful. Renaming it to `as_raw_id_table` makes the method name unique to
this trait and avoids these bad suggestions.
Assisted-by: Antigravity:Gemini
Signed-off-by: Alice Ryhl <aliceryhl(a)google.com>
---
rust/kernel/auxiliary.rs | 2 +-
rust/kernel/device_id.rs | 4 ++--
rust/kernel/driver.rs | 8 +++++---
rust/kernel/i2c.rs | 8 ++++----
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 4 ++--
rust/kernel/usb.rs | 2 +-
7 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index c42928d5a239..fd6940577e0d 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -64,7 +64,7 @@ unsafe fn register(
(*adrv.get()).name = name.as_char_ptr();
(*adrv.get()).probe = Some(Self::probe_callback);
(*adrv.get()).remove = Some(Self::remove_callback);
- (*adrv.get()).id_table = T::ID_TABLE.as_ptr();
+ (*adrv.get()).id_table = T::ID_TABLE.as_raw_id_table();
}
// SAFETY: `adrv` is guaranteed to be a valid `DriverType`.
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 8e9721446014..821da02540b1 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -164,7 +164,7 @@ impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
/// `IdArray` doesn't matter.
pub trait IdTable<T: RawDeviceId, U> {
/// Obtain the pointer to the ID table.
- fn as_ptr(&self) -> *const T::RawType;
+ fn as_raw_id_table(&self) -> *const T::RawType;
/// Obtain the pointer to the bus specific device ID from an index.
fn id(&self, index: usize) -> &T::RawType;
@@ -174,7 +174,7 @@ pub trait IdTable<T: RawDeviceId, U> {
}
impl<T: RawDeviceId, U, const N: usize> IdTable<T, U> for IdArray<T, U, N> {
- fn as_ptr(&self) -> *const T::RawType {
+ fn as_raw_id_table(&self) -> *const T::RawType {
// This cannot be `self.ids.as_ptr()`, as the return pointer must have correct provenance
// to access the sentinel.
core::ptr::from_ref(self).cast()
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index bf5ba0d27553..69068adcbdae 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -341,7 +341,8 @@ fn acpi_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
// SAFETY:
// - `table` has static lifetime, hence it's valid for read,
// - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
- let raw_id = unsafe { bindings::acpi_match_device(table.as_ptr(), dev.as_raw()) };
+ let raw_id =
+ unsafe { bindings::acpi_match_device(table.as_raw_id_table(), dev.as_raw()) };
if raw_id.is_null() {
None
@@ -374,7 +375,8 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
// SAFETY:
// - `table` has static lifetime, hence it's valid for read,
// - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
- let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), dev.as_raw()) };
+ let raw_id =
+ unsafe { bindings::of_match_device(table.as_raw_id_table(), dev.as_raw()) };
if !raw_id.is_null() {
// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct of_device_id`
@@ -404,7 +406,7 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
// - `adev` is a valid pointer to `acpi_device` or is null. It is guaranteed to be
// valid as long as `dev` is alive.
// - `table` has static lifetime, hence it's valid for read.
- if unsafe { acpi_of_match_device(adev, table.as_ptr(), &raw mut raw_id) } {
+ if unsafe { acpi_of_match_device(adev, table.as_raw_id_table(), &raw mut raw_id) } {
// SAFETY:
// - the function returns true, therefore `raw_id` has been set to a pointer to a
// valid `of_device_id`.
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 624b971ca8b0..920794d4089d 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -116,17 +116,17 @@ unsafe fn register(
);
let i2c_table = match T::I2C_ID_TABLE {
- Some(table) => table.as_ptr(),
+ Some(table) => table.as_raw_id_table(),
None => core::ptr::null(),
};
let of_table = match T::OF_ID_TABLE {
- Some(table) => table.as_ptr(),
+ Some(table) => table.as_raw_id_table(),
None => core::ptr::null(),
};
let acpi_table = match T::ACPI_ID_TABLE {
- Some(table) => table.as_ptr(),
+ Some(table) => table.as_raw_id_table(),
None => core::ptr::null(),
};
@@ -208,7 +208,7 @@ fn i2c_id_info(dev: &I2cClient) -> Option<&'static <Self as driver::Adapter>::Id
// SAFETY:
// - `table` has static lifetime, hence it's valid for reads
// - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
- let raw_id = unsafe { bindings::i2c_match_id(table.as_ptr(), dev.as_raw()) };
+ let raw_id = unsafe { bindings::i2c_match_id(table.as_raw_id_table(), dev.as_raw()) };
if raw_id.is_null() {
return None;
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 5071cae6543f..311b4716800b 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -81,7 +81,7 @@ unsafe fn register(
(*pdrv.get()).name = name.as_char_ptr();
(*pdrv.get()).probe = Some(Self::probe_callback);
(*pdrv.get()).remove = Some(Self::remove_callback);
- (*pdrv.get()).id_table = T::ID_TABLE.as_ptr();
+ (*pdrv.get()).id_table = T::ID_TABLE.as_raw_id_table();
}
// SAFETY: `pdrv` is guaranteed to be a valid `DriverType`.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 9b362e0495d3..f6a48f7750da 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -63,12 +63,12 @@ unsafe fn register(
module: &'static ThisModule,
) -> Result {
let of_table = match T::OF_ID_TABLE {
- Some(table) => table.as_ptr(),
+ Some(table) => table.as_raw_id_table(),
None => core::ptr::null(),
};
let acpi_table = match T::ACPI_ID_TABLE {
- Some(table) => table.as_ptr(),
+ Some(table) => table.as_raw_id_table(),
None => core::ptr::null(),
};
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 7aff0c82d0af..14e0602c3f03 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -58,7 +58,7 @@ unsafe fn register(
(*udrv.get()).name = name.as_char_ptr();
(*udrv.get()).probe = Some(Self::probe_callback);
(*udrv.get()).disconnect = Some(Self::disconnect_callback);
- (*udrv.get()).id_table = T::ID_TABLE.as_ptr();
+ (*udrv.get()).id_table = T::ID_TABLE.as_raw_id_table();
}
// SAFETY: `udrv` is guaranteed to be a valid `DriverType`.
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260702-idtable-rename-asptr-3ea4d9e38287
Best regards,
--
Alice Ryhl <aliceryhl(a)google.com>
In a recent discussion with Philip and Danilo the question came up what
was already tried and never finished to cleanup the dma_fence framework.
So here are the different ideas I came with but never fully finished,
with the patches itself modernized and rebased on top of drm-misc-next.
The main goal of those changes is to make it easier to implement dma_fence
backends and don't enforce unnecessary constrains on implementations.
As first step the locking around the dma_fence_ops.signaled callback is
made consistent by removing the dma_fence_is_signaled_locked() function.
This was mostly used by backends itself, but if polling the HW is desired
the backends can call their own functions for this directly without going
through the dma-fence layer.
XE actually seems to be the only driver which make use of that for a bit
more handling. For all other cases testing the signaled flag should be enough.
Then forcefully calling dma_fence_signaled() is removed from the dma-fence
layer and moved into the backend implementations.
This allows the backend implementations to cleanup after they have
signaled the fence. Such cleanup can include removing now signaled fences
from lists, dropping references, starting work etc....
Especially nouveau seems to have some really messy workaround because of
that involving the DMA_FENCE_FLAG_USER_BITS and installing callbacks
because the reference to the context couldn't be dropped directly after
signaling. This can now be cleaned up as far as I can see.
In the long term this should also allow reworking the error handling, e.g.
removing dma_fence_set_error() and instead giving the error as mandatory
parameter to dma_fence_signal().
Then the last piece is dropping calling enable_signaling callback with the
dma_fence lock held. This makes it possible for backends to acquire locks
which are semantically ordered outside of the dma_fence lock.
This is necessary to allows using the dma_fence inline lock in more cases,
previously backends used some common external lock for their dma_fences to
for example make it possible remove fences from linked lists.
Please comment and review,
Christian.