diff mbox series

f2fs: fix to tag FIEMAP_EXTENT_DELALLOC in fiemap() for delay allocated extent

Message ID 20230405144359.930253-1-chao@kernel.org
State New
Headers
Series f2fs: fix to tag FIEMAP_EXTENT_DELALLOC in fiemap() for delay allocated extent |

Commit Message

Chao Yu April 5, 2023, 2:43 p.m. UTC 
  xfstest generic/614 fails to run due below reason:

generic/614 1s ... [not run] test requires delayed allocation buffered writes

The root cause is f2fs tags wrong fiemap flag for delay allocated
extent.

Quoted from fiemap.h:
FIEMAP_EXTENT_UNKNOWN		0x00000002 /* Data location unknown. */
FIEMAP_EXTENT_DELALLOC		0x00000004 /* Location still pending.
						    * Sets EXTENT_UNKNOWN. */
FIEMAP_EXTENT_UNWRITTEN		0x00000800 /* Space allocated, but
						    * no data (i.e. zero). */

FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
been written any data, which status f2fs is not supported now, for all
NEW_ADDR block addresses, it means delay allocated blocks, so let's
tag FIEMAP_EXTENT_DELALLOC instead.

Testcase:
xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
filefrag -v /mnt/f2fs/file

Output:
- Before
Filesystem type is: f2f52010
Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
/mnt/f2fs/file: 1 extent found

After:
Filesystem type is: f2f52010
File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
/mnt/f2fs/file: 1 extent found

Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/data.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Jaegeuk Kim April 7, 2023, 8:59 p.m. UTC | #1 
This breaks generic/009?

On 04/05, Chao Yu wrote:
> xfstest generic/614 fails to run due below reason:
> 
> generic/614 1s ... [not run] test requires delayed allocation buffered writes
> 
> The root cause is f2fs tags wrong fiemap flag for delay allocated
> extent.
> 
> Quoted from fiemap.h:
> FIEMAP_EXTENT_UNKNOWN		0x00000002 /* Data location unknown. */
> FIEMAP_EXTENT_DELALLOC		0x00000004 /* Location still pending.
> 						    * Sets EXTENT_UNKNOWN. */
> FIEMAP_EXTENT_UNWRITTEN		0x00000800 /* Space allocated, but
> 						    * no data (i.e. zero). */
> 
> FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
> been written any data, which status f2fs is not supported now, for all
> NEW_ADDR block addresses, it means delay allocated blocks, so let's
> tag FIEMAP_EXTENT_DELALLOC instead.
> 
> Testcase:
> xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
> filefrag -v /mnt/f2fs/file
> 
> Output:
> - Before
> Filesystem type is: f2f52010
> Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>  ext:     logical_offset:        physical_offset: length:   expected: flags:
>    0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
> /mnt/f2fs/file: 1 extent found
> 
> After:
> Filesystem type is: f2f52010
> File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>  ext:     logical_offset:        physical_offset: length:   expected: flags:
>    0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
> /mnt/f2fs/file: 1 extent found
> 
> Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/data.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 359de650772e..3afc9764743e 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  	}
>  
>  	if (size) {
> -		flags |= FIEMAP_EXTENT_MERGED;
> +		if (flags & FIEMAP_EXTENT_DELALLOC)
> +			phys = 0;
> +		else
> +			flags |= FIEMAP_EXTENT_MERGED;
>  		if (IS_ENCRYPTED(inode))
>  			flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
>  
> @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  				size += blks_to_bytes(inode, 1);
>  			}
>  		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
> -			flags = FIEMAP_EXTENT_UNWRITTEN;
> +			flags = FIEMAP_EXTENT_DELALLOC;
>  		}
>  
>  		start_blk += bytes_to_blks(inode, size);
> -- 
> 2.36.1
Chao Yu April 8, 2023, 2:57 a.m. UTC | #2 
On 2023/4/8 4:59, Jaegeuk Kim wrote:
> This breaks generic/009?

I guess it is as expected?

Please check description of fiemap ioctl manual from [1]:

FIEMAP_EXTENT_UNKNOWN
The location of this extent is currently unknown. This may
indicate the data is stored on an inaccessible volume or that
no storage has been allocated for the file yet.

FIEMAP_EXTENT_DELALLOC
This will also set FIEMAP_EXTENT_UNKNOWN.

Delayed allocation - while there is data for this extent, its
physical location has not been allocated yet.

FIEMAP_EXTENT_UNWRITTEN
Unwritten extent - the extent is allocated but its data has not
been initialized. This indicates the extent’s data will be all
zero if read through the filesystem but the contents are undefined
if read directly from the device.

[1] https://www.kernel.org/doc/html/latest/filesystems/fiemap.html

According to its description, f2fs only support
FIEMAP_EXTENT_{UNKNOWN, DELALLOC}, but not support
FIEMAP_EXTENT_UNWRITTEN.

So 009, 092, 094 .. which expects unwritten status from extent will
fail.

How about disabling those testcase?

Thanks,

> 
> On 04/05, Chao Yu wrote:
>> xfstest generic/614 fails to run due below reason:
>>
>> generic/614 1s ... [not run] test requires delayed allocation buffered writes
>>
>> The root cause is f2fs tags wrong fiemap flag for delay allocated
>> extent.
>>
>> Quoted from fiemap.h:
>> FIEMAP_EXTENT_UNKNOWN		0x00000002 /* Data location unknown. */
>> FIEMAP_EXTENT_DELALLOC		0x00000004 /* Location still pending.
>> 						    * Sets EXTENT_UNKNOWN. */
>> FIEMAP_EXTENT_UNWRITTEN		0x00000800 /* Space allocated, but
>> 						    * no data (i.e. zero). */
>>
>> FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
>> been written any data, which status f2fs is not supported now, for all
>> NEW_ADDR block addresses, it means delay allocated blocks, so let's
>> tag FIEMAP_EXTENT_DELALLOC instead.
>>
>> Testcase:
>> xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
>> filefrag -v /mnt/f2fs/file
>>
>> Output:
>> - Before
>> Filesystem type is: f2f52010
>> Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>   ext:     logical_offset:        physical_offset: length:   expected: flags:
>>     0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
>> /mnt/f2fs/file: 1 extent found
>>
>> After:
>> Filesystem type is: f2f52010
>> File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>   ext:     logical_offset:        physical_offset: length:   expected: flags:
>>     0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
>> /mnt/f2fs/file: 1 extent found
>>
>> Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/data.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 359de650772e..3afc9764743e 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>   	}
>>   
>>   	if (size) {
>> -		flags |= FIEMAP_EXTENT_MERGED;
>> +		if (flags & FIEMAP_EXTENT_DELALLOC)
>> +			phys = 0;
>> +		else
>> +			flags |= FIEMAP_EXTENT_MERGED;
>>   		if (IS_ENCRYPTED(inode))
>>   			flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
>>   
>> @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>   				size += blks_to_bytes(inode, 1);
>>   			}
>>   		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
>> -			flags = FIEMAP_EXTENT_UNWRITTEN;
>> +			flags = FIEMAP_EXTENT_DELALLOC;
>>   		}
>>   
>>   		start_blk += bytes_to_blks(inode, size);
>> -- 
>> 2.36.1
Jaegeuk Kim April 10, 2023, 5:57 p.m. UTC | #3 
On 04/08, Chao Yu wrote:
> On 2023/4/8 4:59, Jaegeuk Kim wrote:
> > This breaks generic/009?
> 
> I guess it is as expected?
> 
> Please check description of fiemap ioctl manual from [1]:
> 
> FIEMAP_EXTENT_UNKNOWN
> The location of this extent is currently unknown. This may
> indicate the data is stored on an inaccessible volume or that
> no storage has been allocated for the file yet.
> 
> FIEMAP_EXTENT_DELALLOC
> This will also set FIEMAP_EXTENT_UNKNOWN.
> 
> Delayed allocation - while there is data for this extent, its
> physical location has not been allocated yet.
> 
> FIEMAP_EXTENT_UNWRITTEN
> Unwritten extent - the extent is allocated but its data has not
> been initialized. This indicates the extent’s data will be all
> zero if read through the filesystem but the contents are undefined
> if read directly from the device.
> 
> [1] https://www.kernel.org/doc/html/latest/filesystems/fiemap.html
> 
> According to its description, f2fs only support
> FIEMAP_EXTENT_{UNKNOWN, DELALLOC}, but not support
> FIEMAP_EXTENT_UNWRITTEN.

No, I don't think so.

> 
> So 009, 092, 094 .. which expects unwritten status from extent will
> fail.
> 
> How about disabling those testcase?
> 
> Thanks,
> 
> > 
> > On 04/05, Chao Yu wrote:
> > > xfstest generic/614 fails to run due below reason:
> > > 
> > > generic/614 1s ... [not run] test requires delayed allocation buffered writes
> > > 
> > > The root cause is f2fs tags wrong fiemap flag for delay allocated
> > > extent.
> > > 
> > > Quoted from fiemap.h:
> > > FIEMAP_EXTENT_UNKNOWN		0x00000002 /* Data location unknown. */
> > > FIEMAP_EXTENT_DELALLOC		0x00000004 /* Location still pending.
> > > 						    * Sets EXTENT_UNKNOWN. */
> > > FIEMAP_EXTENT_UNWRITTEN		0x00000800 /* Space allocated, but
> > > 						    * no data (i.e. zero). */
> > > 
> > > FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
> > > been written any data, which status f2fs is not supported now, for all
> > > NEW_ADDR block addresses, it means delay allocated blocks, so let's
> > > tag FIEMAP_EXTENT_DELALLOC instead.
> > > 
> > > Testcase:
> > > xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
> > > filefrag -v /mnt/f2fs/file
> > > 
> > > Output:
> > > - Before
> > > Filesystem type is: f2f52010
> > > Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
> > >   ext:     logical_offset:        physical_offset: length:   expected: flags:
> > >     0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
> > > /mnt/f2fs/file: 1 extent found
> > > 
> > > After:
> > > Filesystem type is: f2f52010
> > > File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
> > >   ext:     logical_offset:        physical_offset: length:   expected: flags:
> > >     0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
> > > /mnt/f2fs/file: 1 extent found
> > > 
> > > Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > ---
> > >   fs/f2fs/data.c | 7 +++++--
> > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index 359de650772e..3afc9764743e 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > >   	}
> > >   	if (size) {
> > > -		flags |= FIEMAP_EXTENT_MERGED;
> > > +		if (flags & FIEMAP_EXTENT_DELALLOC)
> > > +			phys = 0;
> > > +		else
> > > +			flags |= FIEMAP_EXTENT_MERGED;
> > >   		if (IS_ENCRYPTED(inode))
> > >   			flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
> > > @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > >   				size += blks_to_bytes(inode, 1);
> > >   			}
> > >   		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
> > > -			flags = FIEMAP_EXTENT_UNWRITTEN;
> > > +			flags = FIEMAP_EXTENT_DELALLOC;
> > >   		}
> > >   		start_blk += bytes_to_blks(inode, size);
> > > -- 
> > > 2.36.1
Chao Yu April 11, 2023, 8:14 a.m. UTC | #4 
On 2023/4/11 1:57, Jaegeuk Kim wrote:
> On 04/08, Chao Yu wrote:
>> On 2023/4/8 4:59, Jaegeuk Kim wrote:
>>> This breaks generic/009?
>>
>> I guess it is as expected?
>>
>> Please check description of fiemap ioctl manual from [1]:
>>
>> FIEMAP_EXTENT_UNKNOWN
>> The location of this extent is currently unknown. This may
>> indicate the data is stored on an inaccessible volume or that
>> no storage has been allocated for the file yet.
>>
>> FIEMAP_EXTENT_DELALLOC
>> This will also set FIEMAP_EXTENT_UNKNOWN.
>>
>> Delayed allocation - while there is data for this extent, its
>> physical location has not been allocated yet.
>>
>> FIEMAP_EXTENT_UNWRITTEN
>> Unwritten extent - the extent is allocated but its data has not
>> been initialized. This indicates the extent’s data will be all
>> zero if read through the filesystem but the contents are undefined
>> if read directly from the device.
>>
>> [1] https://www.kernel.org/doc/html/latest/filesystems/fiemap.html
>>
>> According to its description, f2fs only support
>> FIEMAP_EXTENT_{UNKNOWN, DELALLOC}, but not support
>> FIEMAP_EXTENT_UNWRITTEN.
> 
> No, I don't think so.

Jaegeuk,

Could you please check the detailed description of FIEMAP_EXTENT_UNWRITTEN?
The flag indicates two conditions:
1. on-disk blkaddrs were allocated for extent, and the extent is tagged as
unwritten status.
2. data readed on those blocks will be all zero.

So, let's check f2fs' status:
- fallocate only reserve valid block count and set NEW_ADDR in dnode, so
it does not match condition 1)
- pin & fallocate preallocates blkaddrs and set blkaddrs in dnode, but
content on those blkaddrs may contain zero or random data, so it does not
match  condition 2)

Christoph describes this issue in below patch as well, you can check it.
da8c7fecc9c7 ("f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC")

Am I missing something?

Thanks,

> 
>>
>> So 009, 092, 094 .. which expects unwritten status from extent will
>> fail.
>>
>> How about disabling those testcase?
>>
>> Thanks,
>>
>>>
>>> On 04/05, Chao Yu wrote:
>>>> xfstest generic/614 fails to run due below reason:
>>>>
>>>> generic/614 1s ... [not run] test requires delayed allocation buffered writes
>>>>
>>>> The root cause is f2fs tags wrong fiemap flag for delay allocated
>>>> extent.
>>>>
>>>> Quoted from fiemap.h:
>>>> FIEMAP_EXTENT_UNKNOWN		0x00000002 /* Data location unknown. */
>>>> FIEMAP_EXTENT_DELALLOC		0x00000004 /* Location still pending.
>>>> 						    * Sets EXTENT_UNKNOWN. */
>>>> FIEMAP_EXTENT_UNWRITTEN		0x00000800 /* Space allocated, but
>>>> 						    * no data (i.e. zero). */
>>>>
>>>> FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
>>>> been written any data, which status f2fs is not supported now, for all
>>>> NEW_ADDR block addresses, it means delay allocated blocks, so let's
>>>> tag FIEMAP_EXTENT_DELALLOC instead.
>>>>
>>>> Testcase:
>>>> xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
>>>> filefrag -v /mnt/f2fs/file
>>>>
>>>> Output:
>>>> - Before
>>>> Filesystem type is: f2f52010
>>>> Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>    ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>      0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
>>>> /mnt/f2fs/file: 1 extent found
>>>>
>>>> After:
>>>> Filesystem type is: f2f52010
>>>> File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>    ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>      0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
>>>> /mnt/f2fs/file: 1 extent found
>>>>
>>>> Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>>    fs/f2fs/data.c | 7 +++++--
>>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index 359de650772e..3afc9764743e 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>    	}
>>>>    	if (size) {
>>>> -		flags |= FIEMAP_EXTENT_MERGED;
>>>> +		if (flags & FIEMAP_EXTENT_DELALLOC)
>>>> +			phys = 0;
>>>> +		else
>>>> +			flags |= FIEMAP_EXTENT_MERGED;
>>>>    		if (IS_ENCRYPTED(inode))
>>>>    			flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
>>>> @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>    				size += blks_to_bytes(inode, 1);
>>>>    			}
>>>>    		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
>>>> -			flags = FIEMAP_EXTENT_UNWRITTEN;
>>>> +			flags = FIEMAP_EXTENT_DELALLOC;
>>>>    		}
>>>>    		start_blk += bytes_to_blks(inode, size);
>>>> -- 
>>>> 2.36.1
Chao Yu April 11, 2023, 8:36 a.m. UTC | #5 
On 2023/4/11 16:14, Chao Yu wrote:
> On 2023/4/11 1:57, Jaegeuk Kim wrote:
>> On 04/08, Chao Yu wrote:
>>> On 2023/4/8 4:59, Jaegeuk Kim wrote:
>>>> This breaks generic/009?
>>>
>>> I guess it is as expected?
>>>
>>> Please check description of fiemap ioctl manual from [1]:
>>>
>>> FIEMAP_EXTENT_UNKNOWN
>>> The location of this extent is currently unknown. This may
>>> indicate the data is stored on an inaccessible volume or that
>>> no storage has been allocated for the file yet.
>>>
>>> FIEMAP_EXTENT_DELALLOC
>>> This will also set FIEMAP_EXTENT_UNKNOWN.
>>>
>>> Delayed allocation - while there is data for this extent, its
>>> physical location has not been allocated yet.
>>>
>>> FIEMAP_EXTENT_UNWRITTEN
>>> Unwritten extent - the extent is allocated but its data has not
>>> been initialized. This indicates the extent’s data will be all
>>> zero if read through the filesystem but the contents are undefined
>>> if read directly from the device.
>>>
>>> [1] https://www.kernel.org/doc/html/latest/filesystems/fiemap.html
>>>
>>> According to its description, f2fs only support
>>> FIEMAP_EXTENT_{UNKNOWN, DELALLOC}, but not support
>>> FIEMAP_EXTENT_UNWRITTEN.
>>
>> No, I don't think so.
> 
> Jaegeuk,
> 
> Could you please check the detailed description of FIEMAP_EXTENT_UNWRITTEN?
> The flag indicates two conditions:
> 1. on-disk blkaddrs were allocated for extent, and the extent is tagged as
> unwritten status.
> 2. data readed on those blocks will be all zero.

Sorry, I mean:

1. on-disk blkaddrs were allocated for extent;
2. extent is tagged as unwritten status, data readed on those blocks will be
all zero.

Thanks,

> 
> So, let's check f2fs' status:
> - fallocate only reserve valid block count and set NEW_ADDR in dnode, so
> it does not match condition 1)
> - pin & fallocate preallocates blkaddrs and set blkaddrs in dnode, but
> content on those blkaddrs may contain zero or random data, so it does not
> match  condition 2)
> 
> Christoph describes this issue in below patch as well, you can check it.
> da8c7fecc9c7 ("f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC")
> 
> Am I missing something?
> 
> Thanks,
> 
>>
>>>
>>> So 009, 092, 094 .. which expects unwritten status from extent will
>>> fail.
>>>
>>> How about disabling those testcase?
>>>
>>> Thanks,
>>>
>>>>
>>>> On 04/05, Chao Yu wrote:
>>>>> xfstest generic/614 fails to run due below reason:
>>>>>
>>>>> generic/614 1s ... [not run] test requires delayed allocation buffered writes
>>>>>
>>>>> The root cause is f2fs tags wrong fiemap flag for delay allocated
>>>>> extent.
>>>>>
>>>>> Quoted from fiemap.h:
>>>>> FIEMAP_EXTENT_UNKNOWN        0x00000002 /* Data location unknown. */
>>>>> FIEMAP_EXTENT_DELALLOC        0x00000004 /* Location still pending.
>>>>>                             * Sets EXTENT_UNKNOWN. */
>>>>> FIEMAP_EXTENT_UNWRITTEN        0x00000800 /* Space allocated, but
>>>>>                             * no data (i.e. zero). */
>>>>>
>>>>> FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
>>>>> been written any data, which status f2fs is not supported now, for all
>>>>> NEW_ADDR block addresses, it means delay allocated blocks, so let's
>>>>> tag FIEMAP_EXTENT_DELALLOC instead.
>>>>>
>>>>> Testcase:
>>>>> xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
>>>>> filefrag -v /mnt/f2fs/file
>>>>>
>>>>> Output:
>>>>> - Before
>>>>> Filesystem type is: f2f52010
>>>>> Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>>    ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>>      0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
>>>>> /mnt/f2fs/file: 1 extent found
>>>>>
>>>>> After:
>>>>> Filesystem type is: f2f52010
>>>>> File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>>    ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>>      0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
>>>>> /mnt/f2fs/file: 1 extent found
>>>>>
>>>>> Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>> ---
>>>>>    fs/f2fs/data.c | 7 +++++--
>>>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>> index 359de650772e..3afc9764743e 100644
>>>>> --- a/fs/f2fs/data.c
>>>>> +++ b/fs/f2fs/data.c
>>>>> @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>>        }
>>>>>        if (size) {
>>>>> -        flags |= FIEMAP_EXTENT_MERGED;
>>>>> +        if (flags & FIEMAP_EXTENT_DELALLOC)
>>>>> +            phys = 0;
>>>>> +        else
>>>>> +            flags |= FIEMAP_EXTENT_MERGED;
>>>>>            if (IS_ENCRYPTED(inode))
>>>>>                flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
>>>>> @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>>                    size += blks_to_bytes(inode, 1);
>>>>>                }
>>>>>            } else if (map.m_flags & F2FS_MAP_DELALLOC) {
>>>>> -            flags = FIEMAP_EXTENT_UNWRITTEN;
>>>>> +            flags = FIEMAP_EXTENT_DELALLOC;
>>>>>            }
>>>>>            start_blk += bytes_to_blks(inode, size);
>>>>> -- 
>>>>> 2.36.1
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Jaegeuk Kim April 11, 2023, 5:08 p.m. UTC | #6 
On 04/11, Chao Yu wrote:
> On 2023/4/11 16:14, Chao Yu wrote:
> > On 2023/4/11 1:57, Jaegeuk Kim wrote:
> > > On 04/08, Chao Yu wrote:
> > > > On 2023/4/8 4:59, Jaegeuk Kim wrote:
> > > > > This breaks generic/009?
> > > > 
> > > > I guess it is as expected?
> > > > 
> > > > Please check description of fiemap ioctl manual from [1]:
> > > > 
> > > > FIEMAP_EXTENT_UNKNOWN
> > > > The location of this extent is currently unknown. This may
> > > > indicate the data is stored on an inaccessible volume or that
> > > > no storage has been allocated for the file yet.
> > > > 
> > > > FIEMAP_EXTENT_DELALLOC
> > > > This will also set FIEMAP_EXTENT_UNKNOWN.
> > > > 
> > > > Delayed allocation - while there is data for this extent, its
> > > > physical location has not been allocated yet.
> > > > 
> > > > FIEMAP_EXTENT_UNWRITTEN
> > > > Unwritten extent - the extent is allocated but its data has not
> > > > been initialized. This indicates the extent’s data will be all
> > > > zero if read through the filesystem but the contents are undefined
> > > > if read directly from the device.
> > > > 
> > > > [1] https://www.kernel.org/doc/html/latest/filesystems/fiemap.html
> > > > 
> > > > According to its description, f2fs only support
> > > > FIEMAP_EXTENT_{UNKNOWN, DELALLOC}, but not support
> > > > FIEMAP_EXTENT_UNWRITTEN.
> > > 
> > > No, I don't think so.
> > 
> > Jaegeuk,
> > 
> > Could you please check the detailed description of FIEMAP_EXTENT_UNWRITTEN?
> > The flag indicates two conditions:
> > 1. on-disk blkaddrs were allocated for extent, and the extent is tagged as
> > unwritten status.
> > 2. data readed on those blocks will be all zero.
> 
> Sorry, I mean:
> 
> 1. on-disk blkaddrs were allocated for extent;
> 2. extent is tagged as unwritten status, data readed on those blocks will be
> all zero.

I was thinking fallocate/pin cases to give zero data. But, we may need to check
the space discarded securely or disk support?

> 
> Thanks,
> 
> > 
> > So, let's check f2fs' status:
> > - fallocate only reserve valid block count and set NEW_ADDR in dnode, so
> > it does not match condition 1)
> > - pin & fallocate preallocates blkaddrs and set blkaddrs in dnode, but
> > content on those blkaddrs may contain zero or random data, so it does not
> > match  condition 2)
> > 
> > Christoph describes this issue in below patch as well, you can check it.
> > da8c7fecc9c7 ("f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC")
> > 
> > Am I missing something?
> > 
> > Thanks,
> > 
> > > 
> > > > 
> > > > So 009, 092, 094 .. which expects unwritten status from extent will
> > > > fail.
> > > > 
> > > > How about disabling those testcase?
> > > > 
> > > > Thanks,
> > > > 
> > > > > 
> > > > > On 04/05, Chao Yu wrote:
> > > > > > xfstest generic/614 fails to run due below reason:
> > > > > > 
> > > > > > generic/614 1s ... [not run] test requires delayed allocation buffered writes
> > > > > > 
> > > > > > The root cause is f2fs tags wrong fiemap flag for delay allocated
> > > > > > extent.
> > > > > > 
> > > > > > Quoted from fiemap.h:
> > > > > > FIEMAP_EXTENT_UNKNOWN        0x00000002 /* Data location unknown. */
> > > > > > FIEMAP_EXTENT_DELALLOC        0x00000004 /* Location still pending.
> > > > > >                             * Sets EXTENT_UNKNOWN. */
> > > > > > FIEMAP_EXTENT_UNWRITTEN        0x00000800 /* Space allocated, but
> > > > > >                             * no data (i.e. zero). */
> > > > > > 
> > > > > > FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
> > > > > > been written any data, which status f2fs is not supported now, for all
> > > > > > NEW_ADDR block addresses, it means delay allocated blocks, so let's
> > > > > > tag FIEMAP_EXTENT_DELALLOC instead.
> > > > > > 
> > > > > > Testcase:
> > > > > > xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
> > > > > > filefrag -v /mnt/f2fs/file
> > > > > > 
> > > > > > Output:
> > > > > > - Before
> > > > > > Filesystem type is: f2f52010
> > > > > > Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
> > > > > >    ext:     logical_offset:        physical_offset: length:   expected: flags:
> > > > > >      0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
> > > > > > /mnt/f2fs/file: 1 extent found
> > > > > > 
> > > > > > After:
> > > > > > Filesystem type is: f2f52010
> > > > > > File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
> > > > > >    ext:     logical_offset:        physical_offset: length:   expected: flags:
> > > > > >      0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
> > > > > > /mnt/f2fs/file: 1 extent found
> > > > > > 
> > > > > > Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
> > > > > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > > > > ---
> > > > > >    fs/f2fs/data.c | 7 +++++--
> > > > > >    1 file changed, 5 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > > > > index 359de650772e..3afc9764743e 100644
> > > > > > --- a/fs/f2fs/data.c
> > > > > > +++ b/fs/f2fs/data.c
> > > > > > @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > > > > >        }
> > > > > >        if (size) {
> > > > > > -        flags |= FIEMAP_EXTENT_MERGED;
> > > > > > +        if (flags & FIEMAP_EXTENT_DELALLOC)
> > > > > > +            phys = 0;
> > > > > > +        else
> > > > > > +            flags |= FIEMAP_EXTENT_MERGED;
> > > > > >            if (IS_ENCRYPTED(inode))
> > > > > >                flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
> > > > > > @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > > > > >                    size += blks_to_bytes(inode, 1);
> > > > > >                }
> > > > > >            } else if (map.m_flags & F2FS_MAP_DELALLOC) {
> > > > > > -            flags = FIEMAP_EXTENT_UNWRITTEN;
> > > > > > +            flags = FIEMAP_EXTENT_DELALLOC;
> > > > > >            }
> > > > > >            start_blk += bytes_to_blks(inode, size);
> > > > > > -- 
> > > > > > 2.36.1
> > 
> > 
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Chao Yu April 13, 2023, 9:49 a.m. UTC | #7 
On 2023/4/12 1:08, Jaegeuk Kim wrote:
> On 04/11, Chao Yu wrote:
>> On 2023/4/11 16:14, Chao Yu wrote:
>>> On 2023/4/11 1:57, Jaegeuk Kim wrote:
>>>> On 04/08, Chao Yu wrote:
>>>>> On 2023/4/8 4:59, Jaegeuk Kim wrote:
>>>>>> This breaks generic/009?
>>>>>
>>>>> I guess it is as expected?
>>>>>
>>>>> Please check description of fiemap ioctl manual from [1]:
>>>>>
>>>>> FIEMAP_EXTENT_UNKNOWN
>>>>> The location of this extent is currently unknown. This may
>>>>> indicate the data is stored on an inaccessible volume or that
>>>>> no storage has been allocated for the file yet.
>>>>>
>>>>> FIEMAP_EXTENT_DELALLOC
>>>>> This will also set FIEMAP_EXTENT_UNKNOWN.
>>>>>
>>>>> Delayed allocation - while there is data for this extent, its
>>>>> physical location has not been allocated yet.
>>>>>
>>>>> FIEMAP_EXTENT_UNWRITTEN
>>>>> Unwritten extent - the extent is allocated but its data has not
>>>>> been initialized. This indicates the extent’s data will be all
>>>>> zero if read through the filesystem but the contents are undefined
>>>>> if read directly from the device.
>>>>>
>>>>> [1] https://www.kernel.org/doc/html/latest/filesystems/fiemap.html
>>>>>
>>>>> According to its description, f2fs only support
>>>>> FIEMAP_EXTENT_{UNKNOWN, DELALLOC}, but not support
>>>>> FIEMAP_EXTENT_UNWRITTEN.
>>>>
>>>> No, I don't think so.
>>>
>>> Jaegeuk,
>>>
>>> Could you please check the detailed description of FIEMAP_EXTENT_UNWRITTEN?
>>> The flag indicates two conditions:
>>> 1. on-disk blkaddrs were allocated for extent, and the extent is tagged as
>>> unwritten status.
>>> 2. data readed on those blocks will be all zero.
>>
>> Sorry, I mean:
>>
>> 1. on-disk blkaddrs were allocated for extent;
>> 2. extent is tagged as unwritten status, data readed on those blocks will be
>> all zero.
> 
> I was thinking fallocate/pin cases to give zero data. But, we may need to check
> the space discarded securely or disk support?

I thought about similar scheme, we can add F2FS_GET_BLOCK_ZERO for fallocate/pin
case to zero data in valid block address, but the problem is how can we distinguish
zero/valid data from valid block address w/o additional unwritten flag of metadata.

e.g.
1. write page [0,511] w/ valid data;
2. fallocate/pin page [512,1023] w/ zero data;
3. fiemap file  --- what output should it be?

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>> So, let's check f2fs' status:
>>> - fallocate only reserve valid block count and set NEW_ADDR in dnode, so
>>> it does not match condition 1)
>>> - pin & fallocate preallocates blkaddrs and set blkaddrs in dnode, but
>>> content on those blkaddrs may contain zero or random data, so it does not
>>> match  condition 2)
>>>
>>> Christoph describes this issue in below patch as well, you can check it.
>>> da8c7fecc9c7 ("f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC")
>>>
>>> Am I missing something?
>>>
>>> Thanks,
>>>
>>>>
>>>>>
>>>>> So 009, 092, 094 .. which expects unwritten status from extent will
>>>>> fail.
>>>>>
>>>>> How about disabling those testcase?
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>
>>>>>> On 04/05, Chao Yu wrote:
>>>>>>> xfstest generic/614 fails to run due below reason:
>>>>>>>
>>>>>>> generic/614 1s ... [not run] test requires delayed allocation buffered writes
>>>>>>>
>>>>>>> The root cause is f2fs tags wrong fiemap flag for delay allocated
>>>>>>> extent.
>>>>>>>
>>>>>>> Quoted from fiemap.h:
>>>>>>> FIEMAP_EXTENT_UNKNOWN        0x00000002 /* Data location unknown. */
>>>>>>> FIEMAP_EXTENT_DELALLOC        0x00000004 /* Location still pending.
>>>>>>>                              * Sets EXTENT_UNKNOWN. */
>>>>>>> FIEMAP_EXTENT_UNWRITTEN        0x00000800 /* Space allocated, but
>>>>>>>                              * no data (i.e. zero). */
>>>>>>>
>>>>>>> FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
>>>>>>> been written any data, which status f2fs is not supported now, for all
>>>>>>> NEW_ADDR block addresses, it means delay allocated blocks, so let's
>>>>>>> tag FIEMAP_EXTENT_DELALLOC instead.
>>>>>>>
>>>>>>> Testcase:
>>>>>>> xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
>>>>>>> filefrag -v /mnt/f2fs/file
>>>>>>>
>>>>>>> Output:
>>>>>>> - Before
>>>>>>> Filesystem type is: f2f52010
>>>>>>> Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>>>>     ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>>>>       0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
>>>>>>> /mnt/f2fs/file: 1 extent found
>>>>>>>
>>>>>>> After:
>>>>>>> Filesystem type is: f2f52010
>>>>>>> File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>>>>     ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>>>>       0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
>>>>>>> /mnt/f2fs/file: 1 extent found
>>>>>>>
>>>>>>> Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
>>>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>>>> ---
>>>>>>>     fs/f2fs/data.c | 7 +++++--
>>>>>>>     1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>> index 359de650772e..3afc9764743e 100644
>>>>>>> --- a/fs/f2fs/data.c
>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>> @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>>>>         }
>>>>>>>         if (size) {
>>>>>>> -        flags |= FIEMAP_EXTENT_MERGED;
>>>>>>> +        if (flags & FIEMAP_EXTENT_DELALLOC)
>>>>>>> +            phys = 0;
>>>>>>> +        else
>>>>>>> +            flags |= FIEMAP_EXTENT_MERGED;
>>>>>>>             if (IS_ENCRYPTED(inode))
>>>>>>>                 flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
>>>>>>> @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>>>>                     size += blks_to_bytes(inode, 1);
>>>>>>>                 }
>>>>>>>             } else if (map.m_flags & F2FS_MAP_DELALLOC) {
>>>>>>> -            flags = FIEMAP_EXTENT_UNWRITTEN;
>>>>>>> +            flags = FIEMAP_EXTENT_DELALLOC;
>>>>>>>             }
>>>>>>>             start_blk += bytes_to_blks(inode, size);
>>>>>>> -- 
>>>>>>> 2.36.1
>>>
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Chao Yu April 19, 2023, 3:30 p.m. UTC | #8 
On 2023/4/13 17:49, Chao Yu wrote:
> On 2023/4/12 1:08, Jaegeuk Kim wrote:
>> On 04/11, Chao Yu wrote:
>>> On 2023/4/11 16:14, Chao Yu wrote:
>>>> On 2023/4/11 1:57, Jaegeuk Kim wrote:
>>>>> On 04/08, Chao Yu wrote:
>>>>>> On 2023/4/8 4:59, Jaegeuk Kim wrote:
>>>>>>> This breaks generic/009?
>>>>>>
>>>>>> I guess it is as expected?
>>>>>>
>>>>>> Please check description of fiemap ioctl manual from [1]:
>>>>>>
>>>>>> FIEMAP_EXTENT_UNKNOWN
>>>>>> The location of this extent is currently unknown. This may
>>>>>> indicate the data is stored on an inaccessible volume or that
>>>>>> no storage has been allocated for the file yet.
>>>>>>
>>>>>> FIEMAP_EXTENT_DELALLOC
>>>>>> This will also set FIEMAP_EXTENT_UNKNOWN.
>>>>>>
>>>>>> Delayed allocation - while there is data for this extent, its
>>>>>> physical location has not been allocated yet.
>>>>>>
>>>>>> FIEMAP_EXTENT_UNWRITTEN
>>>>>> Unwritten extent - the extent is allocated but its data has not
>>>>>> been initialized. This indicates the extent’s data will be all
>>>>>> zero if read through the filesystem but the contents are undefined
>>>>>> if read directly from the device.
>>>>>>
>>>>>> [1] https://www.kernel.org/doc/html/latest/filesystems/fiemap.html
>>>>>>
>>>>>> According to its description, f2fs only support
>>>>>> FIEMAP_EXTENT_{UNKNOWN, DELALLOC}, but not support
>>>>>> FIEMAP_EXTENT_UNWRITTEN.
>>>>>
>>>>> No, I don't think so.
>>>>
>>>> Jaegeuk,
>>>>
>>>> Could you please check the detailed description of FIEMAP_EXTENT_UNWRITTEN?
>>>> The flag indicates two conditions:
>>>> 1. on-disk blkaddrs were allocated for extent, and the extent is tagged as
>>>> unwritten status.
>>>> 2. data readed on those blocks will be all zero.
>>>
>>> Sorry, I mean:
>>>
>>> 1. on-disk blkaddrs were allocated for extent;
>>> 2. extent is tagged as unwritten status, data readed on those blocks will be
>>> all zero.
>>
>> I was thinking fallocate/pin cases to give zero data. But, we may need to check
>> the space discarded securely or disk support?
> 
> I thought about similar scheme, we can add F2FS_GET_BLOCK_ZERO for fallocate/pin
> case to zero data in valid block address, but the problem is how can we distinguish
> zero/valid data from valid block address w/o additional unwritten flag of metadata.
> 
> e.g.
> 1. write page [0,511] w/ valid data;
> 2. fallocate/pin page [512,1023] w/ zero data;
> 3. fiemap file  --- what output should it be?

Thoughts?

Thanks,

> 
> Thanks,
> 
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> So, let's check f2fs' status:
>>>> - fallocate only reserve valid block count and set NEW_ADDR in dnode, so
>>>> it does not match condition 1)
>>>> - pin & fallocate preallocates blkaddrs and set blkaddrs in dnode, but
>>>> content on those blkaddrs may contain zero or random data, so it does not
>>>> match  condition 2)
>>>>
>>>> Christoph describes this issue in below patch as well, you can check it.
>>>> da8c7fecc9c7 ("f2fs: rename F2FS_MAP_UNWRITTEN to F2FS_MAP_DELALLOC")
>>>>
>>>> Am I missing something?
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> So 009, 092, 094 .. which expects unwritten status from extent will
>>>>>> fail.
>>>>>>
>>>>>> How about disabling those testcase?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>
>>>>>>> On 04/05, Chao Yu wrote:
>>>>>>>> xfstest generic/614 fails to run due below reason:
>>>>>>>>
>>>>>>>> generic/614 1s ... [not run] test requires delayed allocation buffered writes
>>>>>>>>
>>>>>>>> The root cause is f2fs tags wrong fiemap flag for delay allocated
>>>>>>>> extent.
>>>>>>>>
>>>>>>>> Quoted from fiemap.h:
>>>>>>>> FIEMAP_EXTENT_UNKNOWN        0x00000002 /* Data location unknown. */
>>>>>>>> FIEMAP_EXTENT_DELALLOC        0x00000004 /* Location still pending.
>>>>>>>>                               * Sets EXTENT_UNKNOWN. */
>>>>>>>> FIEMAP_EXTENT_UNWRITTEN        0x00000800 /* Space allocated, but
>>>>>>>>                               * no data (i.e. zero). */
>>>>>>>>
>>>>>>>> FIEMAP_EXTENT_UNWRITTEN means block address is preallocated, but w/o
>>>>>>>> been written any data, which status f2fs is not supported now, for all
>>>>>>>> NEW_ADDR block addresses, it means delay allocated blocks, so let's
>>>>>>>> tag FIEMAP_EXTENT_DELALLOC instead.
>>>>>>>>
>>>>>>>> Testcase:
>>>>>>>> xfs_io -f -c 'pwrite 0 64k' /mnt/f2fs/file;
>>>>>>>> filefrag -v /mnt/f2fs/file
>>>>>>>>
>>>>>>>> Output:
>>>>>>>> - Before
>>>>>>>> Filesystem type is: f2f52010
>>>>>>>> Fize of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>>>>>      ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>>>>>        0:        0..      15:          0..        15:     16:             last,unwritten,merged,eof
>>>>>>>> /mnt/f2fs/file: 1 extent found
>>>>>>>>
>>>>>>>> After:
>>>>>>>> Filesystem type is: f2f52010
>>>>>>>> File size of /mnt/f2fs/file is 65536 (16 blocks of 4096 bytes)
>>>>>>>>      ext:     logical_offset:        physical_offset: length:   expected: flags:
>>>>>>>>        0:        0..      15:          0..         0:      0:             last,unknown_loc,delalloc,eof
>>>>>>>> /mnt/f2fs/file: 1 extent found
>>>>>>>>
>>>>>>>> Fixes: 7f63eb77af7b ("f2fs: report unwritten area in f2fs_fiemap")
>>>>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>>>>> ---
>>>>>>>>      fs/f2fs/data.c | 7 +++++--
>>>>>>>>      1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>>> index 359de650772e..3afc9764743e 100644
>>>>>>>> --- a/fs/f2fs/data.c
>>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>>> @@ -1995,7 +1995,10 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>>>>>          }
>>>>>>>>          if (size) {
>>>>>>>> -        flags |= FIEMAP_EXTENT_MERGED;
>>>>>>>> +        if (flags & FIEMAP_EXTENT_DELALLOC)
>>>>>>>> +            phys = 0;
>>>>>>>> +        else
>>>>>>>> +            flags |= FIEMAP_EXTENT_MERGED;
>>>>>>>>              if (IS_ENCRYPTED(inode))
>>>>>>>>                  flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
>>>>>>>> @@ -2035,7 +2038,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>>>>>                      size += blks_to_bytes(inode, 1);
>>>>>>>>                  }
>>>>>>>>              } else if (map.m_flags & F2FS_MAP_DELALLOC) {
>>>>>>>> -            flags = FIEMAP_EXTENT_UNWRITTEN;
>>>>>>>> +            flags = FIEMAP_EXTENT_DELALLOC;
>>>>>>>>              }
>>>>>>>>              start_blk += bytes_to_blks(inode, size);
>>>>>>>> -- 
>>>>>>>> 2.36.1
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-f2fs-devel mailing list
>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
diff mbox series

Patch

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 359de650772e..3afc9764743e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1995,7 +1995,10 @@  int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	}
 
 	if (size) {
-		flags |= FIEMAP_EXTENT_MERGED;
+		if (flags & FIEMAP_EXTENT_DELALLOC)
+			phys = 0;
+		else
+			flags |= FIEMAP_EXTENT_MERGED;
 		if (IS_ENCRYPTED(inode))
 			flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
 
@@ -2035,7 +2038,7 @@  int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 				size += blks_to_bytes(inode, 1);
 			}
 		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
-			flags = FIEMAP_EXTENT_UNWRITTEN;
+			flags = FIEMAP_EXTENT_DELALLOC;
 		}
 
 		start_blk += bytes_to_blks(inode, size);