Message-ID: <d13251fc-dfd0-4634-9d3f-43a8afe47752@linux.alibaba.com>
Date: Thu, 8 Feb 2024 08:04:42 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/5] fuse: Create helper function if DIO write needs
 exclusive lock
Content-Language: en-US
To: Bernd Schubert <bernd.schubert@fastmail.fm>,
 Bernd Schubert <bschubert@ddn.com>, miklos@szeredi.hu
Cc: linux-fsdevel@vger.kernel.org, dsingh@ddn.com,
 Amir Goldstein <amir73il@gmail.com>
References: <20240131230827.207552-1-bschubert@ddn.com>
 <20240131230827.207552-3-bschubert@ddn.com>
 <2d0d6581-14de-46c4-a664-f6e193ab2518@linux.alibaba.com>
 <b9308f8b-cda3-486f-be23-6e84cc0c8b6d@fastmail.fm>
 <f365c731-9cc5-4340-9c1e-f6f5ab422140@linux.alibaba.com>
 <072d2ac6-6281-404c-8897-39748c763b39@fastmail.fm>
From: Jingbo Xu <jefflexu@linux.alibaba.com>
In-Reply-To: <072d2ac6-6281-404c-8897-39748c763b39@fastmail.fm>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 2/7/24 10:13 PM, Bernd Schubert wrote:
> 
> 
> On 2/7/24 14:44, Jingbo Xu wrote:
>>
>>
>> On 2/7/24 9:38 PM, Bernd Schubert wrote:
>>>
>>>
>>> On 2/6/24 10:20, Jingbo Xu wrote:
>>>>
>>>>
>>>> On 2/1/24 7:08 AM, Bernd Schubert wrote:
>>>>> @@ -1591,10 +1616,10 @@ static ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
>>>>>  	else {
>>>>>  		inode_lock_shared(inode);
>>>>>  
>>>>> -		/* A race with truncate might have come up as the decision for
>>>>> -		 * the lock type was done without holding the lock, check again.
>>>>> +		/*
>>>>> +		 * Previous check was without any lock and might have raced.
>>>>>  		 */
>>
>>
>>>>> -		if (fuse_direct_write_extending_i_size(iocb, from)) {
>>>>> +		if (fuse_dio_wr_exclusive_lock(iocb, from)) {
>>>> 			^
>>
>> I mean, in patch 2/5
>>
>>> -		if (fuse_direct_write_extending_i_size(iocb, from)) {
>>> +		if (fuse_io_past_eof(iocb, from)) {
>>
>> is better, otherwise it's not an equivalent change.
> 
> Ah thanks, good catch! Now I see what you mean. Yeah, we can switch to
> fuse_io_past_eof() here. And yeah, 3/5 changes it back.
> Fortunately there is actually not much harm, as
> fuse_dio_wr_exclusive_lock also calls into fuse_io_past_eof.
> 

Yeah fortunately we needn't retest it as patch 3/5 changes it back.

The whole series is good.  The comment is just from per-patch basis.

Thanks,
Jingbo