> -----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@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 > > > > > > 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. > > For your convenience I'm copying the relevant part of the docs below (from > dmaengine_terminate_async(), which is what calls > device_terminate_all(): > > "Calling this function will terminate all active and pending descriptors that have > previously been submitted to the channel. It is not guaranteed though that the > transfer for the active descriptor has stopped when the function returns. > Furthermore it is possible the complete callback of a submitted transfer is still > running when this function returns. > > dmaengine_synchronize() needs to be called before it is safe to free any > memory that is accessed by previously submitted descriptors or before freeing > any resources accessed from within the completion callback of any previously > submitted descriptors." > > Regards, > Lucas
