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

[hongnanLi <hongnan.li@linux.alibaba.com>]

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.

> 
>> -
>> +        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;
>>       }