| Message ID | 20190905110110.32627-1-c17828@cray.com |
|---|---|
| State | New |
| Headers | show |
| Series | ext2fs: add ext2fs_read_sb that returns superblock | expand |
Hello, Should I change anything it this patch? Thanks, Artem Blagodarenko. > On 5 Sep 2019, at 14:01, 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 adds ext2fs_read_sb(), there only initialized superblock > is returned This saves time dramatically. > > Signed-off-by: Artem Blagodarenko <c17828@cray.com> > 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 >
> On Sep 17, 2019, at 12:22 PM, Благодаренко Артём <artem.blagodarenko@gmail.com> wrote: > > Hello, > > Should I change anything it this patch? Looks OK to me: Reviewed-by: Andreas Dilger <adilger@dilger.ca> > > Thanks, > Artem Blagodarenko. > >> On 5 Sep 2019, at 14:01, 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 adds ext2fs_read_sb(), there only initialized superblock >> is returned This saves time dramatically. >> >> Signed-off-by: Artem Blagodarenko <c17828@cray.com> >> 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
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 <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 adds ext2fs_read_sb(), there only initialized superblock > is returned This saves time dramatically. > > Signed-off-by: Artem Blagodarenko <c17828@cray.com> > 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 >
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, Благодаренко Артём <artem.blagodarenko@gmail.com> 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 <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 adds ext2fs_read_sb(), there only initialized superblock >> is returned This saves time dramatically. >> >> Signed-off-by: Artem Blagodarenko <c17828@cray.com> >> 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 >> >
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 <adilger@dilger.ca> 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, Благодаренко Артём <artem.blagodarenko@gmail.com> 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 <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 adds ext2fs_read_sb(), there only initialized superblock >>> is returned This saves time dramatically. >>> >>> Signed-off-by: Artem Blagodarenko <c17828@cray.com> >>> 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 >>> >>
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 <c17828@cray.com> > 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 >
Darrick, thanks for feedback. > On 3 Oct 2019, at 19:01, Darrick J. Wong <darrick.wong@oracle.com> 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 <c17828@cray.com> >> 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
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 <c17828@cray.com> > 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 <tytso@mit.edu> 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 <tytso@mit.edu> 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;
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,
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 <c17828@cray.com> 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(-)