From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80711C4167B for ; Sun, 10 Dec 2023 16:37:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231854AbjLJQht (ORCPT ); Sun, 10 Dec 2023 11:37:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231755AbjLJQhr (ORCPT ); Sun, 10 Dec 2023 11:37:47 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CF3B8F2 for ; Sun, 10 Dec 2023 08:37:53 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B9CE1C433C7; Sun, 10 Dec 2023 16:37:52 +0000 (UTC) Date: Sun, 10 Dec 2023 11:38:29 -0500 From: Steven Rostedt To: Mathieu Desnoyers Cc: LKML , Linux Trace Kernel , Masami Hiramatsu , Mark Rutland Subject: Re: [PATCH] tracing: Allow for max buffer data size trace_marker writes Message-ID: <20231210113829.780c7097@gandalf.local.home> In-Reply-To: References: <20231209175003.63db40ab@gandalf.local.home> <2683467e-cadb-4bb8-8c50-87ef052edacb@efficios.com> <20231210103009.29010d00@gandalf.local.home> X-Mailer: Claws Mail 3.19.1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 10 Dec 2023 11:07:22 -0500 Mathieu Desnoyers wrote: > > It just allows more to be written in one go. > > > > I don't see why the tests need to cover this or detect this change. > > If the purpose of this change is to ensure that the entire > trace marker payload is shown within a single event, then > there should be a test which exercises this, and which > validates that the end result is that the entire payload > is indeed shown within a single event record. No, the purpose of the change is not to do that, because there can always be a bigger trace marker write than a single event can hold. This is the way it has always worked. This is an optimization or "enhancement". The 1KB restriction was actually because of a previous implementation years ago (before selftests even existed) that wrote into a temp buffer before copying into the ring buffer. But since we now can copy directly into the ring buffer, there's no reason not to use the maximum that the ring buffer can accept. > > Otherwise there is no permanent validation that this change > indeed does what it is intended to do, so it can regress > at any time without any test noticing it. What regress? The amount of a trace_marker write that can make it into a the buffer in one go? Now, I agree that we should have a test to make sure that all of the trace marker write gets into the buffer. But it's always been allowed to break up that write however it wanted to. Note, because different architectures have different page sizes, how much that can make it in one go is architecture dependent. So you can have a "regression" by simply running your application on a different architecture. Again, it's not a requirement, it's just an enhancement. -- Steve