lkml.org 
[lkml]   [2020]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] trace: Fix race in trace_open and buffer resize call
From
Date
Sorry for spam, saw some failure so sending mail again.

On 9/15/2020 6:53 PM, Steven Rostedt wrote:
> On Tue, 15 Sep 2020 10:38:03 +0530
> Gaurav Kohli <gkohli@codeaurora.org> wrote:
>
>>
>> >>> +void ring_buffer_mutex_release(struct trace_buffer *buffer)
>> >>> +{
>> >>> + mutex_unlock(&buffer->mutex);
>> >>> +}
>> >>> +EXPORT_SYMBOL_GPL(ring_buffer_mutex_release);
>> >
>> > I really do not like to export these.
>> >
>>
>> Actually available reader lock is not helping
>> here(&cpu_buffer->reader_lock), So i took ring buffer mutex lock to
>> resolve this(this came on 4.19/5.4), in latest tip it is trace buffer
>> lock. Due to this i have exported api.
>
> I'm saying, why don't you take the buffer->mutex in the
> ring_buffer_reset_online_cpus() function? And remove all the
protection in
> tracing_reset_online_cpus()?

Yes, got your point. then we can avoid export. Actually we are seeing
issue in older kernel like 4.19/4.14/5.4 and there below patch is not
present in stable branches:

ommit b23d7a5f4a07 ("ring-buffer: speed up buffer resets by
> avoiding synchronize_rcu for each CPU")

Actually i have also thought to take mutex lock in ring_buffer_reset_cpu
while doing individual cpu reset, but this could cause another problem:

Different cpu buffer may have different state, so i have taken lock in
tracing_reset_online_cpus.
>
> void tracing_reset_online_cpus(struct array_buffer *buf)
> {
> struct trace_buffer *buffer = buf->buffer;
>
> if (!buffer)
> return;
>
> buf->time_start = buffer_ftrace_now(buf, buf->cpu);
>
> ring_buffer_reset_online_cpus(buffer);
> }
>
> The reset_online_cpus() is already doing the synchronization, we
don't need
> to do it twice.
>
> I believe commit b23d7a5f4a07 ("ring-buffer: speed up buffer resets by
> avoiding synchronize_rcu for each CPU") made the synchronization in
> tracing_reset_online_cpus() obsolete.
>
> -- Steve
>

Yes, with above patch no need to take lock in tracing_reset_online_cpus.
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

\
 
 \ /
  Last update: 2020-09-15 20:22    [W:0.770 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site