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 D27BBCD128A for ; Wed, 10 Apr 2024 17:41:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 61B0D6B007B; Wed, 10 Apr 2024 13:41:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5CB6E6B0088; Wed, 10 Apr 2024 13:41:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 493CE6B0089; Wed, 10 Apr 2024 13:41:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 2C0BE6B007B for ; Wed, 10 Apr 2024 13:41:23 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id B32F3A0899 for ; Wed, 10 Apr 2024 17:41:22 +0000 (UTC) X-FDA: 81994338804.04.F652859 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf14.hostedemail.com (Postfix) with ESMTP id 6C22E100005 for ; Wed, 10 Apr 2024 17:41:20 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of "SRS0=DmSZ=LP=goodmis.org=rostedt@kernel.org" designates 145.40.73.55 as permitted sender) smtp.mailfrom="SRS0=DmSZ=LP=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=1712770881; 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=u24wFyszMudyHp173GWtMa6jcpx2WZ4jk1e5aHCk0Jk=; b=wiR+BKrArobDf7nyJTAaIzRAjYQG/i2BSURUUqheJbBJk06EK5rtNrl6A9HZq9f/mCp75E R1OaYWTpd3Qr3N7z2vI/4kiOTYQhs1Nh6sBr9rG4H0znLkNvGviSwfrMSmJBDYv4UlNmdz jW5+dFuXdmyG3JBWo4cRlIpNyENEWl0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712770881; a=rsa-sha256; cv=none; b=J6BFFnOdFA/7ebTWDMZjIIDvNjFofhuNsEI20pKOEyGCROX8qdFLoOGwnNJiIYMiICpeNE uP+hrJbJ6sIFzKORqXtzCNlX/kUmmOW13OXzmA/I7vCfnP9ZNqX3D0ASjtUCslDhPl4Mhn 4v7UAC/AK3dL0BtCrbsLGErqbrjQla4= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of "SRS0=DmSZ=LP=goodmis.org=rostedt@kernel.org" designates 145.40.73.55 as permitted sender) smtp.mailfrom="SRS0=DmSZ=LP=goodmis.org=rostedt@kernel.org"; dmarc=none Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 75341CE2ABD; Wed, 10 Apr 2024 17:41:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CCB65C433F1; Wed, 10 Apr 2024 17:41:15 +0000 (UTC) Date: Wed, 10 Apr 2024 13:43:52 -0400 From: Steven Rostedt To: Vincent Donnefort Cc: mhiramat@kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com, kernel-team@android.com, rdunlap@infradead.org, linux-mm@kvack.org Subject: Re: [PATCH v20 2/5] ring-buffer: Introducing ring-buffer mapping functions Message-ID: <20240410134352.66d35fbc@gandalf.local.home> In-Reply-To: <20240406173649.3210836-3-vdonnefort@google.com> References: <20240406173649.3210836-1-vdonnefort@google.com> <20240406173649.3210836-3-vdonnefort@google.com> 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 X-Rspamd-Queue-Id: 6C22E100005 X-Rspam-User: X-Stat-Signature: tx1jw36rwkx3yzd51mny7keycoh9e1iz X-Rspamd-Server: rspam03 X-HE-Tag: 1712770880-741342 X-HE-Meta: U2FsdGVkX18JOUxIpZy6Su9Kd39D8PN/QSAYBx5+vNDKEbC46hryGzSE20tHE2QXvSaK4P1P3G1FsToFWTlhbOc89s6YkBCq/dnKuCLjX+11HXYk+20mhkhTIGMvac04YYpHlXapWBjn4zX9SPqO+QyECGApYqbPl9XWUPxAlwNmHL4BGZzP4lXREYqdHP9r9lDFjaPY7REP5BzgcXG16RltXPMLp7Xey3UT+qxjjyuLMmg7I3YQKhr+ov9uSrstVkvVyt3d5SbwAWJr9ctvfaVU26YQjWUHR/4y2uu84Du2L4pSBjHE3CCS6GYxdd9LXCAdR6+PlSE8QpsZecGpSn7I1ilOslKBO7t/r+nqyZW8zQzVunQu/M6pvwO3QoYyc6zY9sc2HmRv9MFWjMqjLhIwcy5emCzwFxatfgiEQKyoB4xn5ZoaYS11De03RgT0d2MpBB0kX8daIejB3kEX8VWm3EoCWEnSkQZoLUuDEtNkJhS+P9KILnZByG9SRBpjBfxxDw2AJSbdcFP9HVOeNI8gCvXf4Te3RrAGeIs4xX3QFI/3iob6DdQARfJW6r8gF3MiqteHHqTDQRyLAKU8SfJRJg1uXr6kikrHEkl4zHNbMM1lxXWYrkD/Mp17CemJ/bcfEyRgPBArkvTnyopGnLw3xC8uGQ53FfzKrpJK2+grH8FtUJh5es/ccCq11I/Rggw7M61RDPlD5ZCwN11wlEErxIq4S+IT/oh3UuQiB/Ke0oZkjqxqTfMHrZ3n6mnkPTKuO26G9HJA6eAHb9KK794rcnsIiC4y1S+43SKWAE+5d06HTPsCSLnjkmLf9vkPShu3ivu9KsQjV/RX9fF/+L5Ey/4rgR0XHH0tU1GziXTvFjhT4xIyh88V5qe/zvpGKaCyZXPUYFx3ZaYASzyOVWRB/UttBrwzt8X5sW6NBBOOPvBLmm8A/4W4TyeuDjJ3jVd2LZO0vRRRKlIAJKr 8YwF4tzT nvv1V7u+GLzUZZbGxNG9BAsipx4W3BOgEyuoxk4HA5s4TdkmYn/3Tz13fy5tCTSo2BfBgyA6F5wf0dO2XFPROrPami/UZjqOwFjCulpe8RNH9Jo4jLvIwBWjjnZhVlAOe9LfeBo+3MpyaqenVO0EtRSnG+i5+L6l3CctBQGECUaScH9iVhFOlHD8tBKRbdSsx1QfvtZGnD96F+6vhdj5OSN8aCZhHBVSGJGoXKNLFvV7BHXWw+mD/Ttj/NoTy46X/ARq+GMdriTb4nN4= 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: On Sat, 6 Apr 2024 18:36:46 +0100 Vincent Donnefort wrote: > +int ring_buffer_map(struct trace_buffer *buffer, int cpu, > + struct vm_area_struct *vma) > +{ > + struct ring_buffer_per_cpu *cpu_buffer; > + unsigned long flags, *subbuf_ids; > + int err = 0; > + > + if (!cpumask_test_cpu(cpu, buffer->cpumask)) > + return -EINVAL; > + > + cpu_buffer = buffer->buffers[cpu]; > + > + mutex_lock(&cpu_buffer->mapping_lock); > + > + if (cpu_buffer->mapped) { > + err = __rb_map_vma(cpu_buffer, vma); > + if (!err) > + err = __rb_inc_dec_mapped(cpu_buffer, true); > + mutex_unlock(&cpu_buffer->mapping_lock); > + return err; > + } > + > + /* prevent another thread from changing buffer/sub-buffer sizes */ > + mutex_lock(&buffer->mutex); > + > + err = rb_alloc_meta_page(cpu_buffer); > + if (err) > + goto unlock; > + > + /* subbuf_ids include the reader while nr_pages does not */ > + subbuf_ids = kcalloc(cpu_buffer->nr_pages + 1, sizeof(*subbuf_ids), GFP_KERNEL); > + if (!subbuf_ids) { > + rb_free_meta_page(cpu_buffer); > + err = -ENOMEM; > + goto unlock; > + } > + > + atomic_inc(&cpu_buffer->resize_disabled); > + > + /* > + * Lock all readers to block any subbuf swap until the subbuf IDs are > + * assigned. > + */ > + raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > + rb_setup_ids_meta_page(cpu_buffer, subbuf_ids); > + raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags); > + > + err = __rb_map_vma(cpu_buffer, vma); > + if (!err) { > + raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > + cpu_buffer->mapped = 1; > + raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags); > + } else { > + kfree(cpu_buffer->subbuf_ids); > + cpu_buffer->subbuf_ids = NULL; > + rb_free_meta_page(cpu_buffer); > + } > +unlock: Nit: For all labels, please add a space before them. Otherwise, diffs will show "unlock" as the function and not "ring_buffer_map", making it harder to find where the change is. Same for the labels below. -- Steve > + mutex_unlock(&buffer->mutex); > + mutex_unlock(&cpu_buffer->mapping_lock); > + > + return err; > +} > + > +int ring_buffer_unmap(struct trace_buffer *buffer, int cpu) > +{ > + struct ring_buffer_per_cpu *cpu_buffer; > + unsigned long flags; > + int err = 0; > + > + if (!cpumask_test_cpu(cpu, buffer->cpumask)) > + return -EINVAL; > + > + cpu_buffer = buffer->buffers[cpu]; > + > + mutex_lock(&cpu_buffer->mapping_lock); > + > + if (!cpu_buffer->mapped) { > + err = -ENODEV; > + goto out; > + } else if (cpu_buffer->mapped > 1) { > + __rb_inc_dec_mapped(cpu_buffer, false); > + goto out; > + } > + > + mutex_lock(&buffer->mutex); > + raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > + > + cpu_buffer->mapped = 0; > + > + raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags); > + > + kfree(cpu_buffer->subbuf_ids); > + cpu_buffer->subbuf_ids = NULL; > + rb_free_meta_page(cpu_buffer); > + atomic_dec(&cpu_buffer->resize_disabled); > + > + mutex_unlock(&buffer->mutex); > +out: > + mutex_unlock(&cpu_buffer->mapping_lock); > + > + return err; > +} > + > +int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu) > +{ > + struct ring_buffer_per_cpu *cpu_buffer; > + unsigned long reader_size; > + unsigned long flags; > + > + cpu_buffer = rb_get_mapped_buffer(buffer, cpu); > + if (IS_ERR(cpu_buffer)) > + return (int)PTR_ERR(cpu_buffer); > + > + raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > +consume: > + if (rb_per_cpu_empty(cpu_buffer)) > + goto out; > + > + reader_size = rb_page_size(cpu_buffer->reader_page); > + > + /* > + * There are data to be read on the current reader page, we can > + * return to the caller. But before that, we assume the latter will read > + * everything. Let's update the kernel reader accordingly. > + */ > + if (cpu_buffer->reader_page->read < reader_size) { > + while (cpu_buffer->reader_page->read < reader_size) > + rb_advance_reader(cpu_buffer); > + goto out; > + } > + > + if (WARN_ON(!rb_get_reader_page(cpu_buffer))) > + goto out; > + > + goto consume; > +out: > + /* Some archs do not have data cache coherency between kernel and user-space */ > + flush_dcache_folio(virt_to_folio(cpu_buffer->reader_page->page)); > + > + rb_update_meta_page(cpu_buffer); > + > + raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags); > + rb_put_mapped_buffer(cpu_buffer); > + > + return 0; > +} > + > /* > * We only allocate new buffers, never free them if the CPU goes down. > * If we were to free the buffer, then the user would lose any trace that was in