2022-06-13 03:25:49

by Chao Liu

[permalink] [raw]
Subject: [PATCH v2] docs: filesystems: f2fs: fix description about compress ioctl

From: Chao Liu <[email protected]>

Since commit c61404153eb6 ("f2fs: introduce FI_COMPRESS_RELEASED
instead of using IMMUTABLE bit"), we no longer use the IMMUTABLE
bit to prevent writing data for compression. Let's correct the
corresponding documentation.

BTW, this patch fixes some alignment issues in the compress
metadata layout.

Signed-off-by: Chao Liu <[email protected]>
---
v2:
- s/file size/filesize/
Documentation/filesystems/f2fs.rst | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index ad8dc8c040a2..531b0f8a3946 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -818,10 +818,11 @@ Compression implementation
Instead, the main goal is to reduce data writes to flash disk as much as
possible, resulting in extending disk life time as well as relaxing IO
congestion. Alternatively, we've added ioctl(F2FS_IOC_RELEASE_COMPRESS_BLOCKS)
- interface to reclaim compressed space and show it to user after putting the
- immutable bit. Immutable bit, after release, it doesn't allow writing/mmaping
- on the file, until reserving compressed space via
- ioctl(F2FS_IOC_RESERVE_COMPRESS_BLOCKS) or truncating filesize to zero.
+ interface to reclaim compressed space and show it to user after setting a
+ special flag to the inode. Once the compressed space is released, the flag
+ will block writing data to the file until either the compressed space is
+ reserved via ioctl(F2FS_IOC_RESERVE_COMPRESS_BLOCKS) or the filesize is
+ truncated to zero.

Compress metadata layout::

@@ -830,12 +831,12 @@ Compress metadata layout::
| cluster 1 | cluster 2 | ......... | cluster N |
+-----------------------------------------------+
. . . .
- . . . .
+ . . . .
. Compressed Cluster . . Normal Cluster .
+----------+---------+---------+---------+ +---------+---------+---------+---------+
|compr flag| block 1 | block 2 | block 3 | | block 1 | block 2 | block 3 | block 4 |
+----------+---------+---------+---------+ +---------+---------+---------+---------+
- . .
+ . .
. .
. .
+-------------+-------------+----------+----------------------------+
--
2.36.1


2022-06-13 03:55:45

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH v2] docs: filesystems: f2fs: fix description about compress ioctl

On Mon, Jun 13, 2022 at 10:08:00AM +0800, Chao Liu wrote:
> v2:
> - s/file size/filesize/

Why would you change it to be wrong?

> Documentation/filesystems/f2fs.rst | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index ad8dc8c040a2..531b0f8a3946 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -818,10 +818,11 @@ Compression implementation
> Instead, the main goal is to reduce data writes to flash disk as much as
> possible, resulting in extending disk life time as well as relaxing IO
> congestion. Alternatively, we've added ioctl(F2FS_IOC_RELEASE_COMPRESS_BLOCKS)
> - interface to reclaim compressed space and show it to user after putting the
> - immutable bit. Immutable bit, after release, it doesn't allow writing/mmaping
> - on the file, until reserving compressed space via
> - ioctl(F2FS_IOC_RESERVE_COMPRESS_BLOCKS) or truncating filesize to zero.
> + interface to reclaim compressed space and show it to user after setting a
> + special flag to the inode. Once the compressed space is released, the flag
> + will block writing data to the file until either the compressed space is
> + reserved via ioctl(F2FS_IOC_RESERVE_COMPRESS_BLOCKS) or the filesize is
> + truncated to zero.
>
> Compress metadata layout::
>
> @@ -830,12 +831,12 @@ Compress metadata layout::
> | cluster 1 | cluster 2 | ......... | cluster N |
> +-----------------------------------------------+
> . . . .
> - . . . .
> + . . . .
> . Compressed Cluster . . Normal Cluster .
> +----------+---------+---------+---------+ +---------+---------+---------+---------+
> |compr flag| block 1 | block 2 | block 3 | | block 1 | block 2 | block 3 | block 4 |
> +----------+---------+---------+---------+ +---------+---------+---------+---------+
> - . .
> + . .
> . .
> . .
> +-------------+-------------+----------+----------------------------+
> --
> 2.36.1
>

2022-06-13 08:26:52

by Chao Liu

[permalink] [raw]
Subject: Re: [PATCH v2] docs: filesystems: f2fs: fix description about compress ioctl

On Mon, Jun 13, 2022 at 04:37:01AM +0100, Matthew Wilcox wrote:
> On Mon, Jun 13, 2022 at 10:08:00AM +0800, Chao Liu wrote:
> > v2:
> > - s/file size/filesize/
>
> Why would you change it to be wrong?
>

This is a suggestion from Chao Yu. Maybe he has some other considerations.

Hi Chao,

Can you help with this question?

Thanks.

> > Documentation/filesystems/f2fs.rst | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> > index ad8dc8c040a2..531b0f8a3946 100644
> > --- a/Documentation/filesystems/f2fs.rst
> > +++ b/Documentation/filesystems/f2fs.rst
> > @@ -818,10 +818,11 @@ Compression implementation
> > Instead, the main goal is to reduce data writes to flash disk as much as
> > possible, resulting in extending disk life time as well as relaxing IO
> > congestion. Alternatively, we've added ioctl(F2FS_IOC_RELEASE_COMPRESS_BLOCKS)
> > - interface to reclaim compressed space and show it to user after putting the
> > - immutable bit. Immutable bit, after release, it doesn't allow writing/mmaping
> > - on the file, until reserving compressed space via
> > - ioctl(F2FS_IOC_RESERVE_COMPRESS_BLOCKS) or truncating filesize to zero.
> > + interface to reclaim compressed space and show it to user after setting a
> > + special flag to the inode. Once the compressed space is released, the flag
> > + will block writing data to the file until either the compressed space is
> > + reserved via ioctl(F2FS_IOC_RESERVE_COMPRESS_BLOCKS) or the filesize is
> > + truncated to zero.
> >
> > Compress metadata layout::
> >
> > @@ -830,12 +831,12 @@ Compress metadata layout::
> > | cluster 1 | cluster 2 | ......... | cluster N |
> > +-----------------------------------------------+
> > . . . .
> > - . . . .
> > + . . . .
> > . Compressed Cluster . . Normal Cluster .
> > +----------+---------+---------+---------+ +---------+---------+---------+---------+
> > |compr flag| block 1 | block 2 | block 3 | | block 1 | block 2 | block 3 | block 4 |
> > +----------+---------+---------+---------+ +---------+---------+---------+---------+
> > - . .
> > + . .
> > . .
> > . .
> > +-------------+-------------+----------+----------------------------+
> > --
> > 2.36.1
> >

2022-06-13 20:09:42

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH v2] docs: filesystems: f2fs: fix description about compress ioctl

Chao Liu <[email protected]> writes:

> On Mon, Jun 13, 2022 at 04:37:01AM +0100, Matthew Wilcox wrote:
>> On Mon, Jun 13, 2022 at 10:08:00AM +0800, Chao Liu wrote:
>> > v2:
>> > - s/file size/filesize/
>>
>> Why would you change it to be wrong?
>>
>
> This is a suggestion from Chao Yu. Maybe he has some other considerations.

Sorry, I should have replied to that. I disagree with that suggestion.
"Filesize" is not an English word, and there doesn't seem to be any
reason to use it in our docs.

<checks>

We have to occurrences now - one already in the f2fs docs. I think we
shouldn't add more. So my plan is to apply the first version of this
patch. Chao Liu: is there a reason why you didn't add the Reviewed-by
from Chao Yu in the second version? Chao Yu: is that tag still
applicable even without the "filesize" change?

Thanks,

jon

2022-06-14 03:07:26

by Chao Liu

[permalink] [raw]
Subject: Re: [PATCH v2] docs: filesystems: f2fs: fix description about compress ioctl

On Mon, Jun 13, 2022 at 11:23:07AM -0600, Jonathan Corbet wrote:
> Chao Liu <[email protected]> writes:
>
> > On Mon, Jun 13, 2022 at 04:37:01AM +0100, Matthew Wilcox wrote:
> >> On Mon, Jun 13, 2022 at 10:08:00AM +0800, Chao Liu wrote:
> >> > v2:
> >> > - s/file size/filesize/
> >>
> >> Why would you change it to be wrong?
> >>
> >
> > This is a suggestion from Chao Yu. Maybe he has some other considerations.
>
> Sorry, I should have replied to that. I disagree with that suggestion.
> "Filesize" is not an English word, and there doesn't seem to be any
> reason to use it in our docs.
>
> <checks>
>
> We have to occurrences now - one already in the f2fs docs. I think we
> shouldn't add more. So my plan is to apply the first version of this
> patch. Chao Liu: is there a reason why you didn't add the Reviewed-by
> from Chao Yu in the second version?

Sorry for my carelessness. I should have added it.

Thanks,

2022-06-14 03:07:43

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH v2] docs: filesystems: f2fs: fix description about compress ioctl

On 2022/6/14 1:23, Jonathan Corbet wrote:
> Chao Liu <[email protected]> writes:
>
>> On Mon, Jun 13, 2022 at 04:37:01AM +0100, Matthew Wilcox wrote:
>>> On Mon, Jun 13, 2022 at 10:08:00AM +0800, Chao Liu wrote:
>>>> v2:
>>>> - s/file size/filesize/
>>>
>>> Why would you change it to be wrong?
>>>
>>
>> This is a suggestion from Chao Yu. Maybe he has some other considerations.
>
> Sorry, I should have replied to that. I disagree with that suggestion.
> "Filesize" is not an English word, and there doesn't seem to be any
> reason to use it in our docs.

My bad, out of mind at that time, sorry for my wrong suggestion...

>
> <checks>
>
> We have to occurrences now - one already in the f2fs docs. I think we
> shouldn't add more. So my plan is to apply the first version of this
> patch. Chao Liu: is there a reason why you didn't add the Reviewed-by
> from Chao Yu in the second version? Chao Yu: is that tag still
> applicable even without the "filesize" change?

Yes.

Thanks,

>
> Thanks,
>
> jon