diff mbox series

ext2fs: don't read group descriptors during e2label

Message ID 20181128164238.2286-1-artem.blagodarenko@gmail.com
State Superseded
Headers show
Series ext2fs: don't read group descriptors during e2label | expand

Commit Message

Artem Blagodarenko Nov. 28, 2018, 4:42 p.m. UTC
tune2fs is used to make e2label duties.  ext2fs_open2() reads group
descriptors which are not used during disk label obtaining, but
takes a lot of time on large partitions.

This patch change ext2fs_open2(), so only initialized
superblock is returned. without block descriptors. This save
time dramatically.

Cray-bug-id: LUS-5777
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
---
 lib/ext2fs/ext2fs.h |  1 +
 lib/ext2fs/openfs.c | 22 +++++++++++++++++++---
 misc/tune2fs.c      |  7 ++++++-
 3 files changed, 26 insertions(+), 4 deletions(-)

Comments

Andreas Dilger Nov. 28, 2018, 7:07 p.m. UTC | #1
On Nov 28, 2018, at 9:42 AM, Artem Blagodarenko <artem.blagodarenko@gmail.com> wrote:
> 
> tune2fs is used to make e2label duties.  ext2fs_open2() reads group
> descriptors which are not used during disk label obtaining, but
> takes a lot of time on large partitions.
> 
> This patch change ext2fs_open2(), so only initialized
> superblock is returned. without block descriptors. This save
> time dramatically.
> 
> Cray-bug-id: LUS-5777
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
> ---
> lib/ext2fs/ext2fs.h |  1 +
> lib/ext2fs/openfs.c | 22 +++++++++++++++++++---
> misc/tune2fs.c      |  7 ++++++-
> 3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index 5b87d395..5318b14d 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -204,6 +204,7 @@ typedef struct ext2_file *ext2_file_t;
> #define EXT2_FLAG_IGNORE_CSUM_ERRORS	0x200000
> #define EXT2_FLAG_SHARE_DUP		0x400000
> #define EXT2_FLAG_IGNORE_SB_ERRORS	0x800000
> +#define EXT2_FLAG_LABEL			0x1000000

There are already a couple of similar flags, EXT2_FLAG_SUPER_ONLY,
but I guess that doesn't quite do the right thing either?

The only minor nit I have is that the flag name should describe what
it does, rather than what it is being used for.  For example, a better
name would be "EXT2_FLAG_JOURNAL_ONLY" or similar, so that other
users might use it, instead of thinking it is specific to the label.

Cheers, Andreas

> 
> /*
>  * Special flag in the ext2 inode i_flag field that means that this is
> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
> index 85d73e2a..a5e97e6d 100644
> --- a/lib/ext2fs/openfs.c
> +++ b/lib/ext2fs/openfs.c
> @@ -135,6 +135,7 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
> 	int		j;
> #endif
> 	char		*time_env;
> +	unsigned long	enough_desc_blocks;
> 
> 	EXT2_CHECK_MAGIC(manager, EXT2_ET_MAGIC_IO_MANAGER);
> 
> @@ -440,12 +441,23 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
> 		dest += fs->blocksize*first_meta_bg;
> 	}
> 
> -	for (i = first_meta_bg ; i < fs->desc_blocks; i++) {
> +	/*
> +	 * Need superblock and journal inode only. Do not read
> +	 * met_bg group blocks if journal inode group already loaded
> +	 */
> +	if (flags & EXT2_FLAG_LABEL)
> +		enough_desc_blocks =
> +			fs->super->s_journal_inum /
> +			EXT2_INODES_PER_GROUP(fs->super) + 1;
> +	else
> +		enough_desc_blocks = fs->desc_blocks;
> +
> +	for (i = first_meta_bg; i < enough_desc_blocks; i++) {
> 		blk = ext2fs_descriptor_block_loc2(fs, group_block, i);
> 		io_channel_cache_readahead(fs->io, blk, 1);
> 	}
> 
> -	for (i=first_meta_bg ; i < fs->desc_blocks; i++) {
> +	for (i = first_meta_bg; i < enough_desc_blocks; i++) {
> 		blk = ext2fs_descriptor_block_loc2(fs, group_block, i);
> 		retval = io_channel_read_blk64(fs->io, blk, 1, dest);
> 		if (retval)
> @@ -468,8 +480,12 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
> 	 */
> 	if (superblock > 1 && ext2fs_has_group_desc_csum(fs)) {
> 		dgrp_t group;
> +		dgrp_t enough_desc_count;
> +
> +		enough_desc_count = enough_desc_blocks *
> +				    EXT2_DESC_PER_BLOCK(fs->super);
> 
> -		for (group = 0; group < fs->group_desc_count; group++) {
> +		for (group = 0; group < enough_desc_count; group++) {
> 			ext2fs_bg_flags_clear(fs, group, EXT2_BG_BLOCK_UNINIT);
> 			ext2fs_bg_flags_clear(fs, group, EXT2_BG_INODE_UNINIT);
> 			ext2fs_bg_itable_unused_set(fs, group, 0);
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index cd4057c3..3d0c7d51 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2882,6 +2882,12 @@ retry_open:
> 	/* keep the filesystem struct around to dump MMP data */
> 	open_flag |= EXT2_FLAG_NOFREE_ON_ERROR;
> 
> +	if (print_label || L_flag) {
> +		open_flag |= EXT2_FLAG_LABEL;
> +		/* don't want metadata to be flushed at close */
> +		//open_flag &= ~EXT2_FLAG_RW;
> +	}
> +
> 	retval = ext2fs_open2(device_name, io_options, open_flag,
> 			      0, 0, io_ptr, &fs);
> 	if (retval) {
> @@ -2999,7 +3005,6 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
> 	if ((open_flag & EXT2_FLAG_RW) && !(mount_flags & (EXT2_MF_BUSY | EXT2_MF_MOUNTED)) &&
> 	    ext2fs_has_feature_journal_needs_recovery(fs->super)) {
> 		errcode_t err;
> -
> 		printf(_("Recovering journal.\n"));
> 		err = ext2fs_run_ext3_journal(&fs);
> 		if (err) {
> --
> 2.14.3
> 


Cheers, Andreas
diff mbox series

Patch

diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 5b87d395..5318b14d 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -204,6 +204,7 @@  typedef struct ext2_file *ext2_file_t;
 #define EXT2_FLAG_IGNORE_CSUM_ERRORS	0x200000
 #define EXT2_FLAG_SHARE_DUP		0x400000
 #define EXT2_FLAG_IGNORE_SB_ERRORS	0x800000
+#define EXT2_FLAG_LABEL			0x1000000
 
 /*
  * Special flag in the ext2 inode i_flag field that means that this is
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 85d73e2a..a5e97e6d 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -135,6 +135,7 @@  errcode_t ext2fs_open2(const char *name, const char *io_options,
 	int		j;
 #endif
 	char		*time_env;
+	unsigned long	enough_desc_blocks;
 
 	EXT2_CHECK_MAGIC(manager, EXT2_ET_MAGIC_IO_MANAGER);
 
@@ -440,12 +441,23 @@  errcode_t ext2fs_open2(const char *name, const char *io_options,
 		dest += fs->blocksize*first_meta_bg;
 	}
 
-	for (i = first_meta_bg ; i < fs->desc_blocks; i++) {
+	/*
+	 * Need superblock and journal inode only. Do not read
+	 * met_bg group blocks if journal inode group already loaded
+	 */
+	if (flags & EXT2_FLAG_LABEL)
+		enough_desc_blocks =
+			fs->super->s_journal_inum /
+			EXT2_INODES_PER_GROUP(fs->super) + 1;
+	else
+		enough_desc_blocks = fs->desc_blocks;
+
+	for (i = first_meta_bg; i < enough_desc_blocks; i++) {
 		blk = ext2fs_descriptor_block_loc2(fs, group_block, i);
 		io_channel_cache_readahead(fs->io, blk, 1);
 	}
 
-	for (i=first_meta_bg ; i < fs->desc_blocks; i++) {
+	for (i = first_meta_bg; i < enough_desc_blocks; i++) {
 		blk = ext2fs_descriptor_block_loc2(fs, group_block, i);
 		retval = io_channel_read_blk64(fs->io, blk, 1, dest);
 		if (retval)
@@ -468,8 +480,12 @@  errcode_t ext2fs_open2(const char *name, const char *io_options,
 	 */
 	if (superblock > 1 && ext2fs_has_group_desc_csum(fs)) {
 		dgrp_t group;
+		dgrp_t enough_desc_count;
+
+		enough_desc_count = enough_desc_blocks *
+				    EXT2_DESC_PER_BLOCK(fs->super);
 
-		for (group = 0; group < fs->group_desc_count; group++) {
+		for (group = 0; group < enough_desc_count; group++) {
 			ext2fs_bg_flags_clear(fs, group, EXT2_BG_BLOCK_UNINIT);
 			ext2fs_bg_flags_clear(fs, group, EXT2_BG_INODE_UNINIT);
 			ext2fs_bg_itable_unused_set(fs, group, 0);
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index cd4057c3..3d0c7d51 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2882,6 +2882,12 @@  retry_open:
 	/* keep the filesystem struct around to dump MMP data */
 	open_flag |= EXT2_FLAG_NOFREE_ON_ERROR;
 
+	if (print_label || L_flag) {
+		open_flag |= EXT2_FLAG_LABEL;
+		/* don't want metadata to be flushed at close */
+		//open_flag &= ~EXT2_FLAG_RW;
+	}
+
 	retval = ext2fs_open2(device_name, io_options, open_flag,
 			      0, 0, io_ptr, &fs);
 	if (retval) {
@@ -2999,7 +3005,6 @@  _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 	if ((open_flag & EXT2_FLAG_RW) && !(mount_flags & (EXT2_MF_BUSY | EXT2_MF_MOUNTED)) &&
 	    ext2fs_has_feature_journal_needs_recovery(fs->super)) {
 		errcode_t err;
-
 		printf(_("Recovering journal.\n"));
 		err = ext2fs_run_ext3_journal(&fs);
 		if (err) {