[PATCH] resize2fs: fix over-pessimistic heuristic.

[PATCH] resize2fs: fix over-pessimistic heuristic.

[Dmitry Monakhov]

In worst case we need one extent per moved block. Number of blocks to
be moved is less or equals to blks_needed.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 resize/resize2fs.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index f5f1337..8ca53a3 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -2587,11 +2587,17 @@ blk64_t calculate_minimum_resize_size(ext2_filsys fs, int flags)
 	/*
 	 * We need to reserve a few extra blocks if extents are
 	 * enabled, in case we need to grow the extent tree.  The more
-	 * we shrink the file system, the more space we need.
+	 * we shrink the file system, the more blocks will be moved.
+	 * Worse case is one extent per moved block.
 	 */
 	if (fs->super->s_feature_incompat & EXT3_FEATURE_INCOMPAT_EXTENTS) {
-		blk64_t safe_margin = (ext2fs_blocks_count(fs->super) -
-				       blks_needed)/500;
+		blk64_t safe_margin = ext2fs_blocks_count(fs->super)
+			- blks_needed;
+
+		if (safe_margin > blks_needed)
+			safe_margin = blks_needed;
+		safe_margin /= 500;
+
 #ifdef RESIZE2FS_DEBUG
 		if (flags & RESIZE_DEBUG_MIN_CALC)
 			printf("Extents safety margin: %llu\n", safe_margin);
-- 
1.7.1

Re: [PATCH] resize2fs: fix over-pessimistic heuristic.

[Theodore Ts'o]

On Sun, Apr 06, 2014 at 04:56:05PM +0400, Dmitry Monakhov wrote:
> In worst case we need one extent per moved block. Number of blocks to
> be moved is less or equals to blks_needed.

I can believe that the original safety margin of 0.2% of how much we
will be shrinking the file system might have been overly pessimistic.

However I'm concerned that your change might be over-optimistic.  For
example, if we are shrinking the file system down to under 500 blocks,
then the safety_margin will be 0 --- but it's could very easily be the
case that an extent tree will need to grow.

I'd accept this change, although I don't know it would make a
difference in the cases you are concerned about:

		blk64_t safe_margin = (ext2fs_blocks_count(fs->super) -
				       blks_needed)/500;
		if (safe_margin > blks_needed)
			safe_margin = blks_needed;

How and why are you using this?  I've never been all that exicited
about the -M option, since it's been abused in so many different ways,
and the result when you compress the file system that significantly is
often no good for performance.

					- Ted

Re: [PATCH] resize2fs: fix over-pessimistic heuristic.

[Dmitry Monakhov]

On Thu, 10 Apr 2014 23:39:15 -0400, Theodore Ts'o <tytso@mit.edu> wrote:
> On Sun, Apr 06, 2014 at 04:56:05PM +0400, Dmitry Monakhov wrote:
> > In worst case we need one extent per moved block. Number of blocks to
> > be moved is less or equals to blks_needed.
> 
> I can believe that the original safety margin of 0.2% of how much we
> will be shrinking the file system might have been overly pessimistic.
> 
> However I'm concerned that your change might be over-optimistic.  For
> example, if we are shrinking the file system down to under 500 blocks,
> then the safety_margin will be 0 --- but it's could very easily be the
> case that an extent tree will need to grow.
> 
> I'd accept this change, although I don't know it would make a
> difference in the cases you are concerned about:
> 
> 		blk64_t safe_margin = (ext2fs_blocks_count(fs->super) -
> 				       blks_needed)/500;
> 		if (safe_margin > blks_needed)
> 			safe_margin = blks_needed;
> 
> How and why are you using this? 
We have test-suite for ploop/resize2fs test which try to cover most dark
corners of online resize2fs and offline shrink. Test appeared to be
useful and ideally it should be pushed to xfsetsts, but the way it was
written on python and has too many Parallels speciffic API stuff (instead
of generic 'resize2fs /$DEV $SIZE' it call 'vzctl set $CONTAINERID $SIZE' )
I have plans to rewrite this test according to xfstest rules and submit
it someday(next month or later). One of subtests looks like follows:
# Test that we can grow filesystem up to the limit, and shrink it back.
mke2fs $dev -Eresize=$((~0)) 10G
mount $dev $mnt
# fill filesystem with containers info (~750Mb)
tar -zxf container.tar.gz $mnt
# online grow to maximum size
resize2fs $dev 16T
# Perform online shrink back to original size
umount $dev $mnt
resize2fs $dev 10G
## Last command will fail  because safity_margin = 32G
> I've never been all that exicited
> about the -M option, since it's been abused in so many different ways,
> and the result when you compress the file system that significantly is
> often no good for performance.
That is true. But setting safe_margin to 50% of space IMHO is too
pessimistic, because filesystem becomes unusable from 90-95%
Even more when user ask to shrink filesytem to it's minimum size he
MUST realize that result filesystem will be size optimized in contrast
to performance. Since we have to introduce some heuristic I'll beat on 25%  :) 
               blk64_t safe_margin = (ext2fs_blocks_count(fs->super) -
                                      blks_needed)/500;
               if (safe_margin > blks_needed / 2)
                       safe_margin = blks_needed /2;

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