On 10/9/26 14:42, Jason Gunthorpe wrote:
> On Mon, Oct 05, 2026 at 08:41:32AM +0200, Karl Mehltretter wrote:
>> static inline struct page *sg_page(struct scatterlist *sg)
>> {
>> #ifdef CONFIG_DEBUG_SG
>> BUG_ON(sg_is_chain(sg));
>> #endif
>> + sg_dmabuf_cpu_access_check(sg);
>> return (struct page *)((sg)->page_link & ~SG_PAGE_LINK_MASK);
>> }
>
> I'm not sure I understand the overall intention here, I get what this
> patch does, but no distro could turn this on by default when it
> touches *everyone* using scatterlist in a performance sensitive spot,
> and anyone doing testing can use the existing option - so what is the
> point?
Yeah, I was just about to write something similar.
Patch #1 in the series is correct, but I would add a warning if SWIOTLB is used
since that usually doesn't work either.
> I certainly don't like this patch, and I don't like the word "DMABUF"
> in the scatterlist at all. If we want to add something it should be a
> general mechanism under DEBUG_SG that allows anyone to 'hide' the CPU
> list from any future access.
As Christoph also noted the fundamental problem with sg_tables is that we
stuffed the input and output parameters to the DMA mapping function into one
data structure.
So by using a sg_table DMA-buf accidentally gave importers access to the
physical address as well with all the undesired consequences.
I think when anybody wants to address this mess the first thing which needs to
be done is to have a container for DMA addresses which is independent of
sg_tables.
Regards,
Christian.
>
> Jason