Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Christoph Hellwig]

Michael, can you apply this now that we merged FALLOC_FL_COLLAPSE_RANGE
into the kernel tree?

On Wed, Feb 19, 2014 at 01:39:53AM +0900, Namjae Jeon wrote:
> From: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Update FALLOC_FL_COLLAPSE_RANGE flag in fallocate.
> 
> Signed-off-by: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Ashish Sangwan <a.sangwan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  man2/fallocate.2 |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/man2/fallocate.2 b/man2/fallocate.2
> index b31bbde..06a0b0c 100644
> --- a/man2/fallocate.2
> +++ b/man2/fallocate.2
> @@ -1,5 +1,6 @@
>  .\" Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved
>  .\" Written by Dave Chinner <dgc-sJ/iWh9BUns@public.gmane.org>
> +.\" Updated by Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>  .\"
>  .\" %%%LICENSE_START(GPLv2_ONELINE)
>  .\" May be distributed as per GNU General Public License version 2.
> @@ -7,6 +8,7 @@
>  .\"
>  .\" 2011-09-19: Added FALLOC_FL_PUNCH_HOLE
>  .\" 2011-09-19: Substantial restructuring of the page
> +.\" 2014-02-19: Added FALLOC_FL_COLLAPSE_RANGE
>  .\"
>  .TH FALLOCATE 2 2014-01-24 "Linux" "Linux Programmer's Manual"
>  .SH NAME
> @@ -124,6 +126,25 @@ Btrfs (since Linux 3.7)
>  .IP *
>  tmpfs (since Linux 3.5)
>  .\" commit 83e4fa9c16e4af7122e31be3eca5d57881d236fe
> +.SS Collapsing file space
> +Specifiying the
> +.BR FALLOC_FL_COLLAPSE_RANGE
> +flag in
> +.I mode
> +collpse space in the byte range starting at
> +.I offset
> +and continuing for
> +.I len
> +bytes. Within the specified range, it first de-allocates blocks and eliminates
> +the hole created in this process by shifting data blocks into the hole.
> +
> +Different filesystem may implement different limitatios on the granularity of
> +the operation. Most filesystem will limit operations to filesystem block size
> +boundaries, but this boundary may be larger or smaller depending
> +on the filesystem and/or the configuration of the filesystem or file.
> +
> +If filesystem has such limitation on granularity, It will return error(EINVAL).
> +
>  .SH RETURN VALUE
>  On success,
>  .BR fallocate ()
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Michael Kerrisk (man-pages)]

On 04/16/2014 08:05 AM, Christoph Hellwig wrote:
> Michael, can you apply this now that we merged FALLOC_FL_COLLAPSE_RANGE
> into the kernel tree?

I've applied Namjae Jeon's patch (thanks Namjae!), and done a lot of
tweaking to generate the patch below.

Along the way I noticed that there is an inconsistency between XFS
and JFS that should be fixed (and so have added Dave and Ted to CC,
as well as linux-fsdevel@). 

If 'fd' does not refer to a regular file, then XFS fails with the 
error EINVAL (fs/xfs/xfs_file.c::xfs_file_fallocate()), but ext4 
fails with the error EOPNOTSUP
(fs/ext4/extents::ext4_collapse_range()). I suspect that EINVAL
is the right error for this case.

I'd appreciate review on my revised patch, below.

Cheers,

Michael


diff --git a/man2/fallocate.2 b/man2/fallocate.2
index b31bbde..15d0c2b 100644
--- a/man2/fallocate.2
+++ b/man2/fallocate.2
@@ -124,6 +124,62 @@ Btrfs (since Linux 3.7)
 .IP *
 tmpfs (since Linux 3.5)
 .\" commit 83e4fa9c16e4af7122e31be3eca5d57881d236fe
+.SS Collapsing file space
+.\" commit 00f5e61998dd17f5375d9dfc01331f104b83f841
+Specifying the
+.BR FALLOC_FL_COLLAPSE_RANGE
+flag (available since Linux 3.15) in
+.I mode
+removes a byte range from a file, without leaving a hole.
+The byte range to be collapsed starts at
+.I offset
+and continues for
+.I len
+bytes.
+At the completion of the operation,
+the contents of the file starting at the location
+.I offset+len
+will be appended at the location
+.IR offset ,
+and the file will be
+.I len
+bytes smaller.
+
+A filesystem may place limitations on the granularity of the operation,
+in order to ensure efficient implementation.
+Typically,
+.I offset
+and
+.I len
+must be a multiple of the filesystem logical block size,
+which varies according to the filesystem type and configuration.
+If a filesystem has such a requirement,
+.BR fallocate ()
+will fail with the error
+.BR EINVAL
+if this requirement is violated.
+
+If the region specified by
+.I offset
+plus
+.I len
+reaches or passes the end of file, an error is returned;
+instead, use
+.BR ftruncate (2)
+to truncate a file.
+
+No other flags may be specified in
+.IR mode
+in conjunction with
+.BR FALLOC_FL_COLLAPSE_RANGE .
+
+As at Linux 3.15,
+.B FALLOC_FL_COLLAPSE_RANGE
+is supported by
+ext4 (only for extent-based files)
+.\" commit 9eb79482a97152930b113b51dff530aba9e28c8e
+and XFS.
+.\" commit e1d8fb88a64c1f8094b9f6c3b6d2d9e6719c970d
 .SH RETURN VALUE
 On success,
 .BR fallocate ()
@@ -156,6 +212,36 @@ was less than 0, or
 .\" http://thread.gmane.org/gmane.linux.file-systems/48331/focus=1193526
 was less than or equal to 0.
 .TP
+.B EINVAL
+.I mode
+is
+.BR FALLOC_FL_COLLAPSE_RANGE
+and the range specified by
+.I offset
+plus
+.I len
+reaches or passes the end of the file.
+.TP
+.B EINVAL
+.I mode
+is
+.BR FALLOC_FL_COLLAPSE_RANGE ,
+but either
+.I offset
+or
+.I len
+is not a multiple of the filesystem block size.
+.TP
+.B EINVAL
+mode contains both
+.B FALLOC_FL_COLLAPSE_RANGE
+and other flags;
+no other flags are permitted with
+.BR FALLOC_FL_COLLAPSE_RANGE .
+.\" FIXME Document error when FALLOC_FL_COLLAPSE_RANGE
+.\" is given a nonregular file. As at 3.15-rc1, XFS gives
+.\" EINVAL, but ext4 gives EOPNOTSUPP.
+.TP
 .B EIO
 An I/O error occurred while reading from or writing to a filesystem.
 .TP
@@ -193,6 +279,8 @@ Or:
 .I mode
 specifies
 .BR FALLOC_FL_PUNCH_HOLE
+or
+.BR FALLOC_FL_COLLAPSE_RANGE
 and
 the file referred to by
 .I fd
@@ -203,6 +291,14 @@ is marked append-only
 .B ESPIPE
 .I fd
 refers to a pipe or FIFO.
+.TP
+.B ETXTBSY
+.I mode
+specifies
+.BR FALLOC_FL_COLLAPSE_RANGE ,
+but the file referred to by
+.IR fd
+is currently being executed.
 .SH VERSIONS
 .BR fallocate ()
 is available on Linux since kernel 2.6.23.


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Christoph Hellwig]

On Thu, Apr 17, 2014 at 03:40:05PM +0200, Michael Kerrisk (man-pages) wrote:
> On 04/16/2014 08:05 AM, Christoph Hellwig wrote:
> > Michael, can you apply this now that we merged FALLOC_FL_COLLAPSE_RANGE
> > into the kernel tree?
> 
> I've applied Namjae Jeon's patch (thanks Namjae!), and done a lot of
> tweaking to generate the patch below.
> 
> Along the way I noticed that there is an inconsistency between XFS
> and JFS that should be fixed (and so have added Dave and Ted to CC,

ext4?

Anyway, the documentation looks good to in general, but do you really think
mentioning git commit hashs in a man page is a good idea?

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Michael Kerrisk (man-pages)]

On Thu, Apr 17, 2014 at 5:59 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> On Thu, Apr 17, 2014 at 03:40:05PM +0200, Michael Kerrisk (man-pages) wrote:
>> On 04/16/2014 08:05 AM, Christoph Hellwig wrote:
>> > Michael, can you apply this now that we merged FALLOC_FL_COLLAPSE_RANGE
>> > into the kernel tree?
>>
>> I've applied Namjae Jeon's patch (thanks Namjae!), and done a lot of
>> tweaking to generate the patch below.
>>
>> Along the way I noticed that there is an inconsistency between XFS
>> and JFS that should be fixed (and so have added Dave and Ted to CC,
>
> ext4?

D'oh -- yes!

> Anyway, the documentation looks good to in general, but do you really think
> mentioning git commit hashs in a man page is a good idea?

Those are only comments in the source file, for reference (sometimes,
that's really helpful). They don't appear the rendered output.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Christoph Hellwig]

On Thu, Apr 17, 2014 at 10:11:06PM +0200, Michael Kerrisk (man-pages) wrote:
> Those are only comments in the source file, for reference (sometimes,
> that's really helpful). They don't appear the rendered output.

Ok, that makes a lot more sense.

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Dave Chinner]

On Thu, Apr 17, 2014 at 03:40:05PM +0200, Michael Kerrisk (man-pages) wrote:
> On 04/16/2014 08:05 AM, Christoph Hellwig wrote:
> > Michael, can you apply this now that we merged FALLOC_FL_COLLAPSE_RANGE
> > into the kernel tree?
> 
> I've applied Namjae Jeon's patch (thanks Namjae!), and done a lot of
> tweaking to generate the patch below.
> 
> Along the way I noticed that there is an inconsistency between XFS
> and JFS that should be fixed (and so have added Dave and Ted to CC,
> as well as linux-fsdevel@). 
> 
> If 'fd' does not refer to a regular file, then XFS fails with the 
> error EINVAL (fs/xfs/xfs_file.c::xfs_file_fallocate()), but ext4 
> fails with the error EOPNOTSUP
> (fs/ext4/extents::ext4_collapse_range()). I suspect that EINVAL
> is the right error for this case.

I think EINVAL is correct here - we support the syscall, just the
type of file we are being asked to operate on is invalid.

FWIW, the reason this check is in the filesystems is that there is
no theoretical reason why we can't do things like preallocation for
directories (e.g. to reduce fragmentation as they grow). It's just
not implemented by any filesystem yet.

[ Hmmmm -  I just had a great idea - removing entire directory
contents via punching blocks. No more "remove one dirent at a time"
for rm -rf, just punch the directory from start to end and there
goes millions of directory entries in a single syscall and a handful
of transactions.  FALLOC_FL_DIR_PUNCH, anyone? ]

> I'd appreciate review on my revised patch, below.

Looks fine to me.

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Lukáš Czerner]

On Fri, 18 Apr 2014, Dave Chinner wrote:

> Date: Fri, 18 Apr 2014 08:57:01 +1000
> From: Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org>
> To: "Michael Kerrisk (man-pages)" <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>, Namjae Jeon <linkinjeon-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
>     linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Theodore T'so <tytso-3s7WtUTddSA@public.gmane.org>,
>     Linux-Fsdevel <linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Subject: Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in
>      fallocate
> 
> On Thu, Apr 17, 2014 at 03:40:05PM +0200, Michael Kerrisk (man-pages) wrote:
> > On 04/16/2014 08:05 AM, Christoph Hellwig wrote:
> > > Michael, can you apply this now that we merged FALLOC_FL_COLLAPSE_RANGE
> > > into the kernel tree?
> > 
> > I've applied Namjae Jeon's patch (thanks Namjae!), and done a lot of
> > tweaking to generate the patch below.
> > 
> > Along the way I noticed that there is an inconsistency between XFS
> > and JFS that should be fixed (and so have added Dave and Ted to CC,
> > as well as linux-fsdevel@). 
> > 
> > If 'fd' does not refer to a regular file, then XFS fails with the 
> > error EINVAL (fs/xfs/xfs_file.c::xfs_file_fallocate()), but ext4 
> > fails with the error EOPNOTSUP
> > (fs/ext4/extents::ext4_collapse_range()). I suspect that EINVAL
> > is the right error for this case.
> 
> I think EINVAL is correct here - we support the syscall, just the
> type of file we are being asked to operate on is invalid.

I'd like to point out that do_fallocate() in vfs uses ENODEV in the
case it is not regular file nor directory.

Also I thought of the EOPNOTSUP to be the right way to go. For
example we do not support it on non-extent based files in ext4 so we
return EOPNOTSUP.

The same is for the case that we do not support the particular
fallocate mode.

So I am not really sure about what's the right error to use.

Thanks!
-Lukas

> 
> FWIW, the reason this check is in the filesystems is that there is
> no theoretical reason why we can't do things like preallocation for
> directories (e.g. to reduce fragmentation as they grow). It's just
> not implemented by any filesystem yet.
> 
> [ Hmmmm -  I just had a great idea - removing entire directory
> contents via punching blocks. No more "remove one dirent at a time"
> for rm -rf, just punch the directory from start to end and there
> goes millions of directory entries in a single syscall and a handful
> of transactions.  FALLOC_FL_DIR_PUNCH, anyone? ]
> 
> > I'd appreciate review on my revised patch, below.
> 
> Looks fine to me.
> 
> Cheers,
> 
> Dave.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Theodore Ts'o]

On Fri, Apr 18, 2014 at 11:45:35AM +0200, Lukáš Czerner wrote:
> 
> I'd like to point out that do_fallocate() in vfs uses ENODEV in the
> case it is not regular file nor directory.
> 
> Also I thought of the EOPNOTSUP to be the right way to go. For
> example we do not support it on non-extent based files in ext4 so we
> return EOPNOTSUP.
> 
> The same is for the case that we do not support the particular
> fallocate mode.
> 
> So I am not really sure about what's the right error to use.

Yes, there is some inconsistency here.  I'll change this particular
case (a non-REGULAR file to use EINVAL) to be consistent with xfs, but
I suspect programs are going to have to be flexible.

As far as non-extent based files, at least in theory at some point we
could support COLLAPSE_RANGE on regular files, so I'll leave it as
EOPNOTSUPP for now, but the choice is admittedly somewhat arbitrary.

Cheers,

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Michael Kerrisk (man-pages)]

On 04/18/2014 05:55 PM, Theodore Ts'o wrote:
> On Fri, Apr 18, 2014 at 11:45:35AM +0200, Lukáš Czerner wrote:
>>
>> I'd like to point out that do_fallocate() in vfs uses ENODEV in the
>> case it is not regular file nor directory.
>>
>> Also I thought of the EOPNOTSUP to be the right way to go. For
>> example we do not support it on non-extent based files in ext4 so we
>> return EOPNOTSUP.
>>
>> The same is for the case that we do not support the particular
>> fallocate mode.
>>
>> So I am not really sure about what's the right error to use.
> 
> Yes, there is some inconsistency here.  I'll change this particular
> case (a non-REGULAR file to use EINVAL) to be consistent with xfs, but
> I suspect programs are going to have to be flexible.

Okay -- I'll document EINVAL as the (sole) error for this case.

Thanks, Ted.

> As far as non-extent based files, at least in theory at some point we
> could support COLLAPSE_RANGE on regular files, so I'll leave it as
> EOPNOTSUPP for now, but the choice is admittedly somewhat arbitrary.

Agreed that its somewhat inconsistent, though not as egregiously so as
the other case.

Cheers,

Michael




-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 10/10] manpage: update FALLOC_FL_COLLAPSE_RANGE flag in fallocate

[Michael Kerrisk (man-pages)]

On 04/18/2014 12:57 AM, Dave Chinner wrote:
> On Thu, Apr 17, 2014 at 03:40:05PM +0200, Michael Kerrisk (man-pages) wrote:
>> On 04/16/2014 08:05 AM, Christoph Hellwig wrote:
>>> Michael, can you apply this now that we merged FALLOC_FL_COLLAPSE_RANGE
>>> into the kernel tree?
>>
>> I've applied Namjae Jeon's patch (thanks Namjae!), and done a lot of
>> tweaking to generate the patch below.
>>
>> Along the way I noticed that there is an inconsistency between XFS
>> and JFS that should be fixed (and so have added Dave and Ted to CC,
>> as well as linux-fsdevel@). 
>>
>> If 'fd' does not refer to a regular file, then XFS fails with the 
>> error EINVAL (fs/xfs/xfs_file.c::xfs_file_fallocate()), but ext4 
>> fails with the error EOPNOTSUP
>> (fs/ext4/extents::ext4_collapse_range()). I suspect that EINVAL
>> is the right error for this case.
> 
> I think EINVAL is correct here - we support the syscall, just the
> type of file we are being asked to operate on is invalid.

Hmmm -- you and Lukasz seem to differ on this point.

> FWIW, the reason this check is in the filesystems is that there is
> no theoretical reason why we can't do things like preallocation for
> directories (e.g. to reduce fragmentation as they grow). It's just
> not implemented by any filesystem yet.
> 
> [ Hmmmm -  I just had a great idea - removing entire directory
> contents via punching blocks. No more "remove one dirent at a time"
> for rm -rf, just punch the directory from start to end and there
> goes millions of directory entries in a single syscall and a handful
> of transactions.  FALLOC_FL_DIR_PUNCH, anyone? ]

Interesting idea. CC me and linux-api on the patch, if you do one.

>> I'd appreciate review on my revised patch, below.
> 
> Looks fine to me.

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html