Re: Memory corruption with CONFIG_SWIOTLB_DYNAMIC=y

[Petr Tesarik]

On 11/8/2023 11:52 AM, Halil Pasic wrote:
> On Fri, 3 Nov 2023 19:59:49 +0100
> Petr Tesařík <petr@xxxxxxxxxxx> wrote:
> 
>>> Not sure how to properly fix this as the different alignment
>>> requirements get pretty complex quickly. So would appreciate your
>>> input.  
>>
>> I don't think it's possible to improve the allocation logic without
>> modifying the page allocator and/or the DMA atomic pool allocator to
>> take additional constraints into account.
> 
> I don't understand. What speaks against calculating the amount of space
> needed, so that with the waste we can still fit the bounce-buffer in the
> pool?
> 
> I believe alloc_size + combined_mask is a trivial upper bound, but we can
> do slightly better since we know that we allocate pages.
> 
> For the sake of simplicity let us assume we only have the min_align_mask
> requirement. Then I believe the worst case is that we need 
> (orig_addr & min_align_mask & PAGE_MASK)  + (min_align_mask & ~PAGE_MASK)
> extra space to fit.
> 
> Depending on how the semantics pan out one may be able to replace
> min_align_mask with combined_mask.
> 
> Is your point that for large combined_mask values 
> _get_free_pages(GFP_NOWAIT | __GFP_NOWARN, required_order) is not
> likely to complete successfully?

Yes, that's the reason. OTOH it's probably worth a try. The point is
that mapping a DMA buffer is allowed to fail, so callers should be
prepared anyway.

And for the case you reported initially, I don't think there is any need
to preserve bit 11 (0x800) from the original buffer's physical address,
which is enough to fix it. See also my other email earlier today.

Petr T