Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <a15c3c93-3472-5bed-c8bb-4416bb809325@linux.alibaba.com>
Date:   Fri, 22 Apr 2022 11:10:28 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.6.1
Subject: Re: [PATCH v9 08/21] cachefiles: document on-demand read mode
Content-Language: en-US
To:     David Howells <dhowells@redhat.com>
Cc:     linux-cachefs@redhat.com, xiang@kernel.org, chao@kernel.org,
        linux-erofs@lists.ozlabs.org, torvalds@linux-foundation.org,
        gregkh@linuxfoundation.org, willy@infradead.org,
        linux-fsdevel@vger.kernel.org, joseph.qi@linux.alibaba.com,
        bo.liu@linux.alibaba.com, tao.peng@linux.alibaba.com,
        gerry@linux.alibaba.com, eguan@linux.alibaba.com,
        linux-kernel@vger.kernel.org, luodaowen.backend@bytedance.com,
        tianzichen@kuaishou.com, fannaihao@baidu.com,
        zhangjiachen.jaycee@bytedance.com
References: <20220415123614.54024-9-jefflexu@linux.alibaba.com>
 <20220415123614.54024-1-jefflexu@linux.alibaba.com>
 <1447053.1650552451@warthog.procyon.org.uk>
From:   JeffleXu <jefflexu@linux.alibaba.com>
In-Reply-To: <1447053.1650552451@warthog.procyon.org.uk>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi David, thanks for polishing the documents. It's a detailed and
meticulous review again. Really thanks for your time :) I will fix all
these in the next version.

On 4/21/22 10:47 PM, David Howells wrote:
> Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> 
>> +The essential difference between these two modes is that, in original mode,
>> +when a cache miss occurs, the netfs will fetch the data from the remote server
>> +and then write it to the cache file.  With on-demand read mode, however,
>> +fetching the data and writing it into the cache is delegated to a user daemon.
> 
> The starting sentence seems off.  How about:
> 
>   The essential difference between these two modes is seen when a cache miss
>   occurs: In the original mode, the netfs will fetch the data from the remote
>   server and then write it to the cache file; in on-demand read mode, fetching
>   data and writing it into the cache is delegated to a user daemon.

Okay, it sounds better.

>> the devnode ('/dev/cachefiles') to check if
>> +there's a pending request to be processed.  A POLLIN event will be returned
>> +when there's a pending request.
>> +
>> +The user daemon then reads the devnode to fetch a request and process it
>> +accordingly.
> 
> Reading the devnode doesn't process the request, so I think something like:
> 
> "... and process it accordingly" -> "... that it can then process."
> 
> or:
> 
> "... and process it accordingly" -> "... to process."

Yeah the original statement is indeed misleading.


>> Each cache file has a unique object_id, while it
>> +may have multiple anonymous fds. The user daemon may duplicate anonymous fds
>> +from the initial anonymous fd indicated by the @fd field through dup(). Thus
>> +each object_id can be mapped to multiple anonymous fds, while the usr daemon
>> +itself needs to maintain the mapping.
>> +
>> +With the given anonymous fd, the user daemon can fetch data and write it to the
>> +cache file in the background, even when kernel has not triggered a cache miss
>> +yet.
>> +
>> +The user daemon should complete the READ request
> 
> READ request -> OPEN request?

Good catch. Will be fixed.


>> in the READ request.  The ioctl is of the form::
>> +
>> +	ioctl(fd, CACHEFILES_IOC_CREAD, msg_id);
>> +
>> +	* ``fd`` is one of the anonymous fds associated with the given object_id
>> +	  in the READ request.
> 
> the given object_id in the READ request -> object_id
> 
>> +
>> +	* ``msg_id`` must match the msg_id field of the previous READ request.
> 
> By "previous READ request" is this referring to something different to "the
> READ request" you mentioned against the fd parameter?

Actually it is referring to the same thing (the same READ request). I
will change the statement simply to:

``msg_id`` must match the msg_id field of the READ request.

-- 
Thanks,
Jeffle