[PATCH v2] erofs-utils: mkfs: use extended inodes when ctime is set

[Gao Xiang]

Hi David,

On Fri, Mar 18, 2022 at 05:25:36AM +0000, David Anderson via Linux-erofs wrote:
> Currently ctime is effectively ignored because most inodes are compact.
> If ctime was set, and it's different from the build time, then extended
> inodes should be used instead.
> 

Thanks for your time on this!

I will update this slightly (use mtime rather than ctime), since I will
apply the other patches first...

> To guarantee that timestamps do not cause extended inodes, a fixed
> timestamp should be used (-T). Additionally, a new --ignore-mtime option
> has been added to preserve the old behavior.
> 
> Signed-off-by: David Anderson <dvander at google.com>
> ---
>  include/erofs/config.h | 1 +
>  lib/config.c           | 1 +
>  lib/inode.c            | 5 +++++
>  man/mkfs.erofs.1       | 5 +++++
>  mkfs/main.c            | 5 +++++
>  5 files changed, 17 insertions(+)
> 
> diff --git a/include/erofs/config.h b/include/erofs/config.h
> index cb064b6..0a1b18b 100644
> --- a/include/erofs/config.h
> +++ b/include/erofs/config.h
> @@ -43,6 +43,7 @@ struct erofs_configure {
>  	char c_timeinherit;
>  	char c_chunkbits;
>  	bool c_noinline_data;
> +	bool c_ignore_mtime;
>  
>  #ifdef HAVE_LIBSELINUX
>  	struct selabel_handle *sehnd;
> diff --git a/lib/config.c b/lib/config.c
> index f1c8edf..cc15e57 100644
> --- a/lib/config.c
> +++ b/lib/config.c
> @@ -20,6 +20,7 @@ void erofs_init_configure(void)
>  	cfg.c_dbg_lvl  = EROFS_WARN;
>  	cfg.c_version  = PACKAGE_VERSION;
>  	cfg.c_dry_run  = false;
> +	cfg.c_ignore_mtime = false;
>  	cfg.c_compr_level_master = -1;
>  	cfg.c_force_inodeversion = 0;
>  	cfg.c_inline_xattr_tolerance = 2;
> diff --git a/lib/inode.c b/lib/inode.c
> index 461c797..99a4b2f 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -730,6 +730,11 @@ static bool erofs_should_use_inode_extended(struct erofs_inode *inode)
>  		return true;
>  	if (inode->i_nlink > USHRT_MAX)
>  		return true;
> +	if ((inode->i_ctime != sbi.build_time ||
> +	     inode->i_ctime_nsec != sbi.build_time_nsec) &&
> +	    !cfg.c_ignore_mtime) {
> +		return true;
> +	}
>  	return false;
>  }
>  
> diff --git a/man/mkfs.erofs.1 b/man/mkfs.erofs.1
> index 9c7788e..679291b 100644
> --- a/man/mkfs.erofs.1
> +++ b/man/mkfs.erofs.1
> @@ -109,6 +109,11 @@ Set all file uids to \fIUID\fR.
>  .BI "\-\-force-gid=" GID
>  Set all file gids to \fIGID\fR.
>  .TP
> +.BI "\-\-ignore-mtime"
> +File modification time is ignored whenever it would cause \fBmkfs.erofs\fR to
> +use extended inodes over compact inodes. When not using a fixed timestamp,
> +this can reduce metadata size.
> +.TP
>  .B \-\-help
>  Display this help and exit.
>  .TP
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 3f34450..93caf67 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -49,6 +49,7 @@ static struct option long_options[] = {
>  	{"chunksize", required_argument, NULL, 11},
>  	{"quiet", no_argument, 0, 12},
>  	{"blobdev", required_argument, NULL, 13},
> +	{"ignore-mtime", no_argument, NULL, 14},
>  #ifdef WITH_ANDROID
>  	{"mount-point", required_argument, NULL, 512},
>  	{"product-out", required_argument, NULL, 513},
> @@ -95,6 +96,7 @@ static void usage(void)
>  #endif
>  	      " --force-uid=#         set all file uids to # (# = UID)\n"
>  	      " --force-gid=#         set all file gids to # (# = GID)\n"
> +	      " --ignore-mtime        use build time and ignore inode modification time\n"

				      Use build time and ignore inode modification time if possible

Since if extended inodes are really needed (e.g files more than 4GiB,
etc..), extended inodes are still necessary.

Otherwise looks good to me!
Reviewed-by: Gao Xiang <hsiangkao at linux.alibaba.com>

(Will apply together with other patches later...)

Thanks,
Gao Xiang

>  	      " --help                display this help and exit\n"
>  	      " --max-extent-bytes=#  set maximum decompressed extent size # in bytes\n"
>  	      " --quiet               quiet execution (do not write anything to standard output.)\n"
> @@ -366,6 +368,9 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
>  		case 13:
>  			cfg.c_blobdev_path = optarg;
>  			break;
> +		case 14:
> +			cfg.c_ignore_mtime = true;
> +			break;
>  		case 1:
>  			usage();
>  			exit(0);
> -- 
> 2.35.1.894.gb6a874cedc-goog