[PATCH] mkfs: default log size for small filesystems too large

[PATCH] mkfs: default log size for small filesystems too large

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Recent changes to the log size scaling have resulted in using the
default size multiplier for the log size even on small filesystems.
Commit 88cd79b ("xfs: Add xfs_log_rlimit.c") changed the calculation
of the maximum transaction size that the kernel would issues and
that significantly increased the minimum size of the default log.
As such the size of the log on small filesystems was typically
larger than the prefious default, even though the previous default
was still larger than the minimum needed.

Rework the default log size calculation such that it will use the
original log size default if it is larger than the minimum log size
required, and only use a larger log if the configuration of the
filesystem requires it.

This is especially obvious in xfs/216, where the default log size is
10MB all the way up to 16GB filesystems. The current mkfs selects a
log size of 50MB for the same size filesystems and this is
unnecessarily large.

Return the scaling of the log size for small filesystems to
something similar to what xfs/216 expects.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 mkfs/xfs_mkfs.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index d82128c..4a29eea 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2377,17 +2377,18 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
 		logblocks = MAX(min_logblocks, logblocks);
 
 		/*
-		 * If the default log size doesn't fit in the AG size, use the
-		 * minimum log size instead. This ensures small filesystems
-		 * don't use excessive amounts of space for the log.
+		 * For small filesystems, we want to use the XFS_MIN_LOG_BYTES
+		 * for filesystems smaller than 16G if at all possible, ramping
+		 * up to 128MB at 256GB.
 		 */
-		if (min_logblocks * XFS_DFL_LOG_FACTOR >= agsize) {
-			logblocks = min_logblocks;
-		} else {
-			logblocks = MAX(logblocks,
-				MAX(XFS_DFL_LOG_SIZE,
-					min_logblocks * XFS_DFL_LOG_FACTOR));
+		 if (dblocks < GIGABYTES(16, blocklog)) {
+			logblocks = MIN(XFS_MIN_LOG_BYTES >> blocklog,
+					min_logblocks * XFS_DFL_LOG_FACTOR);
 		}
+
+		if (logblocks >= agsize)
+			logblocks = min_logblocks;
+
 		logblocks = MIN(logblocks, XFS_MAX_LOG_BLOCKS);
 		if ((logblocks << blocklog) > XFS_MAX_LOG_BYTES) {
 			logblocks = XFS_MAX_LOG_BYTES >> blocklog;
-- 
1.8.4.rc3

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] mkfs: default log size for small filesystems too large

[Brian Foster]

On 02/05/2014 12:47 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Recent changes to the log size scaling have resulted in using the
> default size multiplier for the log size even on small filesystems.
> Commit 88cd79b ("xfs: Add xfs_log_rlimit.c") changed the calculation
> of the maximum transaction size that the kernel would issues and
> that significantly increased the minimum size of the default log.
> As such the size of the log on small filesystems was typically
> larger than the prefious default, even though the previous default
		previous
> was still larger than the minimum needed.
> 

Hey Dave,

Can you elaborate on what you mean by the previous default being larger
than the minimum needed? If that is the case, doesn't that mean the
calculations based on the max transaction size are not valid? Perhaps
I'm not parsing something here.

> Rework the default log size calculation such that it will use the
> original log size default if it is larger than the minimum log size
> required, and only use a larger log if the configuration of the
> filesystem requires it.
> 
> This is especially obvious in xfs/216, where the default log size is
> 10MB all the way up to 16GB filesystems. The current mkfs selects a
> log size of 50MB for the same size filesystems and this is
> unnecessarily large.
> 
> Return the scaling of the log size for small filesystems to
> something similar to what xfs/216 expects.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  mkfs/xfs_mkfs.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index d82128c..4a29eea 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2377,17 +2377,18 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
>  		logblocks = MAX(min_logblocks, logblocks);
>  
>  		/*
> -		 * If the default log size doesn't fit in the AG size, use the
> -		 * minimum log size instead. This ensures small filesystems
> -		 * don't use excessive amounts of space for the log.
> +		 * For small filesystems, we want to use the XFS_MIN_LOG_BYTES
> +		 * for filesystems smaller than 16G if at all possible, ramping
> +		 * up to 128MB at 256GB.
>  		 */
> -		if (min_logblocks * XFS_DFL_LOG_FACTOR >= agsize) {
> -			logblocks = min_logblocks;
> -		} else {
> -			logblocks = MAX(logblocks,
> -				MAX(XFS_DFL_LOG_SIZE,
> -					min_logblocks * XFS_DFL_LOG_FACTOR));
> +		 if (dblocks < GIGABYTES(16, blocklog)) {
> +			logblocks = MIN(XFS_MIN_LOG_BYTES >> blocklog,
> +					min_logblocks * XFS_DFL_LOG_FACTOR);
>  		}

Nit: extra space after tab before the 'if (dblocks < GIGABYTES(...)) {'
line...

More generally... by the time we get here, min_logblocks is at least
XFS_MIN_LOG_BLOCKS and XFS_MIN_LOG_BYTES (if the fs is >=1GB). The only
way we would use the min_logblocks based value is if min_logblocks is
less than 1/5 of XFS_MIN_LOG_BYTES (due to DFL_LOG_FACTOR). After
testing this a bit, creating a 20MB fs with 4k blocks gives me an
initial min_logblocks of 853, which works out to ~16MB after
DFL_LOG_FACTOR. So this effectively looks like an assignment of
XFS_MIN_LOG_BYTES in that case.

In the sub 1GB case, we skip the existing XFS_MIN_LOG_BYTES check, but
this new block of code just adds it back, at least in the internal log case.

Given that, I wonder if this can all be cleaned up to start with some
combination of the calculated min_logblocks and defined min blocks/bytes
values, and then add the 2048:1 scaling conditionally in the >=16GB
case. E.g., I modified the MIN() statement this patch adds to a straight
assignment of min_logblocks and xfs/216 still passes.

Thoughts? Would that be sufficient here, or am I missing some other
scenarios?

Brian

> +
> +		if (logblocks >= agsize)
> +			logblocks = min_logblocks;
> +
>  		logblocks = MIN(logblocks, XFS_MAX_LOG_BLOCKS);
>  		if ((logblocks << blocklog) > XFS_MAX_LOG_BYTES) {
>  			logblocks = XFS_MAX_LOG_BYTES >> blocklog;
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] mkfs: default log size for small filesystems too large

[Dave Chinner]

On Wed, Feb 05, 2014 at 02:29:26PM -0500, Brian Foster wrote:
> On 02/05/2014 12:47 AM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Recent changes to the log size scaling have resulted in using the
> > default size multiplier for the log size even on small filesystems.
> > Commit 88cd79b ("xfs: Add xfs_log_rlimit.c") changed the calculation
> > of the maximum transaction size that the kernel would issues and
> > that significantly increased the minimum size of the default log.
> > As such the size of the log on small filesystems was typically
> > larger than the prefious default, even though the previous default
> 		previous
> > was still larger than the minimum needed.
> > 
> 
> Hey Dave,
> 
> Can you elaborate on what you mean by the previous default being larger
> than the minimum needed? If that is the case, doesn't that mean the
> calculations based on the max transaction size are not valid? Perhaps
> I'm not parsing something here.

The previous change for calculating the minimum log size also
changed the default log size at the same time. i.e. it changed it to
min_logblocks * XFS_DFL_LOG_FACTOR if that fit inside an AG.

Here's an example with small AGs (25MB). Current mkfs:

# mkfs.xfs -f -d size=100m /dev/vdc |grep log
log      =internal log           bsize=4096   blocks=4265, version=2

That's a 17MB log in a 100MB filesystem. Way too large, and here's
what prompted this patch:

# mkfs.xfs -f -d size=100m,sunit=512,swidth=2048 /dev/vdc |grep log
log      =internal log           bsize=4096   blocks=1728, version=2

i.e add stripe unit alignment, and the log gets 3x smaller. Say
what?

The new mkfs gives:

# ~/packages/mkfs.xfs -f -d size=100m /dev/vdc |grep log
log      =internal log           bsize=4096   blocks=2560, version=2
# ~/packages/mkfs.xfs -f -d size=100m,sunit=512,swidth=2048 /dev/vdc |grep log
log      =internal log           bsize=4096   blocks=2560, version=2

a 10MB log, which is consistent but arguably still too large for a
100MB filesystem.

So, what did we used to do in the 3.1.x series?

# apt-get install xfsprogs/stable
<installs 3.1.7>
# mkfs.xfs -f -d size=100m /dev/vdc |grep log
log      =internal log           bsize=4096   blocks=1200, version=2
# mkfs.xfs -f -d size=100m,sunit=512,swidth=2048 /dev/vdc |grep log
log      =internal log           bsize=4096   blocks=1216, version=2

Ok, so historically it's been min_logblock sized....

> > Rework the default log size calculation such that it will use the
> > original log size default if it is larger than the minimum log size
> > required, and only use a larger log if the configuration of the
> > filesystem requires it.
> > 
> > This is especially obvious in xfs/216, where the default log size is
> > 10MB all the way up to 16GB filesystems. The current mkfs selects a
> > log size of 50MB for the same size filesystems and this is
> > unnecessarily large.
> > 
> > Return the scaling of the log size for small filesystems to
> > something similar to what xfs/216 expects.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  mkfs/xfs_mkfs.c | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index d82128c..4a29eea 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -2377,17 +2377,18 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
> >  		logblocks = MAX(min_logblocks, logblocks);
> >  
> >  		/*
> > -		 * If the default log size doesn't fit in the AG size, use the
> > -		 * minimum log size instead. This ensures small filesystems
> > -		 * don't use excessive amounts of space for the log.
> > +		 * For small filesystems, we want to use the XFS_MIN_LOG_BYTES
> > +		 * for filesystems smaller than 16G if at all possible, ramping
> > +		 * up to 128MB at 256GB.
> >  		 */
> > -		if (min_logblocks * XFS_DFL_LOG_FACTOR >= agsize) {
> > -			logblocks = min_logblocks;
> > -		} else {
> > -			logblocks = MAX(logblocks,
> > -				MAX(XFS_DFL_LOG_SIZE,
> > -					min_logblocks * XFS_DFL_LOG_FACTOR));
> > +		 if (dblocks < GIGABYTES(16, blocklog)) {
> > +			logblocks = MIN(XFS_MIN_LOG_BYTES >> blocklog,
> > +					min_logblocks * XFS_DFL_LOG_FACTOR);
> >  		}
> 
> Nit: extra space after tab before the 'if (dblocks < GIGABYTES(...)) {'
> line...

Oops. Will fix.

> More generally... by the time we get here, min_logblocks is at least
> XFS_MIN_LOG_BLOCKS and XFS_MIN_LOG_BYTES (if the fs is >=1GB). The only
> way we would use the min_logblocks based value is if min_logblocks is
> less than 1/5 of XFS_MIN_LOG_BYTES (due to DFL_LOG_FACTOR). After
> testing this a bit, creating a 20MB fs with 4k blocks gives me an
> initial min_logblocks of 853, which works out to ~16MB after
> DFL_LOG_FACTOR. So this effectively looks like an assignment of
> XFS_MIN_LOG_BYTES in that case.

Good point - I hadn't considered the initial >1GB check much further
up the stack, and that explains the difference between my patch and
the 3.1.x code.

> In the sub 1GB case, we skip the existing XFS_MIN_LOG_BYTES check, but
> this new block of code just adds it back, at least in the internal log case.

Right, this would handle it, I think.

		if (dblocks < GIGABYTES(1, blocklog)) {
			logblocks = min_logblocks;
		else if (dblocks < GIGABYTES(16, blocklog)) {
			logblocks = MIN(XFS_MIN_LOG_BYTES >> blocklog,
					min_logblocks * XFS_DFL_LOG_FACTOR);
		} else {
			/*
			 * With a 2GB max log size, default to maximum size
			 * at 4TB. This keeps the same ratio from the older
			 * max log size of 128M at 256GB fs size. IOWs,
			 * the ratio of fs size to log size is 2048:1.
			 */
			logblocks = (dblocks << blocklog) / 2048;
			logblocks = logblocks >> blocklog;
			logblocks = MAX(min_logblocks, logblocks);
		}

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs