Re: [PATCH] erofs: simplify readdir operation

[Hongzhen Luo]

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