linux-kernel - Re: [PATCH v10 09/10] erofs: support compressed inodes for page cache share

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a43ac775-aa82-44cc-ab01-9126eba98e75@linux.alibaba.com>
Date: Tue, 23 Dec 2025 16:18:44 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Hongbo Li <lihongbo22@...wei.com>
Cc: linux-fsdevel@...r.kernel.org, linux-erofs@...ts.ozlabs.org,
 linux-kernel@...r.kernel.org, Chao Yu <chao@...nel.org>,
 Christian Brauner <brauner@...nel.org>, djwong@...nel.org,
 Amir Goldstein <amir73il@...il.com>, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v10 09/10] erofs: support compressed inodes for page cache
 share



On 2025/12/23 09:56, Hongbo Li wrote:
> From: Hongzhen Luo <hongzhen@...ux.alibaba.com>
> 
> This patch adds page cache sharing functionality for compressed inodes.
> 
> Signed-off-by: Hongzhen Luo <hongzhen@...ux.alibaba.com>
> Signed-off-by: Hongbo Li <lihongbo22@...wei.com>
> ---
>   fs/erofs/zdata.c | 42 ++++++++++++++++++++++++++++++++----------
>   1 file changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index 65da21504632..465918093984 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -493,7 +493,7 @@ enum z_erofs_pclustermode {
>   };
>   
>   struct z_erofs_frontend {
> -	struct inode *const inode;
> +	struct inode *inode;
>   	struct erofs_map_blocks map;
>   	struct z_erofs_bvec_iter biter;
>   
> @@ -1883,10 +1883,18 @@ static void z_erofs_pcluster_readmore(struct z_erofs_frontend *f,
>   
>   static int z_erofs_read_folio(struct file *file, struct folio *folio)
>   {
> -	struct inode *const inode = folio->mapping->host;
> -	Z_EROFS_DEFINE_FRONTEND(f, inode, folio_pos(folio));
> +	struct inode *const inode = folio->mapping->host, *realinode;
> +	Z_EROFS_DEFINE_FRONTEND(f, NULL, folio_pos(folio));
>   	int err;
>   
> +	if (erofs_is_ishare_inode(inode))
> +		realinode = erofs_ishare_iget(inode);
> +	else
> +		realinode = inode;

I don't think it makes any sense to differ those two cases, just

	struct inode *inode = folio->mapping->host;
	struct inode *realinode = erofs_get_real_inode(inode);
	Z_EROFS_DEFINE_FRONTEND(f, realinode, folio_pos(folio));

...

> +
> +	if (!realinode)
> +		return -EIO;

That is an impossible case, just `DBG_BUGON(!realinode);`

> +	f.inode = realinode;
>   	trace_erofs_read_folio(folio, false);
>   	z_erofs_pcluster_readmore(&f, NULL, true);
>   	err = z_erofs_scan_folio(&f, folio, false);
> @@ -1896,23 +1904,34 @@ static int z_erofs_read_folio(struct file *file, struct folio *folio)
>   	/* if some pclusters are ready, need submit them anyway */
>   	err = z_erofs_runqueue(&f, 0) ?: err;
>   	if (err && err != -EINTR)
> -		erofs_err(inode->i_sb, "read error %d @ %lu of nid %llu",
> -			  err, folio->index, EROFS_I(inode)->nid);
> +		erofs_err(realinode->i_sb, "read error %d @ %lu of nid %llu",
> +			  err, folio->index, EROFS_I(realinode)->nid);
>   
>   	erofs_put_metabuf(&f.map.buf);
>   	erofs_release_pages(&f.pagepool);
> +
> +	if (erofs_is_ishare_inode(inode))
> +		erofs_ishare_iput(realinode);

	erofs_put_real_inode(realinode);

>   	return err;
>   }
>   
>   static void z_erofs_readahead(struct readahead_control *rac)
>   {
> -	struct inode *const inode = rac->mapping->host;
> -	Z_EROFS_DEFINE_FRONTEND(f, inode, readahead_pos(rac));
> +	struct inode *const inode = rac->mapping->host, *realinode;
> +	Z_EROFS_DEFINE_FRONTEND(f, NULL, readahead_pos(rac));
>   	unsigned int nrpages = readahead_count(rac);
>   	struct folio *head = NULL, *folio;
>   	int err;
>   
> -	trace_erofs_readahead(inode, readahead_index(rac), nrpages, false);
> +	if (erofs_is_ishare_inode(inode))
> +		realinode = erofs_ishare_iget(inode);
> +	else
> +		realinode = inode;
> +
> +	if (!realinode)
> +		return;
> +	f.inode = realinode;

Same here.

Thanks,
Gao Xiang