Re: [PATCH] loop: change to punch hole if zero range is not supported

[Shijie Luo <luoshijie1@huawei.com>]

On 2020/7/3 0:57, Darrick J. Wong wrote:
> On Wed, Jul 01, 2020 at 09:54:01PM -0400, Shijie Luo wrote:
>> We found a problem when excuting these operations.
>>
>> $ cd /tmp
>> $ qemu-img create -f raw test.img 10G
>> $ mknod -m 0660 /dev/loop0 b 7 0
>> $ losetup /dev/loop0 test.img
>> $ mkfs /dev/loop0
>>
>> Here is the error message.
>>
>> [  142.364823] blk_update_request: operation not supported error,
>>   dev loop0, sector 20971392 op 0x9:(WRITE_ZEROES) flags 0x1000800
>>   phys_seg 0 prio class 0
>> [  142.371823] blk_update_request: operation not supported error,
>>   dev loop0, sector 5144 op 0x9:(WRITE_ZEROES) flags 0x1000800
>>   phys_seg 0 prio class 0
>>
>> The problem is that not all filesystem support zero range (eg, tmpfs), if
>>   filesystem doesn 't support zero range, change to punch hole to fix it.
> NAK, ZERO_RANGE requires[1] that "Within the specified range, blocks are
> preallocated for the regions that span the holes in the file."
> PUNCH_HOLE has the opposite effect, and does not qualify as a
> replacement.
>
> --D
>
> [1] https://man7.org/linux/man-pages/man2/fallocate.2.html

Thanks for your reply, I know the difference between ZERO_RANGE and 
PUNCH_HOLE,

but it's hard to provide ZERO_RANGE operation for all filesystem. Maybe 
the formatting

tools should not trigger a NOUNMAP request in this situation.

>> Fixes: efcfec579f613 ("loop: fix no-unmap write-zeroes request behavior")
>> Signed-off-by: Shijie Luo <luoshijie1@huawei.com>
>> ---
>>   drivers/block/loop.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>> index c33bbbfd1bd9..504e658adcaf 100644
>> --- a/drivers/block/loop.c
>> +++ b/drivers/block/loop.c
>> @@ -450,6 +450,13 @@ static int lo_fallocate(struct loop_device *lo, struct request *rq, loff_t pos,
>>   	}
>>   
>>   	ret = file->f_op->fallocate(file, mode, pos, blk_rq_bytes(rq));
>> +
>> +	if ((ret == -EOPNOTSUPP) && (mode & FALLOC_FL_ZERO_RANGE)) {
>> +		mode &= ~FALLOC_FL_ZERO_RANGE;
>> +		mode |= FALLOC_FL_PUNCH_HOLE;
>> +		ret = file->f_op->fallocate(file, mode, pos, blk_rq_bytes(rq));
>> +	}
>> +
>>   	if (unlikely(ret && ret != -EINVAL && ret != -EOPNOTSUPP))
>>   		ret = -EIO;
>>    out:
>> -- 
>> 2.19.1
>>
> .
>