| Message ID | 20210711104105.505728-4-leo.yan@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf: Refine barriers for AUX ring buffer | expand |
On Sun, Jul 11, 2021 at 06:40:57PM +0800, Leo Yan wrote: > AUX ring buffer is required to separate the data store and aux_head > store, since the function CS_LOCK() has contained memory barrier mb(), > mb() is a more conservative barrier than smp_wmb() on Arm32/Arm64, thus > it's needless to add any explicit barrier anymore. > > Add comment to make clear for the barrier usage for ETF. > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > drivers/hwtracing/coresight/coresight-tmc-etf.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c > index 45b85edfc690..9a42ee689921 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c > @@ -553,6 +553,12 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, > if (buf->snapshot) > handle->head += to_read; > > + /* > + * AUX ring buffer requires to use memory barrier to separate the trace > + * data store and aux_head store, because CS_LOCK() contains mb() which > + * gives more heavy barrier than smp_wmb(), it's not necessary to > + * explicitly invoke any barrier. > + */ > CS_LOCK(drvdata->base); 'more heavy' is not a correctness argument :-) The argument to make here is that CS_LOCK() ensures completion / visibility of the hardware buffer.
On Tue, Jul 13, 2021 at 02:56:06PM +0200, Peter Zijlstra wrote: > On Sun, Jul 11, 2021 at 06:40:57PM +0800, Leo Yan wrote: > > AUX ring buffer is required to separate the data store and aux_head > > store, since the function CS_LOCK() has contained memory barrier mb(), > > mb() is a more conservative barrier than smp_wmb() on Arm32/Arm64, thus > > it's needless to add any explicit barrier anymore. > > > > Add comment to make clear for the barrier usage for ETF. > > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > --- > > drivers/hwtracing/coresight/coresight-tmc-etf.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c > > index 45b85edfc690..9a42ee689921 100644 > > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c > > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c > > @@ -553,6 +553,12 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, > > if (buf->snapshot) > > handle->head += to_read; > > > > + /* > > + * AUX ring buffer requires to use memory barrier to separate the trace > > + * data store and aux_head store, because CS_LOCK() contains mb() which > > + * gives more heavy barrier than smp_wmb(), it's not necessary to > > + * explicitly invoke any barrier. > > + */ > > CS_LOCK(drvdata->base); > > 'more heavy' is not a correctness argument :-) > > The argument to make here is that CS_LOCK() ensures completion / > visibility of the hardware buffer. Will correct for this, thanks for reminding!
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 45b85edfc690..9a42ee689921 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -553,6 +553,12 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, if (buf->snapshot) handle->head += to_read; + /* + * AUX ring buffer requires to use memory barrier to separate the trace + * data store and aux_head store, because CS_LOCK() contains mb() which + * gives more heavy barrier than smp_wmb(), it's not necessary to + * explicitly invoke any barrier. + */ CS_LOCK(drvdata->base); out: spin_unlock_irqrestore(&drvdata->spinlock, flags);
AUX ring buffer is required to separate the data store and aux_head store, since the function CS_LOCK() has contained memory barrier mb(), mb() is a more conservative barrier than smp_wmb() on Arm32/Arm64, thus it's needless to add any explicit barrier anymore. Add comment to make clear for the barrier usage for ETF. Signed-off-by: Leo Yan <leo.yan@linaro.org> --- drivers/hwtracing/coresight/coresight-tmc-etf.c | 6 ++++++ 1 file changed, 6 insertions(+)