From eca9f7038083c16387d1c26b116bf70d1553738c Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 31 Aug 2021 13:52:46 +0800 Subject: [PATCH 1/2] dma-trace: hold the lock during switching on/off the trace Hold the dma-trace lock when performing on/off switching, to make sure the status is consistent. Signed-off-by: Keyon Jie --- src/trace/dma-trace.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/trace/dma-trace.c b/src/trace/dma-trace.c index 779480e49468..a2eb33c7375c 100644 --- a/src/trace/dma-trace.c +++ b/src/trace/dma-trace.c @@ -475,26 +475,42 @@ void dma_trace_flush(void *t) void dma_trace_on(void) { struct dma_trace_data *trace_data = dma_trace_data_get(); + uint32_t flags; - if (!trace_data || trace_data->enabled) + if (!trace_data) return; - trace_data->enabled = 1; + spin_lock_irq(&trace_data->lock, flags); + + if (trace_data->enabled) + goto finish; + schedule_task(&trace_data->dmat_work, DMA_TRACE_PERIOD, DMA_TRACE_PERIOD); + trace_data->enabled = 1; +finish: + spin_unlock_irq(&trace_data->lock, flags); } void dma_trace_off(void) { struct dma_trace_data *trace_data = dma_trace_data_get(); + uint32_t flags; - if (!trace_data || !trace_data->enabled) + if (!trace_data) return; + spin_lock_irq(&trace_data->lock, flags); + + if (!trace_data->enabled) + goto finish; + schedule_task_cancel(&trace_data->dmat_work); trace_data->enabled = 0; +finish: + spin_unlock_irq(&trace_data->lock, flags); } static int dtrace_calc_buf_overflow(struct dma_trace_buf *buffer, From d639e5ed0cb505b9cfa87706fcc9bb74dadb8fac Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 31 Aug 2021 14:19:43 +0800 Subject: [PATCH 2/2] trace: don't hold trace->lock during dma_trace_on/off As there is trace calling in the dma_trace_on/off() internal, we should not do that with trace->lock held, to avoid deadlock. Signed-off-by: Keyon Jie --- src/trace/trace.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/trace/trace.c b/src/trace/trace.c index fb21dc5ce229..626ab1fadb2e 100644 --- a/src/trace/trace.c +++ b/src/trace/trace.c @@ -486,11 +486,15 @@ void trace_on(void) struct trace *trace = trace_get(); uint32_t flags; - spin_lock_irq(&trace->lock, flags); - - trace->enable = 1; + /* + * To avoid dead lock, we should not do this with trace->lock held, + * as there is log tracing inside the dma_trace_on(), which will ask + * for holding trace->lock also. + */ dma_trace_on(); + spin_lock_irq(&trace->lock, flags); + trace->enable = 1; spin_unlock_irq(&trace->lock, flags); } @@ -499,11 +503,15 @@ void trace_off(void) struct trace *trace = trace_get(); uint32_t flags; - spin_lock_irq(&trace->lock, flags); - - trace->enable = 0; + /* + * To avoid dead lock, we should not do this with trace->lock held, + * as there is log tracing inside the dma_trace_off(), which will ask + * for holding trace->lock also. + */ dma_trace_off(); + spin_lock_irq(&trace->lock, flags); + trace->enable = 0; spin_unlock_irq(&trace->lock, flags); }