[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2e708294-a2df-e775-4ea8-5b1fd0aa4544@linux.alibaba.com>
Date: Fri, 24 Jun 2022 17:31:40 +0800
From: hongnanLi <hongnan.li@...ux.alibaba.com>
To: Gao Xiang <hsiangkao@...ux.alibaba.com>
Cc: Chao Yu <chao@...nel.org>, linux-erofs@...ts.ozlabs.org,
xiang@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] erofs: update ctx->pos for every emitted dirent
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@...ux.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
Powered by blists - more mailing lists