lkml.org 
[lkml]   [2022]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] erofs: update ctx->pos for every emitted dirent
From
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

\
 
 \ /
  Last update: 2022-06-24 11:32    [W:0.057 / U:1.768 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site