Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <70fe93a3-7af5-b563-dcb7-3f7be81348ed@linux.alibaba.com>
Date:   Mon, 20 Jun 2022 17:37:07 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.10.0
From:   hongnanLi <hongnan.li@linux.alibaba.com>
Subject: Re: [PATCH v2] erofs: update ctx->pos for every emitted dirent
To:     Chao Yu <chao@kernel.org>, linux-erofs@lists.ozlabs.org,
        xiang@kernel.org
Cc:     linux-kernel@vger.kernel.org
References: <20220527072536.68516-1-hongnan.li@linux.alibaba.com>
 <20220609034006.76649-1-hongnan.li@linux.alibaba.com>
 <0c139517-e976-5017-8e7a-d34c38f0f6bb@kernel.org>
Content-Language: en-US
In-Reply-To: <0c139517-e976-5017-8e7a-d34c38f0f6bb@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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