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: <65f424ca-d7cd-f53b-cefc-684ec0393bce@bytedance.com>
Date:   Fri, 14 Apr 2023 23:12:55 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.9.1
Subject: Re: Re: [PATCH V5 5/5] cachefiles: add restore command to recover
 inflight ondemand read requests
To:     David Howells <dhowells@redhat.com>
Cc:     linux-cachefs@redhat.com, linux-erofs@lists.ozlabs.org,
        linux-kernel@vger.kernel.org, jefflexu@linux.alibaba.com,
        hsiangkao@linux.alibaba.com, yinxin.x@bytedance.com,
        zhujia.zj@bytedance.com
References: <20230329140155.53272-6-zhujia.zj@bytedance.com>
 <20230329140155.53272-1-zhujia.zj@bytedance.com>
 <1250439.1681480404@warthog.procyon.org.uk>
From:   Jia Zhu <zhujia.zj@bytedance.com>
In-Reply-To: <1250439.1681480404@warthog.procyon.org.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2023/4/14 21:53, David Howells 写道:
> Jia Zhu <zhujia.zj@bytedance.com> wrote:
> 
>> +int cachefiles_ondemand_restore(struct cachefiles_cache *cache, char *args)
>> +{
>> +	struct cachefiles_req *req;
>> +
>> +	XA_STATE(xas, &cache->reqs, 0);
>> +
>> +	if (!test_bit(CACHEFILES_ONDEMAND_MODE, &cache->flags))
>> +		return -EOPNOTSUPP;
>> +
>> +	/*
>> +	 * Reset the requests to CACHEFILES_REQ_NEW state, so that the
>> +	 * requests have been processed halfway before the crash of the
>> +	 * user daemon could be reprocessed after the recovery.
>> +	 */
>> +	xas_lock(&xas);
>> +	xas_for_each(&xas, req, ULONG_MAX)
>> +		xas_set_mark(&xas, CACHEFILES_REQ_NEW);
>> +	xas_unlock(&xas);
>> +
>> +	wake_up_all(&cache->daemon_pollwq);
>> +	return 0;
>> +}
> 
> Should there be a check to see if this is needed?
> 
> David

I've considered whether to add a check here, since the user could invoke
'restore' through ioctl at any time.

If 'restore' is called without user daemon crashing. Then the req being
processed by the user daemon will be reset to CACHEFILES_REQ_NEW and
could be re-read and processed by the user daemon.

For OPEN req:  The user daemon will ignore this repeated req since the
related anonymous fd is the same.

For READ req: The user daemon will read the same part of the data again
and write it to fscache (this will not cause any error)

For CLOSE req: Actually the user daemon will close this anonymous fd,
but fortunately [PATCH 3/5] could handle this case by reopening it
automatically.

Thus "restore" could be called at any time without error, checks
might introduce extra race conditions.

Jia
>