2019-09-05 11:02:03

by Artem Blagodarenko

Subject: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

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 adds ext2fs_read_sb(), there only initialized superblock
is returned This saves time dramatically.

Signed-off-by: Artem Blagodarenko <[email protected]>
Cray-bug-id: LUS-5777
---
lib/ext2fs/ext2fs.h | 2 ++
lib/ext2fs/openfs.c | 16 ++++++++++++++++
misc/tune2fs.c | 23 +++++++++++++++--------
3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 59fd9742..3a63b74d 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
unsigned int block_size, io_manager manager,
ext2_filsys *ret_fs);
+extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
+ struct ext2_super_block * super);
extern errcode_t ext2fs_open2(const char *name, const char *io_options,
int flags, int superblock,
unsigned int block_size, io_manager manager,
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 51b54a44..95f45d84 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
return;
}

+errcode_t ext2fs_read_sb(const char *name, io_manager manager,
+ struct ext2_super_block * super)
+{
+ io_channel io;
+ errcode_t retval = 0;
+
+ retval = manager->open(name, 0, &io);
+ if (!retval) {
+ retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
+ super);
+ io_channel_close(io);
+ }
+
+ return retval;
+}
+
/*
* Note: if superblock is non-zero, block-size must also be non-zero.
* Superblock and block_size can be zero to use the default size.
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 7d2d38d7..fea607e1 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
#endif
io_ptr = unix_io_manager;

+ if (print_label) {
+ /* For e2label emulation */
+ struct ext2_super_block sb;
+
+ /* Read only superblock. Nothing else metters.*/
+ retval = ext2fs_read_sb(device_name, io_ptr, &sb);
+ if (!retval) {
+ printf("%.*s\n", (int) sizeof(sb.s_volume_name),
+ sb.s_volume_name);
+ }
+
+ remove_error_table(&et_ext2_error_table);
+ return retval;
+ }
+
retry_open:
if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
open_flag |= EXT2_FLAG_SKIP_MMP;
@@ -2972,14 +2987,6 @@ retry_open:
sb = fs->super;
fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;

- if (print_label) {
- /* For e2label emulation */
- printf("%.*s\n", (int) sizeof(sb->s_volume_name),
- sb->s_volume_name);
- remove_error_table(&et_ext2_error_table);
- goto closefs;
- }
-
retval = ext2fs_check_if_mounted(device_name, &mount_flags);
if (retval) {
com_err("ext2fs_check_if_mount", retval,
--
2.14.3

2019-09-17 19:41:53

by Artem Blagodarenko

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

Hello,

Should I change anything it this patch?

Thanks,
Artem Blagodarenko.

> On 5 Sep 2019, at 14:01, Artem Blagodarenko <[email protected]> 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 adds ext2fs_read_sb(), there only initialized superblock
> is returned This saves time dramatically.
>
> Signed-off-by: Artem Blagodarenko <[email protected]>
> Cray-bug-id: LUS-5777
> ---
> lib/ext2fs/ext2fs.h | 2 ++
> lib/ext2fs/openfs.c | 16 ++++++++++++++++
> misc/tune2fs.c | 23 +++++++++++++++--------
> 3 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index 59fd9742..3a63b74d 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
> unsigned int block_size, io_manager manager,
> ext2_filsys *ret_fs);
> +extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
> + struct ext2_super_block * super);
> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
> int flags, int superblock,
> unsigned int block_size, io_manager manager,
> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
> index 51b54a44..95f45d84 100644
> --- a/lib/ext2fs/openfs.c
> +++ b/lib/ext2fs/openfs.c
> @@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
> return;
> }
>
> +errcode_t ext2fs_read_sb(const char *name, io_manager manager,
> + struct ext2_super_block * super)
> +{
> + io_channel io;
> + errcode_t retval = 0;
> +
> + retval = manager->open(name, 0, &io);
> + if (!retval) {
> + retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
> + super);
> + io_channel_close(io);
> + }
> +
> + return retval;
> +}
> +
> /*
> * Note: if superblock is non-zero, block-size must also be non-zero.
> * Superblock and block_size can be zero to use the default size.
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 7d2d38d7..fea607e1 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
> #endif
> io_ptr = unix_io_manager;
>
> + if (print_label) {
> + /* For e2label emulation */
> + struct ext2_super_block sb;
> +
> + /* Read only superblock. Nothing else metters.*/
> + retval = ext2fs_read_sb(device_name, io_ptr, &sb);
> + if (!retval) {
> + printf("%.*s\n", (int) sizeof(sb.s_volume_name),
> + sb.s_volume_name);
> + }
> +
> + remove_error_table(&et_ext2_error_table);
> + return retval;
> + }
> +
> retry_open:
> if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
> open_flag |= EXT2_FLAG_SKIP_MMP;
> @@ -2972,14 +2987,6 @@ retry_open:
> sb = fs->super;
> fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
>
> - if (print_label) {
> - /* For e2label emulation */
> - printf("%.*s\n", (int) sizeof(sb->s_volume_name),
> - sb->s_volume_name);
> - remove_error_table(&et_ext2_error_table);
> - goto closefs;
> - }
> -
> retval = ext2fs_check_if_mounted(device_name, &mount_flags);
> if (retval) {
> com_err("ext2fs_check_if_mount", retval,
> --
> 2.14.3
>

2019-09-17 20:21:03

by Andreas Dilger

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

> On Sep 17, 2019, at 12:22 PM, Благодаренко Артём <[email protected]> wrote:
>
> Hello,
>
> Should I change anything it this patch?

Looks OK to me:

Reviewed-by: Andreas Dilger <[email protected]>

>
> Thanks,
> Artem Blagodarenko.
>
>> On 5 Sep 2019, at 14:01, Artem Blagodarenko <[email protected]> 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 adds ext2fs_read_sb(), there only initialized superblock
>> is returned This saves time dramatically.
>>
>> Signed-off-by: Artem Blagodarenko <[email protected]>
>> Cray-bug-id: LUS-5777
>> ---
>> lib/ext2fs/ext2fs.h | 2 ++
>> lib/ext2fs/openfs.c | 16 ++++++++++++++++
>> misc/tune2fs.c | 23 +++++++++++++++--------
>> 3 files changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
>> index 59fd9742..3a63b74d 100644
>> --- a/lib/ext2fs/ext2fs.h
>> +++ b/lib/ext2fs/ext2fs.h
>> @@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
>> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
>> unsigned int block_size, io_manager manager,
>> ext2_filsys *ret_fs);
>> +extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>> + struct ext2_super_block * super);
>> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
>> int flags, int superblock,
>> unsigned int block_size, io_manager manager,
>> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
>> index 51b54a44..95f45d84 100644
>> --- a/lib/ext2fs/openfs.c
>> +++ b/lib/ext2fs/openfs.c
>> @@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
>> return;
>> }
>>
>> +errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>> + struct ext2_super_block * super)
>> +{
>> + io_channel io;
>> + errcode_t retval = 0;
>> +
>> + retval = manager->open(name, 0, &io);
>> + if (!retval) {
>> + retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
>> + super);
>> + io_channel_close(io);
>> + }
>> +
>> + return retval;
>> +}
>> +
>> /*
>> * Note: if superblock is non-zero, block-size must also be non-zero.
>> * Superblock and block_size can be zero to use the default size.
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 7d2d38d7..fea607e1 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
>> #endif
>> io_ptr = unix_io_manager;
>>
>> + if (print_label) {
>> + /* For e2label emulation */
>> + struct ext2_super_block sb;
>> +
>> + /* Read only superblock. Nothing else metters.*/
>> + retval = ext2fs_read_sb(device_name, io_ptr, &sb);
>> + if (!retval) {
>> + printf("%.*s\n", (int) sizeof(sb.s_volume_name),
>> + sb.s_volume_name);
>> + }
>> +
>> + remove_error_table(&et_ext2_error_table);
>> + return retval;
>> + }
>> +
>> retry_open:
>> if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
>> open_flag |= EXT2_FLAG_SKIP_MMP;
>> @@ -2972,14 +2987,6 @@ retry_open:
>> sb = fs->super;
>> fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
>>
>> - if (print_label) {
>> - /* For e2label emulation */
>> - printf("%.*s\n", (int) sizeof(sb->s_volume_name),
>> - sb->s_volume_name);
>> - remove_error_table(&et_ext2_error_table);
>> - goto closefs;
>> - }
>> -
>> retval = ext2fs_check_if_mounted(device_name, &mount_flags);
>> if (retval) {
>> com_err("ext2fs_check_if_mount", retval,
>> --
>> 2.14.3
>>
>


Cheers, Andreas

Attachments:

2019-10-03 16:00:17

by Artem Blagodarenko

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

Hello,

Does anybody wants to give any feedback for this?
e2label became really faster on large partitions with this patch. Probably it can be useful.

Ted, what do you think?

Thanks,
Artem Blagodarenko.



> On 5 Sep 2019, at 14:01, Artem Blagodarenko <[email protected]> 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 adds ext2fs_read_sb(), there only initialized superblock
> is returned This saves time dramatically.
>
> Signed-off-by: Artem Blagodarenko <[email protected]>
> Cray-bug-id: LUS-5777
> ---
> lib/ext2fs/ext2fs.h | 2 ++
> lib/ext2fs/openfs.c | 16 ++++++++++++++++
> misc/tune2fs.c | 23 +++++++++++++++--------
> 3 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index 59fd9742..3a63b74d 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
> unsigned int block_size, io_manager manager,
> ext2_filsys *ret_fs);
> +extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
> + struct ext2_super_block * super);
> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
> int flags, int superblock,
> unsigned int block_size, io_manager manager,
> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
> index 51b54a44..95f45d84 100644
> --- a/lib/ext2fs/openfs.c
> +++ b/lib/ext2fs/openfs.c
> @@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
> return;
> }
>
> +errcode_t ext2fs_read_sb(const char *name, io_manager manager,
> + struct ext2_super_block * super)
> +{
> + io_channel io;
> + errcode_t retval = 0;
> +
> + retval = manager->open(name, 0, &io);
> + if (!retval) {
> + retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
> + super);
> + io_channel_close(io);
> + }
> +
> + return retval;
> +}
> +
> /*
> * Note: if superblock is non-zero, block-size must also be non-zero.
> * Superblock and block_size can be zero to use the default size.
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 7d2d38d7..fea607e1 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
> #endif
> io_ptr = unix_io_manager;
>
> + if (print_label) {
> + /* For e2label emulation */
> + struct ext2_super_block sb;
> +
> + /* Read only superblock. Nothing else metters.*/
> + retval = ext2fs_read_sb(device_name, io_ptr, &sb);
> + if (!retval) {
> + printf("%.*s\n", (int) sizeof(sb.s_volume_name),
> + sb.s_volume_name);
> + }
> +
> + remove_error_table(&et_ext2_error_table);
> + return retval;
> + }
> +
> retry_open:
> if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
> open_flag |= EXT2_FLAG_SKIP_MMP;
> @@ -2972,14 +2987,6 @@ retry_open:
> sb = fs->super;
> fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
>
> - if (print_label) {
> - /* For e2label emulation */
> - printf("%.*s\n", (int) sizeof(sb->s_volume_name),
> - sb->s_volume_name);
> - remove_error_table(&et_ext2_error_table);
> - goto closefs;
> - }
> -
> retval = ext2fs_check_if_mounted(device_name, &mount_flags);
> if (retval) {
> com_err("ext2fs_check_if_mount", retval,
> --
> 2.14.3
>

2019-10-03 16:00:21

by Andreas Dilger

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

We just discussed this on the ext4 developer concall today, and Ted is looking into it.

Cheers, Andreas

> On Oct 3, 2019, at 09:47, Благодаренко Артём <[email protected]> wrote:
>
> Hello,
>
> Does anybody wants to give any feedback for this?
> e2label became really faster on large partitions with this patch. Probably it can be useful.
>
> Ted, what do you think?
>
> Thanks,
> Artem Blagodarenko.
>
>
>
>> On 5 Sep 2019, at 14:01, Artem Blagodarenko <[email protected]> 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 adds ext2fs_read_sb(), there only initialized superblock
>> is returned This saves time dramatically.
>>
>> Signed-off-by: Artem Blagodarenko <[email protected]>
>> Cray-bug-id: LUS-5777
>> ---
>> lib/ext2fs/ext2fs.h | 2 ++
>> lib/ext2fs/openfs.c | 16 ++++++++++++++++
>> misc/tune2fs.c | 23 +++++++++++++++--------
>> 3 files changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
>> index 59fd9742..3a63b74d 100644
>> --- a/lib/ext2fs/ext2fs.h
>> +++ b/lib/ext2fs/ext2fs.h
>> @@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
>> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
>> unsigned int block_size, io_manager manager,
>> ext2_filsys *ret_fs);
>> +extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>> + struct ext2_super_block * super);
>> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
>> int flags, int superblock,
>> unsigned int block_size, io_manager manager,
>> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
>> index 51b54a44..95f45d84 100644
>> --- a/lib/ext2fs/openfs.c
>> +++ b/lib/ext2fs/openfs.c
>> @@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
>> return;
>> }
>>
>> +errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>> + struct ext2_super_block * super)
>> +{
>> + io_channel io;
>> + errcode_t retval = 0;
>> +
>> + retval = manager->open(name, 0, &io);
>> + if (!retval) {
>> + retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
>> + super);
>> + io_channel_close(io);
>> + }
>> +
>> + return retval;
>> +}
>> +
>> /*
>> * Note: if superblock is non-zero, block-size must also be non-zero.
>> * Superblock and block_size can be zero to use the default size.
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 7d2d38d7..fea607e1 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
>> #endif
>> io_ptr = unix_io_manager;
>>
>> + if (print_label) {
>> + /* For e2label emulation */
>> + struct ext2_super_block sb;
>> +
>> + /* Read only superblock. Nothing else metters.*/
>> + retval = ext2fs_read_sb(device_name, io_ptr, &sb);
>> + if (!retval) {
>> + printf("%.*s\n", (int) sizeof(sb.s_volume_name),
>> + sb.s_volume_name);
>> + }
>> +
>> + remove_error_table(&et_ext2_error_table);
>> + return retval;
>> + }
>> +
>> retry_open:
>> if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
>> open_flag |= EXT2_FLAG_SKIP_MMP;
>> @@ -2972,14 +2987,6 @@ retry_open:
>> sb = fs->super;
>> fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
>>
>> - if (print_label) {
>> - /* For e2label emulation */
>> - printf("%.*s\n", (int) sizeof(sb->s_volume_name),
>> - sb->s_volume_name);
>> - remove_error_table(&et_ext2_error_table);
>> - goto closefs;
>> - }
>> -
>> retval = ext2fs_check_if_mounted(device_name, &mount_flags);
>> if (retval) {
>> com_err("ext2fs_check_if_mount", retval,
>> --
>> 2.14.3
>>
>

2019-10-03 16:04:14

by Darrick J. Wong

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

On Thu, Sep 05, 2019 at 02:01:10PM +0300, Artem Blagodarenko 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 adds ext2fs_read_sb(), there only initialized superblock
> is returned This saves time dramatically.
>
> Signed-off-by: Artem Blagodarenko <[email protected]>
> Cray-bug-id: LUS-5777
> ---
> lib/ext2fs/ext2fs.h | 2 ++
> lib/ext2fs/openfs.c | 16 ++++++++++++++++
> misc/tune2fs.c | 23 +++++++++++++++--------
> 3 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index 59fd9742..3a63b74d 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
> unsigned int block_size, io_manager manager,
> ext2_filsys *ret_fs);
> +extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
> + struct ext2_super_block * super);
> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
> int flags, int superblock,
> unsigned int block_size, io_manager manager,
> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
> index 51b54a44..95f45d84 100644
> --- a/lib/ext2fs/openfs.c
> +++ b/lib/ext2fs/openfs.c
> @@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
> return;
> }
>
> +errcode_t ext2fs_read_sb(const char *name, io_manager manager,
> + struct ext2_super_block * super)
> +{
> + io_channel io;
> + errcode_t retval = 0;
> +
> + retval = manager->open(name, 0, &io);
> + if (!retval) {
> + retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
> + super);
> + io_channel_close(io);
> + }
> +
> + return retval;
> +}
> +
> /*
> * Note: if superblock is non-zero, block-size must also be non-zero.
> * Superblock and block_size can be zero to use the default size.
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 7d2d38d7..fea607e1 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
> #endif
> io_ptr = unix_io_manager;
>
> + if (print_label) {
> + /* For e2label emulation */
> + struct ext2_super_block sb;
> +
> + /* Read only superblock. Nothing else metters.*/

matters. */

> + retval = ext2fs_read_sb(device_name, io_ptr, &sb);
> + if (!retval) {
> + printf("%.*s\n", (int) sizeof(sb.s_volume_name),
> + sb.s_volume_name);
> + }

Um, does this drop the error without making a report?

> +
> + remove_error_table(&et_ext2_error_table);
> + return retval;
> + }

I wonder if ext[24] should implement FS_IOC_[GS]ETFSLABEL for mounted
filesystems ... ?

--D

> +
> retry_open:
> if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
> open_flag |= EXT2_FLAG_SKIP_MMP;
> @@ -2972,14 +2987,6 @@ retry_open:
> sb = fs->super;
> fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
>
> - if (print_label) {
> - /* For e2label emulation */
> - printf("%.*s\n", (int) sizeof(sb->s_volume_name),
> - sb->s_volume_name);
> - remove_error_table(&et_ext2_error_table);
> - goto closefs;
> - }
> -
> retval = ext2fs_check_if_mounted(device_name, &mount_flags);
> if (retval) {
> com_err("ext2fs_check_if_mount", retval,
> --
> 2.14.3
>

2019-10-03 17:32:38

by Artem Blagodarenko

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

Darrick, thanks for feedback.

> On 3 Oct 2019, at 19:01, Darrick J. Wong <[email protected]> wrote:
>
> On Thu, Sep 05, 2019 at 02:01:10PM +0300, Artem Blagodarenko 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 adds ext2fs_read_sb(), there only initialized superblock
>> is returned This saves time dramatically.
>>
>> Signed-off-by: Artem Blagodarenko <[email protected]>
>> Cray-bug-id: LUS-5777
>> ---
>> lib/ext2fs/ext2fs.h | 2 ++
>> lib/ext2fs/openfs.c | 16 ++++++++++++++++
>> misc/tune2fs.c | 23 +++++++++++++++--------
>> 3 files changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
>> index 59fd9742..3a63b74d 100644
>> --- a/lib/ext2fs/ext2fs.h
>> +++ b/lib/ext2fs/ext2fs.h
>> @@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
>> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
>> unsigned int block_size, io_manager manager,
>> ext2_filsys *ret_fs);
>> +extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>> + struct ext2_super_block * super);
>> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
>> int flags, int superblock,
>> unsigned int block_size, io_manager manager,
>> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
>> index 51b54a44..95f45d84 100644
>> --- a/lib/ext2fs/openfs.c
>> +++ b/lib/ext2fs/openfs.c
>> @@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
>> return;
>> }
>>
>> +errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>> + struct ext2_super_block * super)
>> +{
>> + io_channel io;
>> + errcode_t retval = 0;
>> +
>> + retval = manager->open(name, 0, &io);
>> + if (!retval) {
>> + retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
>> + super);
>> + io_channel_close(io);
>> + }
>> +
>> + return retval;
>> +}
>> +
>> /*
>> * Note: if superblock is non-zero, block-size must also be non-zero.
>> * Superblock and block_size can be zero to use the default size.
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 7d2d38d7..fea607e1 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
>> #endif
>> io_ptr = unix_io_manager;
>>
>> + if (print_label) {
>> + /* For e2label emulation */
>> + struct ext2_super_block sb;
>> +
>> + /* Read only superblock. Nothing else metters.*/
>
> matters. */
>
>> + retval = ext2fs_read_sb(device_name, io_ptr, &sb);
>> + if (!retval) {
>> + printf("%.*s\n", (int) sizeof(sb.s_volume_name),
>> + sb.s_volume_name);
>> + }
>
> Um, does this drop the error without making a report?

No error message to output, but error code is returned to pointer.
I believe user expect only disk label, no other output.

>
>> +
>> + remove_error_table(&et_ext2_error_table);
>> + return retval;
>> + }
>
> I wonder if ext[24] should implement FS_IOC_[GS]ETFSLABEL for mounted
> filesystems … ?

Probably not very useful for Lustre FS, there disk label is not needed during cluster work.
Darrick, can you suggest any use case for this?
Also such features need to be added in separate patch. This patch is about performance optimisation.

Best regards,
Artem Blagodarenko.
>
> --D
>
>> +
>> retry_open:
>> if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
>> open_flag |= EXT2_FLAG_SKIP_MMP;
>> @@ -2972,14 +2987,6 @@ retry_open:
>> sb = fs->super;
>> fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
>>
>> - if (print_label) {
>> - /* For e2label emulation */
>> - printf("%.*s\n", (int) sizeof(sb->s_volume_name),
>> - sb->s_volume_name);
>> - remove_error_table(&et_ext2_error_table);
>> - goto closefs;
>> - }
>> -
>> retval = ext2fs_check_if_mounted(device_name, &mount_flags);
>> if (retval) {
>> com_err("ext2fs_check_if_mount", retval,
>> --
>> 2.14.3

2019-10-03 18:50:28

by Artem Blagodarenko

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

Thanks.

Sorry, not good new call time for me, but I can join occasionally.
I joined today, but too late.

Best regards,
Artem Blagodarenko.

> On 3 Oct 2019, at 18:55, Andreas Dilger <[email protected]> wrote:
>
> We just discussed this on the ext4 developer concall today, and Ted is looking into it.
>
> Cheers, Andreas
>
>> On Oct 3, 2019, at 09:47, Благодаренко Артём <[email protected]> wrote:
>>
>> Hello,
>>
>> Does anybody wants to give any feedback for this?
>> e2label became really faster on large partitions with this patch. Probably it can be useful.
>>
>> Ted, what do you think?
>>
>> Thanks,
>> Artem Blagodarenko.
>>
>>
>>
>>> On 5 Sep 2019, at 14:01, Artem Blagodarenko <[email protected]> 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 adds ext2fs_read_sb(), there only initialized superblock
>>> is returned This saves time dramatically.
>>>
>>> Signed-off-by: Artem Blagodarenko <[email protected]>
>>> Cray-bug-id: LUS-5777
>>> ---
>>> lib/ext2fs/ext2fs.h | 2 ++
>>> lib/ext2fs/openfs.c | 16 ++++++++++++++++
>>> misc/tune2fs.c | 23 +++++++++++++++--------
>>> 3 files changed, 33 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
>>> index 59fd9742..3a63b74d 100644
>>> --- a/lib/ext2fs/ext2fs.h
>>> +++ b/lib/ext2fs/ext2fs.h
>>> @@ -1630,6 +1630,8 @@ extern int ext2fs_journal_sb_start(int blocksize);
>>> extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
>>> unsigned int block_size, io_manager manager,
>>> ext2_filsys *ret_fs);
>>> +extern errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>>> + struct ext2_super_block * super);
>>> extern errcode_t ext2fs_open2(const char *name, const char *io_options,
>>> int flags, int superblock,
>>> unsigned int block_size, io_manager manager,
>>> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
>>> index 51b54a44..95f45d84 100644
>>> --- a/lib/ext2fs/openfs.c
>>> +++ b/lib/ext2fs/openfs.c
>>> @@ -99,6 +99,22 @@ static void block_sha_map_free_entry(void *data)
>>> return;
>>> }
>>>
>>> +errcode_t ext2fs_read_sb(const char *name, io_manager manager,
>>> + struct ext2_super_block * super)
>>> +{
>>> + io_channel io;
>>> + errcode_t retval = 0;
>>> +
>>> + retval = manager->open(name, 0, &io);
>>> + if (!retval) {
>>> + retval = io_channel_read_blk(io, 1, -SUPERBLOCK_SIZE,
>>> + super);
>>> + io_channel_close(io);
>>> + }
>>> +
>>> + return retval;
>>> +}
>>> +
>>> /*
>>> * Note: if superblock is non-zero, block-size must also be non-zero.
>>> * Superblock and block_size can be zero to use the default size.
>>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>>> index 7d2d38d7..fea607e1 100644
>>> --- a/misc/tune2fs.c
>>> +++ b/misc/tune2fs.c
>>> @@ -2879,6 +2879,21 @@ int tune2fs_main(int argc, char **argv)
>>> #endif
>>> io_ptr = unix_io_manager;
>>>
>>> + if (print_label) {
>>> + /* For e2label emulation */
>>> + struct ext2_super_block sb;
>>> +
>>> + /* Read only superblock. Nothing else metters.*/
>>> + retval = ext2fs_read_sb(device_name, io_ptr, &sb);
>>> + if (!retval) {
>>> + printf("%.*s\n", (int) sizeof(sb.s_volume_name),
>>> + sb.s_volume_name);
>>> + }
>>> +
>>> + remove_error_table(&et_ext2_error_table);
>>> + return retval;
>>> + }
>>> +
>>> retry_open:
>>> if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
>>> open_flag |= EXT2_FLAG_SKIP_MMP;
>>> @@ -2972,14 +2987,6 @@ retry_open:
>>> sb = fs->super;
>>> fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
>>>
>>> - if (print_label) {
>>> - /* For e2label emulation */
>>> - printf("%.*s\n", (int) sizeof(sb->s_volume_name),
>>> - sb->s_volume_name);
>>> - remove_error_table(&et_ext2_error_table);
>>> - goto closefs;
>>> - }
>>> -
>>> retval = ext2fs_check_if_mounted(device_name, &mount_flags);
>>> if (retval) {
>>> com_err("ext2fs_check_if_mount", retval,
>>> --
>>> 2.14.3
>>>
>>

2019-10-23 00:19:04

by Theodore Ts'o

Subject: Re: [PATCH] ext2fs: add ext2fs_read_sb that returns superblock

On Thu, Sep 05, 2019 at 02:01:10PM +0300, Artem Blagodarenko 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 adds ext2fs_read_sb(), there only initialized superblock
> is returned This saves time dramatically.
>
> Signed-off-by: Artem Blagodarenko <[email protected]>
> Cray-bug-id: LUS-5777

Sorry for the delay in getting back to you on this. I've been
thinking about this, and I've found a better to support this
functionality by reusing the pre-existing EXT2_FLAG_SUPER_ONLY flag.
Unlike the previous version of this patch which defined
EXT2_FLAG_JOURNAL_ONLY (which was always a bit strangely named), this
avoids reading *any* block group descriptors when the file system is
open. Instead, we read the block group descriptors on demand when
ext2fs_group_desc() is called.

So this speeds up "dumpe2fs -h" as well "e2label", and we don't have
to read any block group descriptors at all. Oh, and it even works
when setting a label using e2label.

What do you think?

- Ted

commit 639e310d64dd0a2c1302eba8c3f5d0def7eacbf2
Author: Theodore Ts'o <[email protected]>
Date: Tue Oct 22 18:42:25 2019 -0400

Teach ext2fs_open2() to honor the EXT2_FLAG_SUPER_ONLY flag

Opening the file system with EXT2_FLAG_SUPER_ONLY will leave
fs->group_desc to be NULL and modify "dumpe2fs -h" and tune2fs when it
is emulating e2label to use this flag. This speeds up "dumpe2fs -h"
and "e2label" when operating on very large file systems.

To allow other libext2fs functions to work without too many surprises,
ext2fs_group_desc() will read in the block group descriptors on
demand. This allows "dumpe2fs -h" to be able to read the journal
inode, for example.

Signed-off-by: Theodore Ts'o <[email protected]>
Cray-bug-id: LUS-5777

diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
index 9ee5c66e..fdd51df6 100644
--- a/lib/ext2fs/blknum.c
+++ b/lib/ext2fs/blknum.c
@@ -185,9 +185,45 @@ struct ext2_group_desc *ext2fs_group_desc(ext2_filsys fs,
struct opaque_ext2_group_desc *gdp,
dgrp_t group)
{
- int desc_size = EXT2_DESC_SIZE(fs->super) & ~7;
+ struct ext2_group_desc *ret_gdp;
+ errcode_t retval;
+ static char *buf = 0;
+ static int bufsize = 0;
+ blk64_t blk;
+ int desc_size = EXT2_DESC_SIZE(fs->super) & ~7;
+ int desc_per_blk = EXT2_DESC_PER_BLOCK(fs->super);
+
+ if (group > fs->group_desc_count)
+ return NULL;
+ if (gdp)
+ return (struct ext2_group_desc *)((char *)gdp +
+ group * desc_size);
+
+ /*
+ * If fs->group_desc wasn't read in when the file system was
+ * opened, then read it on demand here.
+ */
+ if (bufsize < fs->blocksize)
+ ext2fs_free_mem(&buf);
+ if (!buf) {
+ retval = ext2fs_get_mem(fs->blocksize, &buf);
+ if (retval)
+ return NULL;
+ bufsize = fs->blocksize;
+ }

- return (struct ext2_group_desc *)((char *)gdp + group * desc_size);
+ blk = ext2fs_descriptor_block_loc2(fs, fs->super->s_first_data_block,
+ group / desc_per_blk);
+ retval = io_channel_read_blk(fs->io, blk, 1, buf);
+ if (retval)
+ return NULL;
+
+ ret_gdp = (struct ext2_group_desc *)
+ (buf + ((group % desc_per_blk) * desc_size));
+#ifdef WORDS_BIGENDIAN
+ ext2fs_swap_group_desc2(fs, ret_gdp);
+#endif
+ return ret_gdp;
}

/* Do the same but as an ext4 group desc for internal use here */
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 51b54a44..ec2d6cb4 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -393,6 +393,8 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
}
fs->desc_blocks = ext2fs_div_ceil(fs->group_desc_count,
EXT2_DESC_PER_BLOCK(fs->super));
+ if (flags & EXT2_FLAG_SUPER_ONLY)
+ goto skip_read_bg;
retval = ext2fs_get_array(fs->desc_blocks, fs->blocksize,
&fs->group_desc);
if (retval)
@@ -479,7 +481,7 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
if (fs->flags & EXT2_FLAG_RW)
ext2fs_mark_super_dirty(fs);
}
-
+skip_read_bg:
if (ext2fs_has_feature_mmp(fs->super) &&
!(flags & EXT2_FLAG_SKIP_MMP) &&
(flags & (EXT2_FLAG_RW | EXT2_FLAG_EXCLUSIVE))) {
diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
index 384ce925..18148e2a 100644
--- a/misc/dumpe2fs.c
+++ b/misc/dumpe2fs.c
@@ -666,6 +666,8 @@ int main (int argc, char ** argv)
flags |= EXT2_FLAG_FORCE;
if (image_dump)
flags |= EXT2_FLAG_IMAGE_FILE;
+ if (header_only)
+ flags |= EXT2_FLAG_SUPER_ONLY;
try_open_again:
if (use_superblock && !use_blocksize) {
for (use_blocksize = EXT2_MIN_BLOCK_SIZE;
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 39fce4a9..77a45875 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -1698,7 +1698,7 @@ static void parse_e2label_options(int argc, char ** argv)
argv[1]);
exit(1);
}
- open_flag = EXT2_FLAG_JOURNAL_DEV_OK;
+ open_flag = EXT2_FLAG_JOURNAL_DEV_OK | EXT2_FLAG_SUPER_ONLY;
if (argc == 3) {
open_flag |= EXT2_FLAG_RW;
L_flag = 1;