Re: [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 8/20/25 03:39, Qu Wenruo wrote:
>
>
> 在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
>> This test was written with 4k block size in mind and it fails with
>> 64k block size when tested with btrfs.
>> The test first does pre-allocation, then fills up the
>> filesystem. After that it tries to fragment and fill holes at offsets
>> of 4k(i.e, 1 fsblock) - which works fine with 4k block size, but with
>> 64k block size, the test tries to fragment and fill holes within
>> 1 fsblock(of size 64k). This results in overwrite of 64k fsblocks
>> and the write fails. The reason for this failure is that during
>> overwrite, there is no more space available for COW.
>> Fix this by changing the pwrite block size and offsets to 64k
>> so that the test never tries to punch holes or overwrite within 1 
>> fsblock
>> and the test becomes compatible with all block sizes.
>>
>> For non-COW filesystems/files, this test should work even if the
>> underlying filesytem block size > 64k.
>>
>> Reported-by: Disha Goel <disgoel@linux.ibm.com>
>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>
> Overall looks good to me.
>
> Although still a minor concern inlined below.
>
> [...]>
>>   # Fill the rest of the fs completely
>>   # Note, this will show ENOSPC errors in $seqres.full, that's ok.
>>   echo "Fill fs with 1M IOs; ENOSPC expected" >> $seqres.full
>>   dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M >>$seqres.full 2>&1
>> -echo "Fill fs with 4K IOs; ENOSPC expected" >> $seqres.full
>> -dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=4K >>$seqres.full 2>&1
>> +echo "Fill fs with 64K IOs; ENOSPC expected" >> $seqres.full
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=64K >>$seqres.full 2>&1
>
> Not sure if using 64K block size to fill the fs is the correct way.
>
> For example on a fs with 4K block size, but at end of filling there 
> are only 60K left.
>
> This will fail the filling as we can not reserve 64K data space anymore.
> But it's not 100% filling the data space either.
> This may not matter that much as in the preallocated filling stage, 
> every operation is still in 64K block size though.
>
> I'd prefer to keep the old 4K as block size (as it's the minimal 
> support one), or use the fs block size for filling.
> This will ensure we really use up all the data space.
>
> Thanks,
> Qu

Okay, makes sense. I will revert it to the older 4k size and send an 
updated version.

--NR

>
>>   _scratch_sync
>>   # Last effort, use O_SYNC
>> -echo "Fill fs with 4K DIOs; ENOSPC expected" >> $seqres.full
>> -dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K oflag=sync >>$seqres.full 
>> 2>&1
>> +echo "Fill fs with 64K DIOs; ENOSPC expected" >> $seqres.full
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=64K oflag=sync 
>> >>$seqres.full 2>&1
>>   # Save space usage info
>>   echo "Post-fill space:" >> $seqres.full
>>   df $SCRATCH_MNT >>$seqres.full 2>&1
>> @@ -63,7 +63,7 @@ df $SCRATCH_MNT >>$seqres.full 2>&1
>>   echo "Fill in prealloc space; fragment at offsets:" >> $seqres.full
>>   for i in `seq 1 2 1023`; do
>>       echo -n "$i " >> $seqres.full
>> -    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 
>> conv=notrunc \
>> +    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 
>> conv=notrunc \
>>           >>$seqres.full 2>/dev/null || _fail "failed to write to 
>> test file"
>>   done
>>   _scratch_sync
>> @@ -71,7 +71,7 @@ echo >> $seqres.full
>>   echo "Fill in prealloc space; fill holes at offsets:" >> $seqres.full
>>   for i in `seq 2 2 1023`; do
>>       echo -n "$i " >> $seqres.full
>> -    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 
>> conv=notrunc \
>> +    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 
>> conv=notrunc \
>>           >>$seqres.full 2>/dev/null || _fail "failed to fill test file"
>>   done
>>   _scratch_sync
>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore