Re: [PATCH] erofs: simplify readdir operation

[Hongzhen Luo <hongzhen@linux.alibaba.com>]

On 2024/8/1 19:31, Gao Xiang wrote:
>
>
> 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@linux.alibaba.com>
>> ---
>
> What's the difference from the previous version? why not marking
> it as v2?
>
> Thanks,
> Gao Xiang
>
The previous version was corrupted and couldn't apply the patch using 
`git am`. Sorry, I didn't write a changelog. I will provide a version 
with the changelog added...

Thanks,
Hongzhen Luo

>>   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)