[PATCH] erofs: simplify readdir operation

[Hongbo Li]

On 2024/8/1 19:35, Hongzhen Luo wrote:
> 
> 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 at 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...
May be he also means the subject should be marked [PATCH v..]

Thanks,
Hongbo

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