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 5164F1391; Sat, 24 Jan 2026 20:53:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769288020; cv=none; b=iEYUi9X32cCbIWNyCYVQW0HD3m12/Xu6dWE0El8TsBvkZM5/39DR8+M9S9Gfdv5gvmijBk+IpGRO33jKBac8vT7tvn1GGSpeKc1/RFIK9rA2ux166oo1BbLwKxxHVULRrkWq8r2L92x7+APPrJnZ/3DOykBTFUB3Dvtpfv2l09I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769288020; c=relaxed/simple; bh=+z7P8o/2AvAS2HxAhMpt7l6mG9q9YNMOg7B1XIrmpiY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lFmQ4Wy4nkOPjJJhB633Z71Uqq3KemY169lSmLJuljjBEAkS9xloE5mQ2aU+JGcUeERQTaxICf+lKK1NLrwwaYY8QwjEBwrRjPgbiwnIiEu+czi5AxUC8B9qx1DKOwKyq1Ynv1SbCj4A1MYlPNxXeR7xZI+oNvx1svbXVccE0p8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jrnYUkgM; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="jrnYUkgM" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 75893C19421; Sat, 24 Jan 2026 20:53:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769288019; bh=+z7P8o/2AvAS2HxAhMpt7l6mG9q9YNMOg7B1XIrmpiY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jrnYUkgMgLh95HZVEGLxtKpEEPIJ9i+kCGXVczoDgweCyruT+jbNyrNC0ntS8jKe/ h8ekQd3zwR8Z4KEvr4QksVUdZcFlsWQ9nPDrmgjbA7hghI6ep4soiakTKR6jaQpXPK C0h504KGR/6MLiq82fhUImEg0m7xb/afNx7buBCXCrZHaw9Z4/qC68d5tYpzvWWZrO x7W2QAcFNv/NYmm5JQ54O/qlmWpChFUAs/IBBUvBhftkiCO8o0TDWOa5e7M+/S9zTp yIWglfmCgF3A6YNHzKFcoFihtN7UU1tZPF88iH9op2/GsFRkvLaHbCCFHjwa5pJ1iX kcddSF2E6YUNQ== Date: Sat, 24 Jan 2026 12:53:29 -0800 From: Eric Biggers To: Christoph Hellwig Cc: Al Viro , Christian Brauner , Jan Kara , David Sterba , Theodore Ts'o , Jaegeuk Kim , Chao Yu , Andrey Albershteyn , "Matthew Wilcox (Oracle)" , linux-fsdevel@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, fsverity@lists.linux.dev Subject: Re: [PATCH 05/11] fsverity: kick off hash readahead at data I/O submission time Message-ID: <20260124205329.GE2762@quark> References: <20260122082214.452153-1-hch@lst.de> <20260122082214.452153-6-hch@lst.de> Precedence: bulk X-Mailing-List: fsverity@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260122082214.452153-6-hch@lst.de> On Thu, Jan 22, 2026 at 09:22:01AM +0100, Christoph Hellwig wrote: > +/** > + * generic_readahead_merkle_tree() - generic ->readahead_merkle_tree helper > + * @inode: inode containing the Merkle tree > + * @index: 0-based index of the first page to read ahead in the inode > + * @nr_pages: number of data pages to read ahead > + * > + * The caller needs to adjust @index from the Merkle-tree relative index passed > + * to ->read_merkle_tree_page to the actual index where the Merkle tree is > + * stored in the page cache for @inode. > + */ > +void generic_readahead_merkle_tree(struct inode *inode, pgoff_t index, > + unsigned long nr_pages) > { > struct folio *folio; > > folio = __filemap_get_folio(inode->i_mapping, index, FGP_ACCESSED, 0); > - if (IS_ERR(folio) || !folio_test_uptodate(folio)) { > + if (PTR_ERR(folio) == -ENOENT || !folio_test_uptodate(folio)) { This dereferences an ERR_PTR() when __filemap_get_folio() returns an error other than -ENOENT. > diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c > index cba5d6af4e04..430306abc4c6 100644 > --- a/fs/verity/read_metadata.c > +++ b/fs/verity/read_metadata.c > @@ -28,24 +28,24 @@ static int fsverity_read_merkle_tree(struct inode *inode, > if (offset >= end_offset) > return 0; > offs_in_page = offset_in_page(offset); > + index = offset >> PAGE_SHIFT; > last_index = (end_offset - 1) >> PAGE_SHIFT; > > + __fsverity_readahead(inode, vi, offset, last_index - index + 1); This passes a position in the Merkle tree to a function that expects a position in the file data. I think the correct thing to do here would be the following: if (inode->i_sb->s_vop->readahead_merkle_tree) inode->i_sb->s_vop->readahead_merkle_tree(inode, index, last_index - index + 1); Then __fsverity_readahead() can be folded into fsverity_readahead(). > +void __fsverity_readahead(struct inode *inode, const struct fsverity_info *vi, > + loff_t data_start_pos, unsigned long nr_pages) > +{ > + const struct merkle_tree_params *params = &vi->tree_params; > + u64 start_hidx = data_start_pos >> params->log_blocksize; > + u64 end_hidx = (data_start_pos + ((nr_pages - 1) << PAGE_SHIFT)) >> > + params->log_blocksize; (nr_pages - 1) << PAGE_SHIFT can overflow an 'unsigned long'. (nr_pages - 1) needs to be cast to u64 before doing the shift. But also it would make more sense to pass (pgoff_t start_index, unsigned long nr_pages) instead of (loff_t data_start_pos, unsigned long nr_pages), so that the two numbers have the same units. start_idx and end_hidx could then be computed as follows: u64 start_hidx = (u64)start_index << params->log_blocks_per_page; u64 end_hidx = (((u64)start_index + nr_pages) << params->log_blocks_per_page) - 1; Note that fsverity_readahead() derives the position from the index. If it just used the index directly, that would be more direct. > + int level; > + > + if (!inode->i_sb->s_vop->readahead_merkle_tree) > + return; > + if (unlikely(data_start_pos >= inode->i_size)) > + return; The check against i_size shouldn't be necessary: the caller should just call this only for data it's actually going to read. > + for (level = 0; level < params->num_levels; level++) { > + unsigned long level_start = params->level_start[level]; > + unsigned long next_start_hidx = start_hidx >> params->log_arity; > + unsigned long next_end_hidx = end_hidx >> params->log_arity; > + unsigned long start_idx = (level_start + next_start_hidx) >> > + params->log_blocks_per_page; > + unsigned long end_idx = (level_start + next_end_hidx) >> > + params->log_blocks_per_page; start_idx and end_idx should have type pgoff_t to make it clear that they're page indices. > +EXPORT_SYMBOL_GPL(fsverity_readahead); This should be below the definition of fsverity_readahead, not the definition of __fsverity_readahead. > +/** > + * fsverity_readahead() - kick off readahead on fsverity hashes > + * @folio: first folio that is being read folio => file data folio Otherwise it can be confused with the Merkle tree. > + * Start readahead on fsverity hashes. To be called from the file systems > + * ->read_folio and ->readahead methods to ensure that the hashes are > + * already cached on completion of the file data read if possible. Similarly, it would be helpful to clarify that the readahead is done on the hashes *that will be needed to verify the specified file data*. Otherwise it might sound like the caller is specifying the hashes to readahead directly. > + /** > + * Perform readahad of a Merkle tree for the given inode. readahad => readahead - Eric 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 lists.sourceforge.net (lists.sourceforge.net [216.105.38.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 877B2D715F6 for ; Sat, 24 Jan 2026 20:53:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type:Cc: Reply-To:From:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Subject:In-Reply-To:MIME-Version:References: Message-ID:To:Date:Sender:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=UFq66RlT4MGMoCIPmShR5HnUhnD1JaG3bGyJ0a3v11U=; b=LcpeI6hRcG1fLTFzS1JNCC0+Pn WeFXRZOvF4Fjow6z2OZePrLt8ai/BwpUe/1CAGOGIid0lezTRo3BLfcEYa0Y0BCvgG2dydExBiccs mpKM1tKPdC5ukKGQ79fNZPrcXwV4wB5fOrfESPj/HGbO2IRk/xOfGjKEcV4ePA3Q1BqY=; Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1vjkdf-0006qg-PO; Sat, 24 Jan 2026 20:53:47 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1vjkde-0006qW-0b for linux-f2fs-devel@lists.sourceforge.net; Sat, 24 Jan 2026 20:53:46 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=In-Reply-To:Content-Type:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Aq2/f4e74mPU1lJLpS1IkpMgZuo97fFzjhsVz1rqSlA=; b=EVSNZHlgEDzxkqct/hHFcPu6lf 7Xk9oQh4hZYQGJiFmCN3gEGTu26lp48C3S1iuaNfhCr5cf/+97lK4c1OEXggRmCQewHSXXo2eYLhQ AX+h0xk6NPcxVFE1H49N5O8Vi+m+McpGSwn7Ge1DgpXbkufk/CtZyOS66agbTKAnCSsc=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To :From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=Aq2/f4e74mPU1lJLpS1IkpMgZuo97fFzjhsVz1rqSlA=; b=QuYC2QIFP5k3lf9yRegtzTYX8j CXo0bPr57yB9FrCKHRIatETNBswBiRB4MeAaP7Tlz3Ab4ieCBVEPsG2RVXPHFiX2neZsbFz95FxLw 4/C8i+LMPT/i0ASdFoc544FrGyDqQsCGk00tQ9vb6Zw2EYF+4YK9y8JUy645zdVPPlN0=; Received: from sea.source.kernel.org ([172.234.252.31]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1vjkdd-0006h5-Eq for linux-f2fs-devel@lists.sourceforge.net; Sat, 24 Jan 2026 20:53:45 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 14984443CB; Sat, 24 Jan 2026 20:53:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 75893C19421; Sat, 24 Jan 2026 20:53:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769288019; bh=+z7P8o/2AvAS2HxAhMpt7l6mG9q9YNMOg7B1XIrmpiY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jrnYUkgMgLh95HZVEGLxtKpEEPIJ9i+kCGXVczoDgweCyruT+jbNyrNC0ntS8jKe/ h8ekQd3zwR8Z4KEvr4QksVUdZcFlsWQ9nPDrmgjbA7hghI6ep4soiakTKR6jaQpXPK C0h504KGR/6MLiq82fhUImEg0m7xb/afNx7buBCXCrZHaw9Z4/qC68d5tYpzvWWZrO x7W2QAcFNv/NYmm5JQ54O/qlmWpChFUAs/IBBUvBhftkiCO8o0TDWOa5e7M+/S9zTp yIWglfmCgF3A6YNHzKFcoFihtN7UU1tZPF88iH9op2/GsFRkvLaHbCCFHjwa5pJ1iX kcddSF2E6YUNQ== Date: Sat, 24 Jan 2026 12:53:29 -0800 To: Christoph Hellwig Message-ID: <20260124205329.GE2762@quark> References: <20260122082214.452153-1-hch@lst.de> <20260122082214.452153-6-hch@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20260122082214.452153-6-hch@lst.de> X-Headers-End: 1vjkdd-0006h5-Eq Subject: Re: [f2fs-dev] [PATCH 05/11] fsverity: kick off hash readahead at data I/O submission time X-BeenThere: linux-f2fs-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Eric Biggers via Linux-f2fs-devel Reply-To: Eric Biggers Cc: fsverity@lists.linux.dev, Christian Brauner , Theodore Ts'o , Andrey Albershteyn , "Matthew Wilcox \(Oracle\)" , linux-f2fs-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, Al Viro , Jaegeuk Kim , David Sterba , Jan Kara , linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net On Thu, Jan 22, 2026 at 09:22:01AM +0100, Christoph Hellwig wrote: > +/** > + * generic_readahead_merkle_tree() - generic ->readahead_merkle_tree helper > + * @inode: inode containing the Merkle tree > + * @index: 0-based index of the first page to read ahead in the inode > + * @nr_pages: number of data pages to read ahead > + * > + * The caller needs to adjust @index from the Merkle-tree relative index passed > + * to ->read_merkle_tree_page to the actual index where the Merkle tree is > + * stored in the page cache for @inode. > + */ > +void generic_readahead_merkle_tree(struct inode *inode, pgoff_t index, > + unsigned long nr_pages) > { > struct folio *folio; > > folio = __filemap_get_folio(inode->i_mapping, index, FGP_ACCESSED, 0); > - if (IS_ERR(folio) || !folio_test_uptodate(folio)) { > + if (PTR_ERR(folio) == -ENOENT || !folio_test_uptodate(folio)) { This dereferences an ERR_PTR() when __filemap_get_folio() returns an error other than -ENOENT. > diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c > index cba5d6af4e04..430306abc4c6 100644 > --- a/fs/verity/read_metadata.c > +++ b/fs/verity/read_metadata.c > @@ -28,24 +28,24 @@ static int fsverity_read_merkle_tree(struct inode *inode, > if (offset >= end_offset) > return 0; > offs_in_page = offset_in_page(offset); > + index = offset >> PAGE_SHIFT; > last_index = (end_offset - 1) >> PAGE_SHIFT; > > + __fsverity_readahead(inode, vi, offset, last_index - index + 1); This passes a position in the Merkle tree to a function that expects a position in the file data. I think the correct thing to do here would be the following: if (inode->i_sb->s_vop->readahead_merkle_tree) inode->i_sb->s_vop->readahead_merkle_tree(inode, index, last_index - index + 1); Then __fsverity_readahead() can be folded into fsverity_readahead(). > +void __fsverity_readahead(struct inode *inode, const struct fsverity_info *vi, > + loff_t data_start_pos, unsigned long nr_pages) > +{ > + const struct merkle_tree_params *params = &vi->tree_params; > + u64 start_hidx = data_start_pos >> params->log_blocksize; > + u64 end_hidx = (data_start_pos + ((nr_pages - 1) << PAGE_SHIFT)) >> > + params->log_blocksize; (nr_pages - 1) << PAGE_SHIFT can overflow an 'unsigned long'. (nr_pages - 1) needs to be cast to u64 before doing the shift. But also it would make more sense to pass (pgoff_t start_index, unsigned long nr_pages) instead of (loff_t data_start_pos, unsigned long nr_pages), so that the two numbers have the same units. start_idx and end_hidx could then be computed as follows: u64 start_hidx = (u64)start_index << params->log_blocks_per_page; u64 end_hidx = (((u64)start_index + nr_pages) << params->log_blocks_per_page) - 1; Note that fsverity_readahead() derives the position from the index. If it just used the index directly, that would be more direct. > + int level; > + > + if (!inode->i_sb->s_vop->readahead_merkle_tree) > + return; > + if (unlikely(data_start_pos >= inode->i_size)) > + return; The check against i_size shouldn't be necessary: the caller should just call this only for data it's actually going to read. > + for (level = 0; level < params->num_levels; level++) { > + unsigned long level_start = params->level_start[level]; > + unsigned long next_start_hidx = start_hidx >> params->log_arity; > + unsigned long next_end_hidx = end_hidx >> params->log_arity; > + unsigned long start_idx = (level_start + next_start_hidx) >> > + params->log_blocks_per_page; > + unsigned long end_idx = (level_start + next_end_hidx) >> > + params->log_blocks_per_page; start_idx and end_idx should have type pgoff_t to make it clear that they're page indices. > +EXPORT_SYMBOL_GPL(fsverity_readahead); This should be below the definition of fsverity_readahead, not the definition of __fsverity_readahead. > +/** > + * fsverity_readahead() - kick off readahead on fsverity hashes > + * @folio: first folio that is being read folio => file data folio Otherwise it can be confused with the Merkle tree. > + * Start readahead on fsverity hashes. To be called from the file systems > + * ->read_folio and ->readahead methods to ensure that the hashes are > + * already cached on completion of the file data read if possible. Similarly, it would be helpful to clarify that the readahead is done on the hashes *that will be needed to verify the specified file data*. Otherwise it might sound like the caller is specifying the hashes to readahead directly. > + /** > + * Perform readahad of a Merkle tree for the given inode. readahad => readahead - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel