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

Reply via email to