Message ID | 20231210225447.48476a6a@rorschach.local.home |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp6818489vqy; Sun, 10 Dec 2023 19:54:59 -0800 (PST) X-Google-Smtp-Source: AGHT+IHr6e9JpjgLNcCVpXfBrWIbIBsqN0Dik7fmu+e0pSyzJGDT6zXblDbtAilnJ5xk/MPfsGs8 X-Received: by 2002:a05:620a:1915:b0:77e:fcc9:d2a4 with SMTP id bj21-20020a05620a191500b0077efcc9d2a4mr5464774qkb.45.1702266898805; Sun, 10 Dec 2023 19:54:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702266898; cv=none; d=google.com; s=arc-20160816; b=EVFyPa/JNEltxbEKPBMPxlkN5SlWKTLN2UzshHy89Fg1b0/CyCJ4b5dV7bjX2vH2Md 7T6oyP1hMDLXvb8yDxUQgJmjxx1kgSUzD3hhLOfLhaxmboJqy7n+EaefQKaebn5Od/2G 8wmpjVI9m8ML/I6oQbEqx1i3x2ESomNyRWUmGIXvvtdZhZbp+0QNHNGLmyynHFRK3gRf MjPZPWbZ55oLAO2OawaWXeNWKBxHn9zw4BarCScgpW4haZ1WT80KnVKqHlpqoqlq4WcZ LbCfoDAynSFwwVCIdQuUDI9Y9OYfmGorpLa4ArCdZlqe/+mcdyhD/oqzotFcIkazmALq KSEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:subject:cc:to:from:date; bh=myGmaQjjVbrqgGFDO5nqhR1lyH1KZNBGydOzUZYdPIQ=; fh=10neyeg9SpQc0WdV4aMamIFr7tikcy7JJ0bptR+ICfw=; b=vnrIwGOg/JiAXXA6UYgIvR/9hOK8zxp5eiSwlGdCC/GbI0T/pXhyrxGI4Q1i8rNHZh 8zoAD/9FNdi8UojPzY7x75iW+TMOWHdadN1FsA52Y3y2fu1LJNXWT2Zo7f2UYCE4Gusg sVFjYkf2PaQN64aEk/P86oHXmIBj9LykOiVLAtDySlxMgoVaIJLHqTyJZcWUgZelYGZ7 LXzXwKeJBEBNR9b+LlABXaE0DzWvpkRpvKQTjtX2boIooYfHyJL+EdML84zpmwa4G86B tvgm+vYuY79wLx85m6ZZVm9LwdkP7KZu35/zO6D6RV17NZ+52DxtAPEmhNPzIEsomyL9 rpZg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id p19-20020a17090adf9300b00286b88b108bsi5377977pjv.174.2023.12.10.19.54.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Dec 2023 19:54:58 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id C6C0B8092DAC; Sun, 10 Dec 2023 19:54:57 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230286AbjLKDyp (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Sun, 10 Dec 2023 22:54:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59216 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229448AbjLKDyo (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 10 Dec 2023 22:54:44 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C718BED for <linux-kernel@vger.kernel.org>; Sun, 10 Dec 2023 19:54:50 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AF60DC433C7; Mon, 11 Dec 2023 03:54:49 +0000 (UTC) Date: Sun, 10 Dec 2023 22:54:47 -0500 From: Steven Rostedt <rostedt@goodmis.org> To: LKML <linux-kernel@vger.kernel.org>, Linux trace kernel <linux-trace-kernel@vger.kernel.org> Cc: Masami Hiramatsu <mhiramat@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Subject: [PATCH] tracing: Update snapshot buffer on resize if it is allocated Message-ID: <20231210225447.48476a6a@rorschach.local.home> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Sun, 10 Dec 2023 19:54:57 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784956215762208149 X-GMAIL-MSGID: 1784956215762208149 |
Series |
tracing: Update snapshot buffer on resize if it is allocated
|
|
Commit Message
Steven Rostedt
Dec. 11, 2023, 3:54 a.m. UTC
From: "Steven Rostedt (Google)" <rostedt@goodmis.org> The snapshot buffer is to mimic the main buffer so that when a snapshot is needed, the snapshot and main buffer are swapped. When the snapshot buffer is allocated, it is set to the minimal size that the ring buffer may be at and still functional. When it is allocated it becomes the same size as the main ring buffer, and when the main ring buffer changes in size, it should do. Currently, the resize only updates the snapshot buffer if it's used by the current tracer (ie. the preemptirqsoff tracer). But it needs to be updated anytime it is allocated. When changing the size of the main buffer, instead of looking to see if the current tracer is utilizing the snapshot buffer, just check if it is allocated to know if it should be updated or not. Also fix typo in comment just above the code change. Cc: stable@vger.kernel.org Fixes: ad909e21bbe69 ("tracing: Add internal tracing_snapshot() functions") Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> --- kernel/trace/trace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On Sun, 10 Dec 2023 22:54:47 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > The snapshot buffer is to mimic the main buffer so that when a snapshot is > needed, the snapshot and main buffer are swapped. When the snapshot buffer > is allocated, it is set to the minimal size that the ring buffer may be at > and still functional. When it is allocated it becomes the same size as the > main ring buffer, and when the main ring buffer changes in size, it should > do. nit: There seems two "when the snapshot buffer is allocated" case, maybe latter "it" means main buffer? > > Currently, the resize only updates the snapshot buffer if it's used by the > current tracer (ie. the preemptirqsoff tracer). But it needs to be updated > anytime it is allocated. > > When changing the size of the main buffer, instead of looking to see if > the current tracer is utilizing the snapshot buffer, just check if it is > allocated to know if it should be updated or not. > > Also fix typo in comment just above the code change. > Looks good to me. Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> BTW, the historical naming leads this kind of issues. Maybe we'd better to rename 'max_buffer' to 'snapshot_buffer'? Thanks, > Cc: stable@vger.kernel.org > Fixes: ad909e21bbe69 ("tracing: Add internal tracing_snapshot() functions") > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > kernel/trace/trace.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index aa8f99f3e5de..6c79548f9574 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -6348,7 +6348,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr, > if (!tr->array_buffer.buffer) > return 0; > > - /* Do not allow tracing while resizng ring buffer */ > + /* Do not allow tracing while resizing ring buffer */ > tracing_stop_tr(tr); > > ret = ring_buffer_resize(tr->array_buffer.buffer, size, cpu); > @@ -6356,7 +6356,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr, > goto out_start; > > #ifdef CONFIG_TRACER_MAX_TRACE > - if (!tr->current_trace->use_max_tr) > + if (!tr->allocated_snapshot) > goto out; > > ret = ring_buffer_resize(tr->max_buffer.buffer, size, cpu); > -- > 2.42.0 >
On Mon, 11 Dec 2023 21:31:34 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > On Sun, 10 Dec 2023 22:54:47 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > > > The snapshot buffer is to mimic the main buffer so that when a snapshot is > > needed, the snapshot and main buffer are swapped. When the snapshot buffer > > is allocated, it is set to the minimal size that the ring buffer may be at > > and still functional. When it is allocated it becomes the same size as the > > main ring buffer, and when the main ring buffer changes in size, it should > > do. > > nit: There seems two "when the snapshot buffer is allocated" case, maybe latter > "it" means main buffer? I changed the paragraph to be: The snapshot buffer is to mimic the main buffer so that when a snapshot is needed, the snapshot and main buffer are swapped. When the snapshot buffer is allocated, it is set to the minimal size that the ring buffer may be at and still functional. When it is allocated it becomes the same size as the main ring buffer, and when the main ring buffer changes in size, the snapshot should also change in size if it is allocated. > > > > > Currently, the resize only updates the snapshot buffer if it's used by the > > current tracer (ie. the preemptirqsoff tracer). But it needs to be updated > > anytime it is allocated. > > > > When changing the size of the main buffer, instead of looking to see if > > the current tracer is utilizing the snapshot buffer, just check if it is > > allocated to know if it should be updated or not. > > > > Also fix typo in comment just above the code change. > > > > Looks good to me. > > Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thanks! > > BTW, the historical naming leads this kind of issues. > Maybe we'd better to rename 'max_buffer' to 'snapshot_buffer'? Agreed. But that's a cleanup for another day. Hmm, maybe that too should be marked as "KTODO"? https://lore.kernel.org/all/369bc919-1a1d-4f37-9cc9-742a86a41282@kadam.mountain/ There's a lot of things that we have been discussing on these ring-buffer patches that could be KTODO items. -- Steve
On Mon, 11 Dec 2023 12:51:52 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 11 Dec 2023 21:31:34 +0900 > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > > On Sun, 10 Dec 2023 22:54:47 -0500 > > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > > > > > The snapshot buffer is to mimic the main buffer so that when a snapshot is > > > needed, the snapshot and main buffer are swapped. When the snapshot buffer > > > is allocated, it is set to the minimal size that the ring buffer may be at > > > and still functional. When it is allocated it becomes the same size as the > > > main ring buffer, and when the main ring buffer changes in size, it should > > > do. > > > > nit: There seems two "when the snapshot buffer is allocated" case, maybe latter > > "it" means main buffer? > > I changed the paragraph to be: > > The snapshot buffer is to mimic the main buffer so that when a snapshot is > needed, the snapshot and main buffer are swapped. When the snapshot buffer > is allocated, it is set to the minimal size that the ring buffer may be at > and still functional. When it is allocated it becomes the same size as the > main ring buffer, and when the main ring buffer changes in size, the > snapshot should also change in size if it is allocated. Yeah, this makes clearer. > > > > > > > > > Currently, the resize only updates the snapshot buffer if it's used by the > > > current tracer (ie. the preemptirqsoff tracer). But it needs to be updated > > > anytime it is allocated. > > > > > > When changing the size of the main buffer, instead of looking to see if > > > the current tracer is utilizing the snapshot buffer, just check if it is > > > allocated to know if it should be updated or not. > > > > > > Also fix typo in comment just above the code change. > > > > > > > Looks good to me. > > > > Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Thanks! > > > > > BTW, the historical naming leads this kind of issues. > > Maybe we'd better to rename 'max_buffer' to 'snapshot_buffer'? > > Agreed. But that's a cleanup for another day. Hmm, maybe that too should be > marked as "KTODO"? Yes! > > https://lore.kernel.org/all/369bc919-1a1d-4f37-9cc9-742a86a41282@kadam.mountain/ > Ah, this is a good idea. > > There's a lot of things that we have been discussing on these ring-buffer > patches that could be KTODO items. Thanks, > > -- Steve
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index aa8f99f3e5de..6c79548f9574 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -6348,7 +6348,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr, if (!tr->array_buffer.buffer) return 0; - /* Do not allow tracing while resizng ring buffer */ + /* Do not allow tracing while resizing ring buffer */ tracing_stop_tr(tr); ret = ring_buffer_resize(tr->array_buffer.buffer, size, cpu); @@ -6356,7 +6356,7 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr, goto out_start; #ifdef CONFIG_TRACER_MAX_TRACE - if (!tr->current_trace->use_max_tr) + if (!tr->allocated_snapshot) goto out; ret = ring_buffer_resize(tr->max_buffer.buffer, size, cpu);