Re: [PATCH v2] erofs: update ctx->pos for every emitted dirent

[hongnanLi]

on 2022/6/19 8:19, Chao Yu wrote:
> On 2022/6/9 11:40, Hongnan Li wrote:
> > erofs_readdir update ctx->pos after filling a batch of dentries
> > and it may cause dir/files duplication for NFS readdirplus which
> > depends on ctx->pos to fill dir correctly. So update ctx->pos for
> > every emitted dirent in erofs_fill_dentries to fix it.
> >
> > Fixes: 3e917cc305c6 ("erofs: make filesystem exportable")
> > Signed-off-by: Hongnan Li <hongnan.li@xxxxxxxxxxxxxxxxx>
> > ---
> >   fs/erofs/dir.c | 20 ++++++++++----------
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
> > index 18e59821c597..94ef5287237a 100644
> > --- a/fs/erofs/dir.c
> > +++ b/fs/erofs/dir.c
> > @@ -22,10 +22,9 @@ static void debug_one_dentry(unsigned char d_type, 
> > const char *de_name,
> >   }
> >   static int erofs_fill_dentries(struct inode *dir, struct dir_context 
> > *ctx,
> > -                   void *dentry_blk, unsigned int *ofs,
> > +                   void *dentry_blk, struct erofs_dirent *de,
> >                      unsigned int nameoff, unsigned int maxsize)
> >   {
> > -    struct erofs_dirent *de = dentry_blk + *ofs;
> >       const struct erofs_dirent *end = dentry_blk + nameoff;
> >       while (de < end) {
> > @@ -59,9 +58,8 @@ static int erofs_fill_dentries(struct inode *dir, 
> > struct dir_context *ctx,
> >               /* stopped by some reason */
> >               return 1;
> >           ++de;
> > -        *ofs += sizeof(struct erofs_dirent);
> > +        ctx->pos += sizeof(struct erofs_dirent);
> >       }
> > -    *ofs = maxsize;
> >       return 0;
> >   }
> > @@ -95,7 +93,7 @@ static int erofs_readdir(struct file *f, struct 
> > dir_context *ctx)
> >                     "invalid de[0].nameoff %u @ nid %llu",
> >                     nameoff, EROFS_I(dir)->nid);
> >               err = -EFSCORRUPTED;
> > -            goto skip_this;
> > +            break;
> >           }
> >           maxsize = min_t(unsigned int,
> > @@ -106,17 +104,19 @@ static int erofs_readdir(struct file *f, struct 
> > dir_context *ctx)
> >               initial = false;
> >               ofs = roundup(ofs, sizeof(struct erofs_dirent));
> > -            if (ofs >= nameoff)
> > +            if (ofs >= nameoff) {
> > +                ctx->pos = blknr_to_addr(i) + ofs;
> >                   goto skip_this;
> > +            }
> >           }
> > -        err = erofs_fill_dentries(dir, ctx, de, &ofs,
> > -                      nameoff, maxsize);
> > -skip_this:
> >           ctx->pos = blknr_to_addr(i) + ofs;
>
> Why updating ctx->pos before erofs_fill_dentries()?
>
> Thanks,

It’s to ensure the ctx->pos is correct and up to date in 
erofs_fill_dentries() so that we can update ctx->pos instead of ofs for 
every emitted dirent.

> > -
> > +        err = erofs_fill_dentries(dir, ctx, de, (void *)de + ofs,
> > +                      nameoff, maxsize);
> >           if (err)
> >               break;
> > +        ctx->pos = blknr_to_addr(i) + maxsize;
> > +skip_this:
> >           ++i;
> >           ofs = 0;
> >       }