Page MenuHomeFreeBSD

busdma/bounce: force bouncing of full buffer
AbandonedPublic

Authored by royger on Apr 11 2016, 5:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 24, 8:44 PM
Unknown Object (File)
Feb 9 2024, 10:40 PM
Unknown Object (File)
Dec 20 2023, 3:41 AM
Unknown Object (File)
Dec 19 2023, 6:35 AM
Unknown Object (File)
Aug 25 2023, 9:14 AM
Unknown Object (File)
Jul 4 2023, 6:28 PM
Unknown Object (File)
May 19 2023, 3:05 PM
Unknown Object (File)
Dec 18 2022, 11:51 PM
Subscribers

Details

Summary

Due to r292255 when doing partial bouncing of a buffer (ie: only bouncing
certain segments) the offsets might not be contiguous anymore, breaking
certain devices that expect them to be.

In order to make sure the segments have contiguous offsets always fully
bounce the whole buffer.

PR: 208365
Sponsored by: Citrix Systems R&D

Test Plan

TBH, I'm not sure whether to simply revert r292255 or to apply this fix. The
following needs to be taken into account:

  • r292255 does compaction, so that we will always use the minimum amount of bounce pages.
  • Since r292255 the bounce code now tries to always fill the bounce pages, the offset of the segments is changed and we need to bounce the whole buffer.
  • Is it worth bouncing the full buffer (so more memcpy) in exchange for maybe using less bounce pages and having the bounce code slightly simplified?

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3220
Build 3253: arc lint + arc unit

Event Timeline

royger retitled this revision from to busdma/bounce: force bouncing of full buffer.
royger updated this object.
royger edited the test plan for this revision. (Show Details)
royger added reviewers: kib, jah, hselasky.

If the length is <= PAGE_SIZE, compaction is safe. Else don't compact. The benifit might be less optimal when length > PAGE_SIZE? Will waste a page anyway.

If the length is <= PAGE_SIZE, compaction is safe. Else don't compact. The benifit might be less optimal when length > PAGE_SIZE? Will waste a page anyway.

Not really, take the following example; a buffer of size 8192 (2*PAGE_SIZE) that starts at offset 2048. With the previous code this would require 3 bounce pages, with the current code it only requires two bounce pages, because we change the offset of the first segment to be 0 and then it just fits into 2 pages.

Sure, but if you look at how much memory you waste it is less than or equal to 50%:

Memory waste is 1.0 - (8192 / (3*4096)) = 33% ???

which I think is acceptable.

The biggest gain will be for loading less than PAGE_SIZE bytes, which cross a PAGE boundary ??

Sure, but if you look at how much memory you waste it is less than or equal to 50%:

Memory waste is 1.0 - (8192 / (3*4096)) = 33% ???

which I think is acceptable.

The biggest gain will be for loading less than PAGE_SIZE bytes, which cross a PAGE boundary ??

Yes, if we have a buffer that has a size of PAGE_SIZE (or less) and it crosses a page boundary we can save 50%. But having different behaviors depending on the size/alignment of the buffer seems like opening a can of worms, TBH, I don't think we want to make the code any more complex that what it already is.

About complexity - this fix makes busdma more complicated. Should we then revert the PAGE squashing feature. We should make a decision before next week, because 11.0 is going to be branched soon!

I think it's better to revert r292255 than to add this. Like Hans mentioned it adds complexity, and the fact that it forces entire buffers to be bounced seems wrong to me. The added memcpy probably isn't that big of a deal, since performance goes out the window if you're bouncing anyway. But the memory those bounce pages come from is likely to be somewhat precious. This could make large transfers use a lot more of that, which makes me nervous.

If you want to save on bounce buffer usage for smaller segments, it's probably better to implement bounce buffer coalescing (similar to the segment coalescing we already do), and then probably only for !BUS_DMA_KEEP_PG_OFFSET. I think ian has also been working on some changes to make arm/mips use sub-page allocations for small bounce segments, so that might be useful here too.

In D5917#126795, @jah wrote:

I think it's better to revert r292255 than to add this. Like Hans mentioned it adds complexity, and the fact that it forces entire buffers to be bounced seems wrong to me. The added memcpy probably isn't that big of a deal, since performance goes out the window if you're bouncing anyway. But the memory those bounce pages come from is likely to be somewhat precious. This could make large transfers use a lot more of that, which makes me nervous.

If you want to save on bounce buffer usage for smaller segments, it's probably better to implement bounce buffer coalescing (similar to the segment coalescing we already do), and then probably only for !BUS_DMA_KEEP_PG_OFFSET. I think ian has also been working on some changes to make arm/mips use sub-page allocations for small bounce segments, so that might be useful here too.

I agree, and I've already reverted r292255. Thanks for the input.

Roger.