From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 417C425541; Wed, 20 Dec 2023 13:00:29 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 98EAAC433C8; Wed, 20 Dec 2023 13:00:28 +0000 (UTC) Date: Wed, 20 Dec 2023 08:01:29 -0500 From: Steven Rostedt To: David Laight Cc: "linux-kernel@vger.kernel.org" , "linux-trace-kernel@vger.kernel.org" , Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Andrew Morton , Tzvetomir Stoyanov , Vincent Donnefort , "Kent Overstreet" Subject: Re: [PATCH v5 02/15] ring-buffer: Page size per ring buffer Message-ID: <20231220080129.3453bca8@gandalf.local.home> In-Reply-To: <84d3b41a72bd43dbb9d44921ef535c92@AcuMS.aculab.com> References: <20231219185414.474197117@goodmis.org> <20231219185628.009147038@goodmis.org> <84d3b41a72bd43dbb9d44921ef535c92@AcuMS.aculab.com> X-Mailer: Claws Mail 3.19.1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 20 Dec 2023 08:48:02 +0000 David Laight wrote: > From: Steven Rostedt > > Sent: 19 December 2023 18:54 > > From: "Tzvetomir Stoyanov (VMware)" > > > > Currently the size of one sub buffer page is global for all buffers and > > it is hard coded to one system page. In order to introduce configurable > > ring buffer sub page size, the internal logic should be refactored to > > work with sub page size per ring buffer. > > > ... > > - nr_pages = DIV_ROUND_UP(size, BUF_PAGE_SIZE); > > + /* Default buffer page size - one system page */ > > + buffer->subbuf_size = PAGE_SIZE - BUF_PAGE_HDR_SIZE; > > + > > + /* Max payload is buffer page size - header (8bytes) */ > > + buffer->max_data_size = buffer->subbuf_size - (sizeof(u32) * 2); > > + > > + nr_pages = DIV_ROUND_UP(size, buffer->subbuf_size); > > While not new, does this really make any sense for systems with 64k pages? > Wouldn't it be better to have units of 4k? Unfortunately, it has to be PAGE_SIZE (and for now it's a power of 2 to make masking easy). It's used for splice and will also be used for memory mapping with user space. > > ... > > @@ -5102,14 +5110,14 @@ unsigned long ring_buffer_size(struct trace_buffer *buffer, int cpu) > > { > > /* > > * Earlier, this method returned > > - * BUF_PAGE_SIZE * buffer->nr_pages > > + * buffer->subbuf_size * buffer->nr_pages > > * Since the nr_pages field is now removed, we have converted this to > > * return the per cpu buffer value. > > Overenthusiastic global replace... Possibly, but the comment still applies, and should probably be removed, as it's rather old (2012). It's basically just saying that the size use to be calculated from buffer->nr_pages and now it's calculated by buffer->buffers[cpu]->nr_pages. I think I'll just add a patch to remove that comment. Thanks, -- Steve