btrfs-progs: fi defrag: change default extent target size to 32 MiB

Commit Message

David Sterba July 28, 2016, 11:17 a.m. UTC

The kernel default is too low, 32 MiB is recommended and should give
better results.

Signed-off-by: David Sterba <[e-mail]>
---
 Documentation/btrfs-filesystem.asciidoc | 16 ++++++++++++----
 cmds-filesystem.c                       | 11 +++++++++--
 2 files changed, 21 insertions(+), 6 deletions(-)

Comments

Graham Cobb July 28, 2016, 11:28 a.m. UTC | #1

On 28/07/16 12:17, David Sterba wrote:
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index ef1f550b51c0..6b381c582ea7 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -968,7 +968,7 @@ static const char * const cmd_filesystem_defrag_usage[] = {
>  	"-f             flush data to disk immediately after defragmenting",
>  	"-s start       defragment only from byte onward",
>  	"-l len         defragment only up to len bytes",
> -	"-t size        target extent size hint",
> +	"-t size        target extent size hint (default: 32 MiB)",

As a user... might it be better to say the default is 32M as that is the
format the option requires?

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Sterba July 28, 2016, 11:35 a.m. UTC | #2

On Thu, Jul 28, 2016 at 12:28:27PM +0100, Graham Cobb wrote:
> On 28/07/16 12:17, David Sterba wrote:
> > diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> > index ef1f550b51c0..6b381c582ea7 100644
> > --- a/cmds-filesystem.c
> > +++ b/cmds-filesystem.c
> > @@ -968,7 +968,7 @@ static const char * const cmd_filesystem_defrag_usage[] = {
> >  	"-f             flush data to disk immediately after defragmenting",
> >  	"-s start       defragment only from byte onward",
> >  	"-l len         defragment only up to len bytes",
> > -	"-t size        target extent size hint",
> > +	"-t size        target extent size hint (default: 32 MiB)",
> 
> As a user... might it be better to say the default is 32M as that is the
> format the option requires?

Good point, thanks, I'll fix that.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/btrfs-filesystem.asciidoc b/Documentation/btrfs-filesystem.asciidoc
index 8fb15eacafc9..28e772466cfa 100644
--- a/Documentation/btrfs-filesystem.asciidoc
+++ b/Documentation/btrfs-filesystem.asciidoc
@@ -117,15 +117,23 @@  compression. See also section 'EXAMPLES'.
 -r::::
 defragment files recursively in given directories
 -f::::
-flush data for each file before going to the next file. This will limit the amount
-of dirty data to current file, otherwise the amount cumulates from several files
-and may increase system load.
+flush data for each file before going to the next file.
++
+This will limit the amount of dirty data to current file, otherwise the amount
+cumulates from several files and will increase system load. This can also lead
+to ENOSPC if there's too much dirty data to write and it's not possible to make
+the reservations for the new data (ie. how the COW design works).
++
 -s <start>[kKmMgGtTpPeE]::::
 defragmentation will start from the given offset, default is beginning of a file
 -l <len>[kKmMgGtTpPeE]::::
 defragment only up to 'len' bytes, default is the file size
 -t <size>[kKmMgGtTpPeE]::::
-target extent size, do not touch extents bigger than 'size'
+target extent size, do not touch extents bigger than 'size', default: 32 MiB
++
+The value is only advisory and the final size of the extents may differ,
+depending on the state of the free space and fragmentation or other internal
+logic. Reasonable values are from tens to hundreds of megabytes.
 
 *du* [options] <path> [<path>..]::
 Calculate disk usage of the target files using FIEMAP. For individual
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index ef1f550b51c0..6b381c582ea7 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -968,7 +968,7 @@  static const char * const cmd_filesystem_defrag_usage[] = {
 	"-f             flush data to disk immediately after defragmenting",
 	"-s start       defragment only from byte onward",
 	"-l len         defragment only up to len bytes",
-	"-t size        target extent size hint",
+	"-t size        target extent size hint (default: 32 MiB)",
 	NULL
 };
 
@@ -1029,7 +1029,7 @@  static int cmd_filesystem_defrag(int argc, char **argv)
 	int flush = 0;
 	u64 start = 0;
 	u64 len = (u64)-1;
-	u64 thresh = 0;
+	u64 thresh;
 	int i;
 	int recursive = 0;
 	int ret = 0;
@@ -1037,6 +1037,13 @@  static int cmd_filesystem_defrag(int argc, char **argv)
 	int compress_type = BTRFS_COMPRESS_NONE;
 	DIR *dirstream;
 
+	/*
+	 * Kernel has a different default (256K) that is supposed to be safe,
+	 * but it does not defragment very well. The 32M will likely lead to
+	 * better results and is independent of the kernel default.
+	 */
+	thresh = 32 * 1024 * 1024;
+
 	defrag_global_errors = 0;
 	defrag_global_verbose = 0;
 	defrag_global_errors = 0;