Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 15 Sep 2020 09:23:53 -0400
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
Message-ID: <20200915092353.5b805468@gandalf.local.home>
In-Reply-To: <2fe2a843-e2b5-acf8-22e4-7231d24a9382@codeaurora.org>
References: <1599199797-25978-1-git-send-email-gkohli@codeaurora.org>
        <d4691a90-9a47-b946-f2cd-bb1fce3981b0@codeaurora.org>
        <2fe2a843-e2b5-acf8-22e4-7231d24a9382@codeaurora.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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()?

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