linux-kernel - Re: [PATCH] erofs: simplify readdir operation

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: <6c91643e-f55b-4998-b2b2-8eaa3ad747f3@linux.alibaba.com>
Date: Thu, 1 Aug 2024 19:31:46 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Hongzhen Luo <hongzhen@...ux.alibaba.com>, linux-erofs@...ts.ozlabs.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] erofs: simplify readdir operation



On 2024/8/1 19:26, Hongzhen Luo wrote:
>   - Use i_size instead of i_size_read() due to immutable fses;
> 
>   - Get rid of an unneeded goto since erofs_fill_dentries() also works;
> 
>   - Remove unnecessary lines.
> 
> Signed-off-by: Hongzhen Luo <hongzhen@...ux.alibaba.com>
> ---

What's the difference from the previous version? why not marking
it as v2?

Thanks,
Gao Xiang

>   fs/erofs/dir.c      | 35 ++++++++++++-----------------------
>   fs/erofs/internal.h |  2 +-
>   2 files changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
> index 2193a6710c8f..c3b90abdee37 100644
> --- a/fs/erofs/dir.c
> +++ b/fs/erofs/dir.c
> @@ -8,19 +8,15 @@
>   
>   static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
>   			       void *dentry_blk, struct erofs_dirent *de,
> -			       unsigned int nameoff, unsigned int maxsize)
> +			       unsigned int nameoff0, unsigned int maxsize)
>   {
> -	const struct erofs_dirent *end = dentry_blk + nameoff;
> +	const struct erofs_dirent *end = dentry_blk + nameoff0;
>   
>   	while (de < end) {
> -		const char *de_name;
> +		unsigned char d_type = fs_ftype_to_dtype(de->file_type);
> +		unsigned int nameoff = le16_to_cpu(de->nameoff);
> +		const char *de_name = (char *)dentry_blk + nameoff;
>   		unsigned int de_namelen;
> -		unsigned char d_type;
> -
> -		d_type = fs_ftype_to_dtype(de->file_type);
> -
> -		nameoff = le16_to_cpu(de->nameoff);
> -		de_name = (char *)dentry_blk + nameoff;
>   
>   		/* the last dirent in the block? */
>   		if (de + 1 >= end)
> @@ -52,21 +48,20 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
>   	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>   	struct super_block *sb = dir->i_sb;
>   	unsigned long bsz = sb->s_blocksize;
> -	const size_t dirsize = i_size_read(dir);
> -	unsigned int i = erofs_blknr(sb, ctx->pos);
>   	unsigned int ofs = erofs_blkoff(sb, ctx->pos);
>   	int err = 0;
>   	bool initial = true;
>   
>   	buf.mapping = dir->i_mapping;
> -	while (ctx->pos < dirsize) {
> +	while (ctx->pos < dir->i_size) {
> +		erofs_off_t dbstart = ctx->pos - ofs;
>   		struct erofs_dirent *de;
>   		unsigned int nameoff, maxsize;
>   
> -		de = erofs_bread(&buf, erofs_pos(sb, i), EROFS_KMAP);
> +		de = erofs_bread(&buf, dbstart, EROFS_KMAP);
>   		if (IS_ERR(de)) {
>   			erofs_err(sb, "fail to readdir of logical block %u of nid %llu",
> -				  i, EROFS_I(dir)->nid);
> +				  erofs_blknr(sb, dbstart), EROFS_I(dir)->nid);
>   			err = PTR_ERR(de);
>   			break;
>   		}
> @@ -79,25 +74,19 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
>   			break;
>   		}
>   
> -		maxsize = min_t(unsigned int, dirsize - ctx->pos + ofs, bsz);
> -
> +		maxsize = min_t(unsigned int, dir->i_size - dbstart, bsz);
>   		/* search dirents at the arbitrary position */
>   		if (initial) {
>   			initial = false;
> -
>   			ofs = roundup(ofs, sizeof(struct erofs_dirent));
> -			ctx->pos = erofs_pos(sb, i) + ofs;
> -			if (ofs >= nameoff)
> -				goto skip_this;
> +			ctx->pos = dbstart + ofs;
>   		}
>   
>   		err = erofs_fill_dentries(dir, ctx, de, (void *)de + ofs,
>   					  nameoff, maxsize);
>   		if (err)
>   			break;
> -skip_this:
> -		ctx->pos = erofs_pos(sb, i) + maxsize;
> -		++i;
> +		ctx->pos = dbstart + maxsize;
>   		ofs = 0;
>   	}
>   	erofs_put_metabuf(&buf);
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 736607675396..45dc15ebd870 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -220,7 +220,7 @@ struct erofs_buf {
>   };
>   #define __EROFS_BUF_INITIALIZER	((struct erofs_buf){ .page = NULL })
>   
> -#define erofs_blknr(sb, addr)	((addr) >> (sb)->s_blocksize_bits)
> +#define erofs_blknr(sb, addr)	((erofs_blk_t)((addr) >> (sb)->s_blocksize_bits))
>   #define erofs_blkoff(sb, addr)	((addr) & ((sb)->s_blocksize - 1))
>   #define erofs_pos(sb, blk)	((erofs_off_t)(blk) << (sb)->s_blocksize_bits)
>   #define erofs_iblks(i)	(round_up((i)->i_size, i_blocksize(i)) >> (i)->i_blkbits)