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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7188FC27C52 for ; Thu, 6 Jun 2024 21:26:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 09FEB6B00AB; Thu, 6 Jun 2024 17:26:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 028E86B00AD; Thu, 6 Jun 2024 17:26:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE4EE6B00B0; Thu, 6 Jun 2024 17:26:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id BC7036B00AB for ; Thu, 6 Jun 2024 17:26:25 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 412A4120FBE for ; Thu, 6 Jun 2024 21:26:25 +0000 (UTC) X-FDA: 82201747530.23.680BF72 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf20.hostedemail.com (Postfix) with ESMTP id 8AD501C000B for ; Thu, 6 Jun 2024 21:26:23 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=none; spf=pass (imf20.hostedemail.com: domain of "SRS0=WH+2=NI=goodmis.org=rostedt@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=WH+2=NI=goodmis.org=rostedt@kernel.org"; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717709183; a=rsa-sha256; cv=none; b=sfV3Q4phLaYxMjfL1FYIMt8aqx326aG2T08ao9RoO1qBqP6PUp0Sw1ALCR+Vu7TnO7p0Gn IkxukMBotmnITlaiw/Y6abpp860+QK/MCGBYZM5+0Rlj+oF/FISo2zOEdAscUDY3LMulL+ 85yot+iV1xu2kxrO387tyb96Zji6+ZA= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=none; spf=pass (imf20.hostedemail.com: domain of "SRS0=WH+2=NI=goodmis.org=rostedt@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=WH+2=NI=goodmis.org=rostedt@kernel.org"; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717709183; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s3kxjfwKBIlKyh9ydlf6kQ7HUhPeiMEvU3E57WA+mAc=; b=u+/yltDVsxgczATSTwI/CuEk7cux0YW92NwZzBSgwX9l9LdYuCOOaOsvFh0CxH5VGA8OaP qa4iePnWYJd9yoqQ5iiu7vKmKEacF5scAXXb6N7SKkr0GLo068dI+nwecGNyUr2J3/W+dK ZgjyLqVSR4J0/r5NwBVS9NZLw6fTUD8= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id B179D61C1A; Thu, 6 Jun 2024 21:26:22 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 711ABC2BD10; Thu, 6 Jun 2024 21:26:19 +0000 (UTC) Date: Thu, 6 Jun 2024 17:26:31 -0400 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Cc: Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , Vincent Donnefort , Joel Fernandes , Daniel Bristot de Oliveira , Ingo Molnar , Peter Zijlstra , suleiman@google.com, Thomas Gleixner , Vineeth Pillai , Youssef Esmat , Beau Belgrave , Alexander Graf , Baoquan He , Borislav Petkov , "Paul E. McKenney" , David Howells , Mike Rapoport , Dave Hansen , Tony Luck , Guenter Roeck , Ross Zwisler , Kees Cook , linux-mm@kvack.org Subject: Re: [PATCH v3 08/13] tracing: Add option to use memmapped memory for trace boot instance Message-ID: <20240606172631.4a4b1cf7@rorschach.local.home> In-Reply-To: <20240606212137.333436708@goodmis.org> References: <20240606211735.684785459@goodmis.org> <20240606212137.333436708@goodmis.org> 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-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 8AD501C000B X-Stat-Signature: 1qg8qgmhg4cqykartygw6fwsf59ppm4j X-HE-Tag: 1717709183-718793 X-HE-Meta: U2FsdGVkX18S305p2eyEY98ZRV/czxiIzhFpNFrCeGmofO6h77bTg9ByDJDD1X4mqUtfQd9U/aC610Q+gu43ZxrDOt2dQFvsJ3vl4OlfZvugTBIMRPlcvbsb9g7yVT2r6t3M6Ndma1gABW0P7IfXjtOEw1nh7YdLMJbuw1DOEHA6UtvI6jtM4o5O4yEZkt12X4UIUWa9WytmKooe7MyDPZfcaMkI0NankKDjc8Kzh1q5j6jodxof0pHg3ixlAtl6L7QttMmszKmk1Ongd7Hf3T9yvzbQWVr/mIsx24cMpAy9HKH3p2WgVOZEDmdqG44H5hrvHdKJdpoHHH1QVyOhaLogceGlKhVax7aYWa/yIzY6PHs7ojH1+a1QkDEZH8P/axFAEHkFEL2JO6q9dMdL04/ClDCiptskG0qxjP3ybYVezoGZ6SHB83XpWdDBb1lBosuigf/aNFM9bi36LTjA8o/gdsc5u66BFbv4aDnpC1FLvoYftbDpqlvX9y2OVjREHHvUbyuCDT2GYGUWSDguAFvBmnao2Efs24lHa2XeSW6EzNGWReNopNSwEq6lzt8OV1RnSZ3Z/78qFzPvQp3Yeaui+lURP9dHaZ6+BvnDNoAF+30oNlmCfDyN9/F5RfgYqfILO8QQExPQx7w3Dtt0LcTUGcl1cVfgl8eSqH1cBiNrooDlNEb++jHGf61cDCt5hn3Qyt4myuSlKs8KdZm74y4VlWwdUbmI+ZU5NL3mp+BEqe+1EO9ktfkq0q27Qru2j4joAgl+AQ1DN5YQ89tcSXg5USPCubQ/8ZQp8xDozJ9eV+hM53RuZrQ9QUbA7N1qvc5JnkC7uoiQnXwnzzbRVM+w9kUMchVtQcenYkzARxwMyAro7MIzbFzVH2rcFhvvz5uKxd6RxnF2wYM1b0G2CYn19tkxYxwpryT1qB2nvdwJzAGCu9SqoSfKUwDlzbPmdNJphY4f0qY72ZggszH xhCZlJRz X5Q1FsBF92qDetzE7O8crm+nV1hPC3M5BXIRDnamJVwq8ii6ZLaLxCXCWPTjHLjN6UhHcSA8tzf8TjY2jNIFXidsGxpakINnEc5iNhyckly1RLCioiC8f2RlwHNDM684o9V1nR6ywZUlI1LylHX8C34ZJGd53URhY7tyof8ChOQRfkGsAYYKdcD6kLXAuHwRTxN3QPreKj0ygR+C4Iy4r6x9t+xlRxaL22jztecfWS8aqOIMiN3PyYTUYiNmjmv9f+f1QWLgKkEbs614UemuWPF6LRNIfgoHyURtwiskmdtrjwJjishBE3qkjGMyNcYE7OUL9g/Pop5Jke0beYKqLzCmU5DUR3D2U/dC6lxqag2i6j1cOIAqTTUaVBQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Memory management folks. Please review this patch. Specifically the "map_pages()" function below. On Thu, 06 Jun 2024 17:17:43 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > Add an option to the trace_instance kernel command line parameter that > allows it to use the reserved memory from memmap boot parameter. > > memmap=12M$0x284500000 trace_instance=boot_mapped@0x284500000:12M > > The above will reserves 12 megs at the physical address 0x284500000. > The second parameter will create a "boot_mapped" instance and use the > memory reserved as the memory for the ring buffer. > > That will create an instance called "boot_mapped": > > /sys/kernel/tracing/instances/boot_mapped > > Note, because the ring buffer is using a defined memory ranged, it will > act just like a memory mapped ring buffer. It will not have a snapshot > buffer, as it can't swap out the buffer. The snapshot files as well as any > tracers that uses a snapshot will not be present in the boot_mapped > instance. > > Cc: linux-mm@kvack.org > Signed-off-by: Steven Rostedt (Google) > --- > .../admin-guide/kernel-parameters.txt | 9 +++ > kernel/trace/trace.c | 75 +++++++++++++++++-- > 2 files changed, 78 insertions(+), 6 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index b600df82669d..ff26b6094e79 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -6754,6 +6754,15 @@ > the same thing would happen if it was left off). The irq_handler_entry > event, and all events under the "initcall" system. > > + If memory has been reserved (see memmap for x86), the instance > + can use that memory: > + > + memmap=12M$0x284500000 trace_instance=boot_map@0x284500000:12M > + > + The above will create a "boot_map" instance that uses the physical > + memory at 0x284500000 that is 12Megs. The per CPU buffers of that > + instance will be split up accordingly. > + > trace_options=[option-list] > [FTRACE] Enable or disable tracer options at boot. > The option-list is a comma delimited list of options > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 622fe670949d..13e89023f33b 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -9504,6 +9504,31 @@ static int instance_mkdir(const char *name) > return ret; > } > > +static u64 map_pages(u64 start, u64 size) > +{ > + struct page **pages; > + phys_addr_t page_start; > + unsigned int page_count; > + unsigned int i; > + void *vaddr; > + > + page_count = DIV_ROUND_UP(size, PAGE_SIZE); > + > + page_start = start; > + pages = kmalloc_array(page_count, sizeof(struct page *), GFP_KERNEL); > + if (!pages) > + return 0; > + > + for (i = 0; i < page_count; i++) { > + phys_addr_t addr = page_start + i * PAGE_SIZE; > + pages[i] = pfn_to_page(addr >> PAGE_SHIFT); > + } > + vaddr = vmap(pages, page_count, VM_MAP, PAGE_KERNEL); > + kfree(pages); > + > + return (u64)(unsigned long)vaddr; > +} If for some reason the memmap=nn$ss fails, but this still gets called, will the above just map over any memory. That is, is it possible that the kernel could have used this memory? Is there a way to detect this? That is, I don't want this to succeed if the memory location it's about to map to is used by the kernel, or will be used by user space. -- Steve > + > /** > * trace_array_get_by_name - Create/Lookup a trace array, given its name. > * @name: The name of the trace array to be looked up/created. > @@ -10350,6 +10375,7 @@ __init static void enable_instances(void) > { > struct trace_array *tr; > char *curr_str; > + char *name; > char *str; > char *tok; > > @@ -10358,19 +10384,56 @@ __init static void enable_instances(void) > str = boot_instance_info; > > while ((curr_str = strsep(&str, "\t"))) { > + unsigned long start = 0; > + unsigned long size = 0; > + unsigned long addr = 0; > > tok = strsep(&curr_str, ","); > + name = strsep(&tok, "@"); > + if (tok) { > + start = memparse(tok, &tok); > + if (!start) { > + pr_warn("Tracing: Invalid boot instance address for %s\n", > + name); > + continue; > + } > + } > > - if (IS_ENABLED(CONFIG_TRACER_MAX_TRACE)) > - do_allocate_snapshot(tok); > + if (start) { > + if (*tok != ':') { > + pr_warn("Tracing: No size specified for instance %s\n", name); > + continue; > + } > + tok++; > + size = memparse(tok, &tok); > + if (!size) { > + pr_warn("Tracing: Invalid boot instance size for %s\n", > + name); > + continue; > + } > + addr = map_pages(start, size); > + if (addr) { > + pr_info("Tracing: mapped boot instance %s at physical memory 0x%lx of size 0x%lx\n", > + name, start, size); > + } else { > + pr_warn("Tracing: Failed to map boot instance %s\n", name); > + continue; > + } > + } else { > + /* Only non mapped buffers have snapshot buffers */ > + if (IS_ENABLED(CONFIG_TRACER_MAX_TRACE)) > + do_allocate_snapshot(tok); > + } > > - tr = trace_array_get_by_name(tok, NULL); > + tr = trace_array_create_systems(name, NULL, addr, size); > if (!tr) { > - pr_warn("Failed to create instance buffer %s\n", curr_str); > + pr_warn("Tracing: Failed to create instance buffer %s\n", curr_str); > continue; > } > - /* Allow user space to delete it */ > - trace_array_put(tr); > + > + /* Only allow non mapped buffers to be deleted */ > + if (!start) > + trace_array_put(tr); > > while ((tok = strsep(&curr_str, ","))) { > early_enable_events(tr, tok, true);