From: Steven Rostedt <rostedt@goodmis.org>
To: Gaurav Kohli <gkohli@codeaurora.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, 15 Sep 2020 14:13:04 -0400 [thread overview]
Message-ID: <20200915141304.41fa7c30@gandalf.local.home> (raw)
In-Reply-To: <08d6f338-3be3-c5a2-ba4b-0116de9672c2@codeaurora.org>
On Tue, 15 Sep 2020 22:53:32 +0530
Gaurav Kohli <gkohli@codeaurora.org> wrote:
> 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 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")
>
> 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.
>
> 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
> >
> > 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.
next prev parent reply other threads:[~2020-09-15 18:34 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 [this message]
2020-09-16 6:32 ` Gaurav Kohli
2020-09-22 7:31 ` Gaurav Kohli
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=20200915141304.41fa7c30@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=gkohli@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).