[PATCH] f2fs: allocate blocks for pinned file

[PATCH] f2fs: allocate blocks for pinned file

[Jaegeuk Kim]

This patch allows fallocate to allocate physical blocks for pinned file.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/file.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index e7c368db8185..cdfd4338682d 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1528,7 +1528,12 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (off_end)
 		map.m_len++;
 
-	err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO);
+	if (f2fs_is_pinned_file(inode))
+		map.m_seg_type = CURSEG_COLD_DATA;
+
+	err = f2fs_map_blocks(inode, &map, 1, (f2fs_is_pinned_file(inode) ?
+						F2FS_GET_BLOCK_PRE_DIO :
+						F2FS_GET_BLOCK_PRE_AIO));
 	if (err) {
 		pgoff_t last_off;
 
-- 
2.19.0.605.g01d371f741-goog

Re: [f2fs-dev] [PATCH] f2fs: allocate blocks for pinned file

[Chao Yu]

Hi Jaegeuk,

On 2019/6/28 1:05, Jaegeuk Kim wrote:
> This patch allows fallocate to allocate physical blocks for pinned file.

Quoted from manual of fallocate(2):
"
Any subregion within the range specified by offset and len that did not contain
data before the  call  will  be  initialized  to zero.

If  the  FALLOC_FL_KEEP_SIZE  flag  is specified in mode .... Preallocating
zeroed blocks beyond the end of the file in this manner is useful for optimizing
append workloads.
"

As quoted description, our change may break the rule of fallocate(, mode = 0),
because with after this change, we can't guarantee that preallocated physical
block contains zeroed data

Should we introduce an additional ioctl for this case? Or maybe add one more
flag in fallocate() for unzeroed block preallocation, not sure.

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/file.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index e7c368db8185..cdfd4338682d 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1528,7 +1528,12 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>  	if (off_end)
>  		map.m_len++;
>  
> -	err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO);
> +	if (f2fs_is_pinned_file(inode))
> +		map.m_seg_type = CURSEG_COLD_DATA;
> +
> +	err = f2fs_map_blocks(inode, &map, 1, (f2fs_is_pinned_file(inode) ?
> +						F2FS_GET_BLOCK_PRE_DIO :
> +						F2FS_GET_BLOCK_PRE_AIO));
>  	if (err) {
>  		pgoff_t last_off;
>  
> 

Re: [f2fs-dev] [PATCH] f2fs: allocate blocks for pinned file

[Jaegeuk Kim]

On 06/28, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2019/6/28 1:05, Jaegeuk Kim wrote:
> > This patch allows fallocate to allocate physical blocks for pinned file.
> 
> Quoted from manual of fallocate(2):
> "
> Any subregion within the range specified by offset and len that did not contain
> data before the  call  will  be  initialized  to zero.
> 
> If  the  FALLOC_FL_KEEP_SIZE  flag  is specified in mode .... Preallocating
> zeroed blocks beyond the end of the file in this manner is useful for optimizing
> append workloads.
> "
> 
> As quoted description, our change may break the rule of fallocate(, mode = 0),
> because with after this change, we can't guarantee that preallocated physical
> block contains zeroed data
> 
> Should we introduce an additional ioctl for this case? Or maybe add one more
> flag in fallocate() for unzeroed block preallocation, not sure.

I thought like that, but this is a very corner case for the pinned file only in
f2fs. And, the pinned file is also indeed used by this purpose.

Thanks,

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/file.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index e7c368db8185..cdfd4338682d 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -1528,7 +1528,12 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
> >  	if (off_end)
> >  		map.m_len++;
> >  
> > -	err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO);
> > +	if (f2fs_is_pinned_file(inode))
> > +		map.m_seg_type = CURSEG_COLD_DATA;
> > +
> > +	err = f2fs_map_blocks(inode, &map, 1, (f2fs_is_pinned_file(inode) ?
> > +						F2FS_GET_BLOCK_PRE_DIO :
> > +						F2FS_GET_BLOCK_PRE_AIO));
> >  	if (err) {
> >  		pgoff_t last_off;
> >  
> > 

Re: [f2fs-dev] [PATCH] f2fs: allocate blocks for pinned file

[Chao Yu]

On 2019/6/28 10:15, Jaegeuk Kim wrote:
> On 06/28, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2019/6/28 1:05, Jaegeuk Kim wrote:
>>> This patch allows fallocate to allocate physical blocks for pinned file.
>>
>> Quoted from manual of fallocate(2):
>> "
>> Any subregion within the range specified by offset and len that did not contain
>> data before the  call  will  be  initialized  to zero.
>>
>> If  the  FALLOC_FL_KEEP_SIZE  flag  is specified in mode .... Preallocating
>> zeroed blocks beyond the end of the file in this manner is useful for optimizing
>> append workloads.
>> "
>>
>> As quoted description, our change may break the rule of fallocate(, mode = 0),
>> because with after this change, we can't guarantee that preallocated physical
>> block contains zeroed data
>>
>> Should we introduce an additional ioctl for this case? Or maybe add one more
>> flag in fallocate() for unzeroed block preallocation, not sure.
> 
> I thought like that, but this is a very corner case for the pinned file only in
> f2fs. And, the pinned file is also indeed used by this purpose.

Okay, I think we need to find one place to document such behavior that is not
matching regular poxis fallocate's sematic, user should be noticed about it.

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>>  fs/f2fs/file.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index e7c368db8185..cdfd4338682d 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -1528,7 +1528,12 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
>>>  	if (off_end)
>>>  		map.m_len++;
>>>  
>>> -	err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO);
>>> +	if (f2fs_is_pinned_file(inode))
>>> +		map.m_seg_type = CURSEG_COLD_DATA;
>>> +
>>> +	err = f2fs_map_blocks(inode, &map, 1, (f2fs_is_pinned_file(inode) ?
>>> +						F2FS_GET_BLOCK_PRE_DIO :
>>> +						F2FS_GET_BLOCK_PRE_AIO));
>>>  	if (err) {
>>>  		pgoff_t last_off;
>>>  
>>>
> .
> 

Re: [PATCH v2] f2fs: allocate blocks for pinned file

[Jaegeuk Kim]

This patch allows fallocate to allocate physical blocks for pinned file.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 Documentation/filesystems/f2fs.txt | 25 +++++++++++++++++++++++++
 fs/f2fs/file.c                     |  7 ++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
index bebd1be3ba49..496fa28b2492 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -804,3 +804,28 @@ WRITE_LIFE_NOT_SET    WARM_DATA                WRITE_LIFE_NOT_SET
 WRITE_LIFE_NONE       "                        WRITE_LIFE_NONE
 WRITE_LIFE_MEDIUM     "                        WRITE_LIFE_MEDIUM
 WRITE_LIFE_LONG       "                        WRITE_LIFE_LONG
+
+Fallocate(2) Policy
+-------------------
+
+The default policy follows the below posix rule.
+
+Allocating disk space
+    The default operation (i.e., mode is zero) of fallocate() allocates
+    the disk space within the range specified by offset and len.  The
+    file size (as reported by stat(2)) will be changed if offset+len is
+    greater than the file size.  Any subregion within the range specified
+    by offset and len that did not contain data before the call will be
+    initialized to zero.  This default behavior closely resembles the
+    behavior of the posix_fallocate(3) library function, and is intended
+    as a method of optimally implementing that function.
+
+However, once F2FS receives ioctl(fd, F2FS_IOC_SET_PIN_FILE) in prior to
+fallocate(fd, DEFAULT_MODE), it allocates on-disk blocks addressess having
+zero or random data, which is useful to the below scenario where:
+ 1. create(fd)
+ 2. ioctl(fd, F2FS_IOC_SET_PIN_FILE)
+ 3. fallocate(fd, 0, 0, size)
+ 4. address = fibmap(fd, offset)
+ 5. open(blkdev)
+ 6. write(blkdev, address)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index e7c368db8185..cdfd4338682d 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1528,7 +1528,12 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
 	if (off_end)
 		map.m_len++;
 
-	err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO);
+	if (f2fs_is_pinned_file(inode))
+		map.m_seg_type = CURSEG_COLD_DATA;
+
+	err = f2fs_map_blocks(inode, &map, 1, (f2fs_is_pinned_file(inode) ?
+						F2FS_GET_BLOCK_PRE_DIO :
+						F2FS_GET_BLOCK_PRE_AIO));
 	if (err) {
 		pgoff_t last_off;
 
-- 
2.19.0.605.g01d371f741-goog

Re: [f2fs-dev] [PATCH v2] f2fs: allocate blocks for pinned file

[Chao Yu]

On 2019/6/28 23:36, Jaegeuk Kim wrote:
> This patch allows fallocate to allocate physical blocks for pinned file.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Looks good to me now.

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,