Messages in this thread | | | Date | Fri, 24 Jun 2022 17:31:40 +0800 | Subject | Re: [PATCH v2] erofs: update ctx->pos for every emitted dirent | From | hongnanLi <> |
| |
Hi Gao Xiang,
on 2022/6/20 下午8:19, Gao Xiang wrote: > Hi Hongnan, > > On Mon, Jun 20, 2022 at 05:37:07PM +0800, hongnanLi wrote: >> 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@linux.alibaba.com> >>>> --- >>>> 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. >> > > How about this, since blknr_to_addr(i) + maxsize should be the start of > the next dir block. > > if (initial) { > ofs = roundup(ofs, sizeof(struct erofs_dirent)); > ctx->pos = blknr_to_addr(i) + ofs; > if (ofs >= nameoff) > goto skip_this; > } > err = erofs_fill_dentries(dir, ctx, de, (void *)de + ofs, > nameoff, maxsize); > if (err) > break; > ctx->pos = blknr_to_addr(i) + maxsize; >
Thanks for your suggestion. It looks good and works well in my test. I will send PATCH v3 later if everything else is okay.
Thanks, Hongnan Li
| |