On 06-03-20, 13:57, Radhey Shyam Pandey wrote: > > -----Original Message----- > > From: Vinod Koul <vkoul@xxxxxxxxxx> > > Sent: Friday, March 6, 2020 7:04 PM > > To: Sebastian von Ohr <vonohr@xxxxxxxxxxx>; Appana Durga Kedareswara > > Rao <appanad@xxxxxxxxxx>; Radhey Shyam Pandey <radheys@xxxxxxxxxx>; > > Michal Simek <michals@xxxxxxxxxx> > > Cc: dmaengine@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH] dmaengine: xilinx_dma: Add missing check for empty list > > Minor nit - Better to also add <...> "in device_tx_status callback " > > > > On 03-03-20, 14:05, Sebastian von Ohr wrote: > > > The DMA transfer might finish just after checking the state with > > > dma_cookie_status, but before the lock is acquired. Not checking for > > > an empty list in xilinx_dma_tx_status may result in reading random > > > data or data corruption when desc is written to. This can be reliably > > > triggered by using dma_sync_wait to wait for DMA completion. > > > > Appana, Radhey can you please test this..? > > Sure, we will test it. Changes look fine. Though had a question in mind, > for a generic fix to this problem, should we make locking mandatory for > all cookie helper functions? Or is there any limitation? > > The framework say for dma_cookie_status says locking is not required. This > scenario is a race condition when the driver calls dma_cookie_status and > it sees it's not completed, but then since there is no locking and dma > completion comes and it changes cookie state and removes the element > from active list to done list. When driver access it in tx_status it results > in data corruption/crash. The expectation is that you would lock while looking at list and then return.. So you should not have issues.. -- ~Vinod
