From: Gaurav Kohli <gkohli@codeaurora.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: mingo@redhat.com, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] trace: Fix race in trace_open and buffer resize call
Date: Tue, 22 Sep 2020 13:01:29 +0530 [thread overview]
Message-ID: <42818d2b-41b8-ef43-24eb-c7c75b939cf5@codeaurora.org> (raw)
In-Reply-To: <17b53f76-fa90-0086-8a9e-de166b789e60@codeaurora.org>
On 9/16/2020 12:02 PM, Gaurav Kohli wrote:
>>>
>>> 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 was not
>>> present in stable branches:
>>>
>>> ommit b23d7a5f4a07 ("ring-buffer: speed up buffer resets by
>>> > avoiding synchronize_rcu for each CPU")
>>
>> If you mark this patch for stable, you can add:
>>
>> Depends-on: b23d7a5f4a07 ("ring-buffer: speed up buffer resets by
>> avoiding synchronize_rcu for each CPU")
>>
>
> Thanks Steven, Yes this needs to be back ported. I have tried this in
> 5.4 but this need more patches like
> 13292494379f92f532de71b31a54018336adc589
> tracing: Make struct ring_buffer less ambiguous
>
> Instead of protecting all reset, can we do it individually like below:
>
>
> +++ b/kernel/trace/ring_buffer.c
> @@ -4838,7 +4838,9 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer)
> static void reset_disabled_cpu_buffer(struct ring_buffer_per_cpu
> *cpu_buffer)
> {
> unsigned long flags;
> + struct trace_buffer *buffer = cpu_buffer->buffer;
>
> + mutex_lock(&buffer->mutex);
> raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
>
> if (RB_WARN_ON(cpu_buffer, local_read(&cpu_buffer->committing)))
> @@ -4852,6 +4854,7 @@ static void reset_disabled_cpu_buffer(struct
> ring_buffer_per_cpu *cpu_buffer)
>
> out:
> raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
> + mutex_unlock(&buffer->mutex);
> }
>
Hi Steven,
Not seeing issue with above patch in 5.4, Please let me know if above
approach looks good to you, will raise patch for same.
Otherwise we will raise patch for older approach by marking depends on
of below patch:
depends-on: b23d7a5f4a07 ("ring-buffer: speed up buffer resets by
Thanks,
Gaurav
> Please let me know, if above looks good, we will do testing with this.
> And this we can directly use in older kernel as well in
> ring_buffer_reset_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:
>>
>> Hmm, I think we should also take the buffer lock in the reset_cpu() call
>> too, and modify tracing_reset_cpu() the same way.
>>
>
> if we take above patch, then this is not required.
> Please let me know for the approach.
>>>
>>> Different cpu buffer may have different state, so i have taken lock in
>>> tracing_reset_online_cpus.
>>
>> Why would different states be an issue in synchronizing?
>>
>> -- Steve
>>
>
> Yes, this should not be problem.
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
next prev parent reply other threads:[~2020-09-22 7:32 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 6:09 [PATCH] trace: Fix race in trace_open and buffer resize call Gaurav Kohli
2020-09-14 4:30 ` Gaurav Kohli
2020-09-14 16:19 ` Steven Rostedt
2020-09-15 5:08 ` Gaurav Kohli
2020-09-15 13:23 ` Steven Rostedt
2020-09-15 17:23 ` Gaurav Kohli
2020-09-15 18:13 ` Steven Rostedt
2020-09-16 6:32 ` Gaurav Kohli
2020-09-22 7:31 ` Gaurav Kohli [this message]
2020-09-22 14:00 ` Steven Rostedt
2020-09-15 18:05 ` Gaurav Kohli
2021-01-24 9:56 Gaurav Kohli
2021-01-24 10:05 ` Greg KH
2021-01-24 16:41 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=42818d2b-41b8-ef43-24eb-c7c75b939cf5@codeaurora.org \
--to=gkohli@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.