2019-06-27 17:05:36

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH] f2fs: allocate blocks for pinned file

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

Signed-off-by: Jaegeuk Kim <[email protected]>
---
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


2019-06-28 01:30:08

by Chao Yu

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

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 <[email protected]>
> ---
> 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;
>
>

2019-06-28 02:16:12

by Jaegeuk Kim

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

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 <[email protected]>
> > ---
> > 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;
> >
> >

2019-06-28 02:42:33

by Chao Yu

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

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 <[email protected]>
>>> ---
>>> 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;
>>>
>>>
> .
>

2019-06-28 15:37:52

by Jaegeuk Kim

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

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

Signed-off-by: Jaegeuk Kim <[email protected]>
---
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

2019-06-29 08:36:51

by Chao Yu

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

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 <[email protected]>

Looks good to me now.

Reviewed-by: Chao Yu <[email protected]>

Thanks,