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 29AA9C4345F for ; Thu, 2 May 2024 13:45:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B3ADD6B009E; Thu, 2 May 2024 09:45:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AEC736B009F; Thu, 2 May 2024 09:45:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9D9A56B00A0; Thu, 2 May 2024 09:45:36 -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 7F2186B009E for ; Thu, 2 May 2024 09:45:36 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id EFA5AA0E7F for ; Thu, 2 May 2024 13:45:35 +0000 (UTC) X-FDA: 82073578230.09.BC611F9 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf02.hostedemail.com (Postfix) with ESMTP id 9B92180030 for ; Thu, 2 May 2024 13:45:33 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=none; spf=pass (imf02.hostedemail.com: domain of "SRS0=wRLO=MF=goodmis.org=rostedt@kernel.org" designates 145.40.73.55 as permitted sender) smtp.mailfrom="SRS0=wRLO=MF=goodmis.org=rostedt@kernel.org"; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714657534; a=rsa-sha256; cv=none; b=nU0h7xr+PJY0mA1cm/ei+EMbL43PiAwF/Fc/9jiNxYgQ81ZnEm7BTgUNI9ouWD1/HmAQaR eFqlH40CblV1jPvia5ccP3xHBBp5FNrudy4r7NQw9aUA20xR5+6oiIf9wHvdAofz0SHu0B nz5Ns/nqpHlJIxb9ECrrVHAXwGXtEh4= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=none; spf=pass (imf02.hostedemail.com: domain of "SRS0=wRLO=MF=goodmis.org=rostedt@kernel.org" designates 145.40.73.55 as permitted sender) smtp.mailfrom="SRS0=wRLO=MF=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=1714657534; 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=e0GHO9vMhHLeDphNtn8yGjWWwrGHlouNxeIuchibEvw=; b=FFDCnvEd6cjVolbAWPzJgFObXYAnGchDoAYgH8rQn8C/JIblWRhAEfDVW9c5ck/bdrRIpX ZVhbatW/tOPR2DZqB2TElv6rfhrmfqgg43FWc0Z7G/4vjb/EJVE5rHJpW0lpE602P52RJ+ fxi8Wzpjl1coNH+9TdeWqIa9uTOQ+/8= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 40F5FCE16B9; Thu, 2 May 2024 13:45:29 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4F32CC113CC; Thu, 2 May 2024 13:45:26 +0000 (UTC) Date: Thu, 2 May 2024 09:46:12 -0400 From: Steven Rostedt To: Vincent Donnefort Cc: David Hildenbrand , 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, rppt@kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v22 2/5] ring-buffer: Introducing ring-buffer mapping functions Message-ID: <20240502094612.7f92a3e4@gandalf.local.home> In-Reply-To: References: <20240430111354.637356-1-vdonnefort@google.com> <20240430111354.637356-3-vdonnefort@google.com> <78e20e98-bdfc-4d7b-a59c-988b81fcc58b@redhat.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-Stat-Signature: n4irca3rdfid4ruhcq1gc4bgihyih5ue X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 9B92180030 X-HE-Tag: 1714657533-467952 X-HE-Meta: U2FsdGVkX1/DY691kZXTg88LCXbkPImignqIUdzvTt6QU5pCNfv1du07lBggQBWlfyTEvHVlMX+fk8yIYJ9bstfa1Hgis6d6yFkX6VgA8dSPwjhMwqrEFiLhwPUNXY0fcqScdtnPpImPJ1YjvZYR7PSRx6ZazBaXdWlJPqKWqNk8wrGgiHsdsoOCMhC/0c3dbd90hcI8oQc6jLzjVHcI2otaaIMR2bdh2nycgHZB7sOMkGEyLBEsqKJVJohydX1E+1na1lKjlB5xSmkTGK8uj+6s2Pm2cyNyOwYF6UVFsL5Vzmt0ugi1iBdQ9c/0C28ToYBvPNhIR/KxXpWsETDmFiL6Y8Al+j6cT1Q0IAfxlVRiXz1oh+tMna32zxsCyWTSh4RvF8qXnZlglaLTWj/X+8jRdsGVJxAQm+t5IdFvE/If6FDkpkNZzt0xR32R3rQkAOPnl/qeUGVheAR4sgJTa3hvHQ6YGtQmTlkTPT6QRlR/Rug6XQPBO1Sll2r/u1jFIH1uJ/KUF6Yo/Xil/i4P17nrRc8SCu9h+eot/hcj6uZ5ds+SrlIo77/kr2o98BCWU0cgJyxT2otYqf245TQdaE9d3Nghk1unRT1wS3gjXvhpeY/chWwnBVuNWtaf6IamxkditEb72UygOaiLi+Kyi5EYF7Rb8X4nVK4xO4G4yhl5Fk/3KMTEVYmA6jP1Bo3GgT8dZhAugB++clqeHNcC9zjZKqHxO3Cs/5ZpwD/u7uQ3WcriGzbdSyE39vRWznOwAf5MDjqbxRXKTtiMlALiDSTa3p2SOnX2qw4HAJj8jxmXC5GjTJNTTlhCV5YaDz7d/trjAUb7j+VS7Wl72lOGs27++fA+FfElMml2l20Tp4VPZd2g8zsoTJW6OASgR2wZEG8C1pa3Tf2+CH7L2liqs+GdaJDc9aY8UPtJ4DFwtsb12KYPIQpFuj6lWL/AQTkcgblCBm5Ohf+lM5yE6C+ aEL7c9m0 POdSrAGDX87VlUUFqibEH3wjkPZLJsWKNvczWCreoPhkpt/hTl0MTTOamGlBURZHSwujdZ6dvbVWFlUBdfWV0ka++DMG8znvbBpQ0F1QvFddbcBkp7VweI9+KAJ0GPu2udpgjbJNESKBuslefRqmUWk5BwQhHRDrVbvocGnSLAoRQKuxZ8+7rjo57dWuuuuRQyPHusxocnruoj1u0Un+5OJt7aEvXr3/fSIDZe4uxg9RUeyi2KW2mIb1j7JeHWf9/ueH15PdP/yz4rYPrAuw0JBYg2w== 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 Thu, 2 May 2024 14:38:32 +0100 Vincent Donnefort wrote: > > > + while (s < nr_subbufs && p < nr_pages) { > > > + struct page *page = virt_to_page(cpu_buffer->subbuf_ids[s]); > > > + int off = 0; > > > + > > > + for (; off < (1 << (subbuf_order)); off++, page++) { > > > + if (p >= nr_pages) > > > + break; > > > + > > > + pages[p++] = page; > > > + } > > > + s++; > > > + } > > > + > > > + err = vm_insert_pages(vma, vma->vm_start, pages, &nr_pages); > > > > Nit: I did not immediately understand if we could end here with p < nr_pages > > (IOW, pages[] not completely filled). > > > > One source of confusion is the "s < nr_subbufs" check in the while loop: why > > is "p < nr_pages" insufficient? > > Hum, indeed, the "s < nr_subbufs" check is superfluous, nr_pages, is already > capped by the number of subbufs, there's no way we can overflow subbuf_ids[]. We can keep it as is, or perhaps change it to: while (p < nr_pages) { struct page *page; int off = 0; if (WARN_ON_ONCE(s >= nr_subbufs)) break; page = virt_to_page(cpu_buffer->subbuf_ids[s]); for (; off < (1 << (subbuf_order)); off++, page++) { if (p >= nr_pages) break; pages[p++] = page; } s++; } I don't like having an unchecked dependency between s and p. -- Steve