Hi Antti, >we should be aligning the data[0] pointer inside the struct instead. I believe you are correct. Now, I checked to see at runtime if temp->data was aligned and it was. I cannot tell you why :) That code is copy-paste from the tegra-ehci driver. >with the alignment hacks altogether and hit the issue with a heavier I feel bad about the alignment hacks as well, and would like the original allocation from the URB thing to be aligned... no additional kmalloc, no memcpy. Is there a reason why we shouldn't try to fix that? >pointer are separate and no corruptions should occur. The corruptions themselves are bad, and should be cured. Thanks, Boris. Antti Seppälä wrote: >Hi Guenter, > >On Wed, 19 Feb 2020 at 23:11, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: >> >> Yes, those patches didn't address the core problem. Can you test with the >> attached two patches ? >> >> Thanks, >> Guenter > >I took a look at your patch (usb: dwc2: Simplify DMA alignment code) >and I don't believe it is correct. > >The patch re-introduces the dma_aligned_buffer struct and takes some >care to align the beginning of the struct to dma cache lines. However >we should be aligning the data[0] pointer inside the struct instead. >With the code in the patch data[0] gets pushed to be at an offset from >the alignment by kmalloc_ptr and old_xfer_buffer pointers. In other >words data[0] is now not aligned to dma cache boundaries. > >Reviewing the code got me thinking that what if we stopped playing >with the alignment hacks altogether and hit the issue with a heavier >hammer instead? Attached you can find a new patch that introduces a >list to keep track of the allocations. The code then looks up the >entry from the list when it is time to restore the original pointer. >This way the allocations for the aligned dma area and the original >pointer are separate and no corruptions should occur. > >Thoughts, comments? I should note that the patch has received only >light testing and not very thorough thinking. I can prepare a proper >patch to be sent inline if the idea seems worth exploring further. > >-- >Antti
