On Tue, 8 Feb 2022 at 13:49, Arnd Bergmann <arnd@xxxxxxxxxx> wrote: > > On Thu, Feb 3, 2022 at 9:47 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > On Thu, 3 Feb 2022 at 09:38, Arnd Bergmann <arnd@xxxxxxxxxx> wrote: > > > > > There are two main downsides: > > > > > > - rather than using a dynamically sized pool, this buffer needs > > > to be allocated at probe time using a fixed size. Without > > > having any idea of what it should be, I picked a size of > > > 64KB, which is between what the other two OHCI front-ends use > > > in their SRAM. If anyone has a better idea what that size > > > is reasonable, this can be trivially changed. > > > > > > > I suppose this is a problem if the driver falls back to ordinary DRAM > > once the allocation runs out? > > From what I can tell, there is no such fallback. If the localmem_pool > runs out, the allocation fails, which may cause other problems, but > it never falls back to the wrong DMA address. > OK that is the least bad outcome I suppose. > > > - Previously, only USB transfers to unaddressable memory needed > > > to go through the bounce buffer, now all of them do, which may > > > impact runtime performance for USB endpoints that do a lot of > > > transfers. > > > > > > On the upside, the local_mem support uses write-combining buffers, > > > which should be a bit faster for transfers to the device compared to > > > normal uncached coherent memory as used in dmabounce. > > > > > > > Talking from past experience using this trick on a NXP ARM9 SoC ~10 > > years ago, using on-chip SRAM for USB DMA likely results in a > > significant performance boost, even without write combining, although > > the exact scenario obviously matters. > > Right, that makes sense, but it won't help here because there is > no SRAM. One detail I noticed is that the localmem pool normally > gets mapped as WC, which is what I did in the new code as well, but > dma_alloc_flags(..., DMA_ATTR_WRITE_COMBINE) does not always > honor this flag. I think it will do it here because a GFP_KERNEL > allocation should be served by the remap_allocator, while > GFP_ATOMIC allocations would be served by pool_allocator_alloc(), > which ignores the flag. > Ah yes, ignore me. For some reason, I thought this was about on-chip SRAM.
