Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <490c77e9-e3d4-4499-8471-128804fb2e7a@suse.com>
Date:   Tue, 5 Dec 2023 17:16:13 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] tracing: Simplify and fix "buffered event"
 synchronization
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
        zhengyejian1@huawei.com, linux-trace-kernel@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20231127151248.7232-1-petr.pavlu@suse.com>
 <20231127151248.7232-2-petr.pavlu@suse.com>
 <20231127124130.1041ffd4@gandalf.local.home>
 <77037ca1-8116-4bc6-b286-67059db0848e@suse.com>
 <20231128102748.23328618@gandalf.local.home>
 <bb6c9771-1a7c-4367-bb14-63524d9c560a@suse.com>
 <20231129095826.1aec6381@gandalf.local.home>
 <d404113e-4ffe-4e9c-ab45-1b076c1f498c@suse.com>
 <20231201094639.03a1913c@gandalf.local.home>
Content-Language: en-US
From:   Petr Pavlu <petr.pavlu@suse.com>
In-Reply-To: <20231201094639.03a1913c@gandalf.local.home>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/1/23 15:46, Steven Rostedt wrote:
> On Fri, 1 Dec 2023 15:17:35 +0100
> Petr Pavlu <petr.pavlu@suse.com> wrote:
> 
>> Ok, keeping the current approach, my plan for v2 is to prepare the
>> following patches:
>>
>> [...]
>> * Fix the potential race between trace_buffered_event_enable() and
>>   trace_event_buffer_lock_reserve() where the latter might already see
>>   a valid trace_buffered_event pointer but not all initialization yet.
>>
>>   I think this might be actually best to address by using the same
>>   maintenance exclusion as is implemented in
>>   trace_buffered_event_disable(). It would make both maintenance
>>   operations consistent but for the cost of making the enable operation
>>   somewhat slower.
> 
> I wouldn't do them the same just to make them consistent. I think the
> smp_wmb() is sufficient. Don't you think?

Looking at this again, I think it is actually a non-issue. Function
trace_buffered_event_enable() only writes the header part of
ring_buffer_event but that is never written nor read by the actual users
which obtain the buffer from trace_event_buffer_lock_reserve().

No change is then needed, it is left out in v2 of the series.

-- Petr