On 11/30/2016 09:30 AM, Lars-Peter Clausen wrote:
On 11/30/2016 09:22 AM, Jiada Wang wrote:
From: Andreas Pape apape@de.adit-jv.com
Setting pointer and afterwards check for wrap around leads to the possibility of returning the inconsistent pointer position. This patch increments buffer pointer atomically to avoid this issue.
Makes sense.
Signed-off-by: Andreas Pape apape@de.adit-jv.com Signed-off-by: Jiada Wang jiada_wang@mentor.com
sound/core/pcm_dmaengine.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c index 8eb58c7..6f6da11 100644 --- a/sound/core/pcm_dmaengine.c +++ b/sound/core/pcm_dmaengine.c @@ -139,12 +139,14 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_set_config_from_dai_data);
static void dmaengine_pcm_dma_complete(void *arg) {
- unsigned int new_pos; struct snd_pcm_substream *substream = arg; struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
- prtd->pos += snd_pcm_lib_period_bytes(substream);
- if (prtd->pos >= snd_pcm_lib_buffer_bytes(substream))
prtd->pos = 0;
- new_pos = prtd->pos + snd_pcm_lib_period_bytes(substream);
- if (new_pos >= snd_pcm_lib_buffer_bytes(substream))
new_pos = 0;- prtd->pos = new_pos;
But to really make it atomic I think this needs READ_ONCE/WRITE_ONCE.
And the access to prtd->pos in snd_dmaengine_pcm_pointer_no_residue() should also use READ_ONCE(). It is very unlikely that the code gets mis-compiled to generate more than one access, but having READ_ONCE() acts as a annotation that makes it explicit that this is data that can be updated concurrently without further synchronization.