Am Dienstag, den 04.09.2018, 02:36 +0000 schrieb Robin Gong: > > -----Original Message----- > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > Sent: 2018年9月3日 21:12 > > To: Robin Gong <yibin.gong@xxxxxxx>; Vinod Koul <vkoul@xxxxxxxxxx> > > Cc: dmaengine@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; > > kernel@xxxxxxxxxxxxxx; patchwork-lst@xxxxxxxxxxxxxx > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel > > termination via worker > > > > Am Montag, den 03.09.2018, 08:59 +0000 schrieb Robin Gong: > > > > -----Original Message----- > > > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > > > Sent: 2018年9月3日 16:41 > > > > To: Robin Gong <yibin.gong@xxxxxxx>; Vinod Koul <vkoul@kernel.o > > > > rg> > > > > Cc: dmaengine@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > > > > ; > > > > kernel@xxxxxxxxxxxxxx; patchwork-lst@xxxxxxxxxxxxxx > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel > > > > termination via worker > > > > > > > > Hi Robin, > > > > > > > > Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong: > > > > > Hi Lucas, > > > > > Seems I miss your previous mail. Thanks for your patch, > > > > > but if > > > > > move most jobs of sdma_disable_channel_with_delay() into > > > > > worker, > > > > > that will bring another race condition that upper driver such > > > > > as > > > > > Audio terminate channel and free resource of dma channel > > > > > without > > > > > really channel stop, if dma transfer done interrupt come > > > > > after > > > > > that, oops or kernel cash may be caught. Leave 'sdmac->desc = > > > > > NULL' in the > > > > > > > > sdma_disable_channel_with_delay() may fix such potential issue. > > > > > > > > No, there is no such issue. The audio channel terminate will > > > > call > > > > dmaengine_terminate_sync(), which internally calls > > > > dmaengine_terminate_async() and then does a > > > > dmaengine_synchronize(). > > > > As this patchset implements the device_synchronize function in > > > > the > > > > sdma driver, this will wait for the worker to finish its > > > > execution, > > > > so there is no race condition to worry about here. > > > > > > > > Regards, > > > > Lucas > > > > > > Yes, but how about other drivers which not call > > > dmaengine_terminate_sync()? > > > > Please read the dmaengine documentation. device_terminate_all has > > no > > requirement that the transfer is actually canceled when the call > > returns. If the > > caller needs a guarantee that the channel is stopped it _must_ call > > device_synchronize. > > I know that, but the fact is some driver still use > dmaengine_terminate_all() such as > Spi/uart driver. My concern is how to avoid to break their > function. They should simply be fixed to not use a deprecated function. Both of those are only using device_terminate_all in error or shutdown paths, so the risk of races is pretty minimal even with the current code. And I think the SPI driver is trivial to fix, as we can just use the terminate_sync variant there. The UART driver is a bit more tricky. Regards, Lucas
