mkfs: enable reflink and rmap by default
diff mbox series

Message ID 20190503005717.GO5207@magnolia
State New
Headers show
Series
  • mkfs: enable reflink and rmap by default
Related show

Commit Message

Darrick J. Wong May 3, 2019, 12:57 a.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

Enable reflink and reverse mapping by default.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Eric Sandeen May 3, 2019, 3:55 p.m. UTC | #1
On 5/2/19 7:57 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Enable reflink and reverse mapping by default.

Soooo until now, I thought I had understood that rmap was unlikely
to be default ever, due to performance impacts.  Is this no longer the case?
Of course I can't find any emails or IRC logs to indicate I didn't
just dream this up...

Also, at this point, remind me again what uses rmap and why it should
be enabled now?

-Eric

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  mkfs/xfs_mkfs.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 0862621a..3874f7dd 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2021,14 +2021,14 @@ _("sparse inodes not supported without CRC support\n"));
>  		}
>  		cli->sb_feat.spinodes = false;
>  
> -		if (cli->sb_feat.rmapbt) {
> +		if (cli->sb_feat.rmapbt && cli_opt_set(&mopts, M_RMAPBT)) {
>  			fprintf(stderr,
>  _("rmapbt not supported without CRC support\n"));
>  			usage();
>  		}
>  		cli->sb_feat.rmapbt = false;
>  
> -		if (cli->sb_feat.reflink) {
> +		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
>  			fprintf(stderr,
>  _("reflink not supported without CRC support\n"));
>  			usage();
> @@ -3934,8 +3934,8 @@ main(
>  			.dirftype = true,
>  			.finobt = true,
>  			.spinodes = true,
> -			.rmapbt = false,
> -			.reflink = false,
> +			.rmapbt = true,
> +			.reflink = true,
>  			.parent_pointers = false,
>  			.nodalign = false,
>  			.nortalign = false,
>
Darrick J. Wong May 3, 2019, 9:49 p.m. UTC | #2
On Fri, May 03, 2019 at 10:55:05AM -0500, Eric Sandeen wrote:
> On 5/2/19 7:57 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Enable reflink and reverse mapping by default.
> 
> Soooo until now, I thought I had understood that rmap was unlikely
> to be default ever, due to performance impacts.  Is this no longer the case?

No, it's still the case.

> Of course I can't find any emails or IRC logs to indicate I didn't
> just dream this up...
> 
> Also, at this point, remind me again what uses rmap and why it should
> be enabled now?

TBH it's most useful for enhancing xfs_scrub's powers, so there's no
reason to enable it by default until at least 5.2 when we finally land
the last of the scrubbers.  Probably.  One could make a solid argument
that we ought to wait until the scrub/health code has been stable for
~6mo before even thinking about rmap by default.

Anyway I'll break this into separate pieces and resend the reflink part
for 5.1.

--D

> 
> -Eric
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  mkfs/xfs_mkfs.c |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 0862621a..3874f7dd 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -2021,14 +2021,14 @@ _("sparse inodes not supported without CRC support\n"));
> >  		}
> >  		cli->sb_feat.spinodes = false;
> >  
> > -		if (cli->sb_feat.rmapbt) {
> > +		if (cli->sb_feat.rmapbt && cli_opt_set(&mopts, M_RMAPBT)) {
> >  			fprintf(stderr,
> >  _("rmapbt not supported without CRC support\n"));
> >  			usage();
> >  		}
> >  		cli->sb_feat.rmapbt = false;
> >  
> > -		if (cli->sb_feat.reflink) {
> > +		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
> >  			fprintf(stderr,
> >  _("reflink not supported without CRC support\n"));
> >  			usage();
> > @@ -3934,8 +3934,8 @@ main(
> >  			.dirftype = true,
> >  			.finobt = true,
> >  			.spinodes = true,
> > -			.rmapbt = false,
> > -			.reflink = false,
> > +			.rmapbt = true,
> > +			.reflink = true,
> >  			.parent_pointers = false,
> >  			.nodalign = false,
> >  			.nortalign = false,
> >

Patch
diff mbox series

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 0862621a..3874f7dd 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2021,14 +2021,14 @@  _("sparse inodes not supported without CRC support\n"));
 		}
 		cli->sb_feat.spinodes = false;
 
-		if (cli->sb_feat.rmapbt) {
+		if (cli->sb_feat.rmapbt && cli_opt_set(&mopts, M_RMAPBT)) {
 			fprintf(stderr,
 _("rmapbt not supported without CRC support\n"));
 			usage();
 		}
 		cli->sb_feat.rmapbt = false;
 
-		if (cli->sb_feat.reflink) {
+		if (cli->sb_feat.reflink && cli_opt_set(&mopts, M_REFLINK)) {
 			fprintf(stderr,
 _("reflink not supported without CRC support\n"));
 			usage();
@@ -3934,8 +3934,8 @@  main(
 			.dirftype = true,
 			.finobt = true,
 			.spinodes = true,
-			.rmapbt = false,
-			.reflink = false,
+			.rmapbt = true,
+			.reflink = true,
 			.parent_pointers = false,
 			.nodalign = false,
 			.nortalign = false,