linux-ext4 - Re: [PATCH] libext2fs: readahead for meta

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <4F4ECDB0-7939-4F3D-8995-0BA6A96C658E@dilger.ca>
Date:   Tue, 28 Feb 2017 19:50:48 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     Alexey Lyashkov <alexey.lyashkov@...il.com>
Cc:     Artem Blagodarenko <artem.blagodarenko@...il.com>,
        linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] libext2fs: readahead for meta_bg

On Feb 28, 2017, at 7:19 PM, Alexey Lyashkov <alexey.lyashkov@...il.com> wrote:
> 
> Andreas,
> 
> we have do it first. But patch was much and much complex due checksums handling in code.
> and ext2_flush() will be need to read an all GD in memory to as use a flat array to write a GD to the disk.

Yes, I saw ext2_flush() was accessing the array directly, and would have a
problem as you describe.  One option would be to skip writing uninitialized
GDT blocks, but that also becomes more complex to get correct.

> If you want we have submit it also, but it have no benefits (like a few seconds), against it simple version.

I guess a large part of your speedup is because of submitting the GDT reads
in parallel to a disk array.  If the GDT blocks are all mapped to a single
disk in the array (entirely possible with META_BG, depending on array geometry)
then the prefetch will have minimal benefits.

Another option would be to change debugfs/tune2fs/dumpe2fs to use the
EXT2_FLAG_SUPER_ONLY flag to only read the superblock on open if the
requested operations do not need access to the group descriptors at all?
For a large filesystem as you describe, 37K GDT blocks is still over 144MB
of data that needs to be read from disk, vs 4KB for the superblock.

Cheers, Andreas

>> 1 марта 2017 г., в 3:10, Andreas Dilger <adilger@...ger.ca> написал(а):
>> 
>> On Feb 20, 2017, at 3:03 AM, Artem Blagodarenko <artem.blagodarenko@...il.com> wrote:
>>> 
>>> From: Alexey Lyashkov <alexey.lyashkov@...gate.com>
>>> 
>>> There are ~37k of random IOs with meta_bg option on 300T target.
>>> Debugfs requires 20 minutes to be started. Enabling readahead for
>>> group blocks metadata save time dramatically. Only 12s to start.
>>> 
>>> Signed-off-by: Alexey Lyashkov <alexey.lyashkov@...gate.com>
>> 
>> This patch looks good by itself.
>> 
>> Reviewed-by: Andreas Dilger <adilger@...ger.ca>
>> ----
>> 
>> On a related note, I've been wondering if it would make sense to have
>> a second patch that *only* does the readahead of the group descriptor blocks
>> in ext2fs_open2(), and move io_channel_read_blk64() to ext2fs_group_desc()
>> when the group descriptor blocks are actually accessed the first time?  This
>> would allow tools like tune2fs, debugfs, dumpe2fs, etc. that may not access
>> group descriptors to load _much_ faster than if it loads all of the bitmaps
>> synchronously at filesystem open time.  Even if they _do_ access the GDT it
>> will at least allow the prefetch more time to run in the background, and the
>> GDT swabbing happen incrementally upon access rather than all at the start.
>> 
>> A quick look through lib/ext2fs looks like ext2fs_group_desc() is used for
>> the majority of group descriptor accesses, but there are a few places that
>> access fs->group_desc directly.  The ext2fs_group_desc() code could check
>> whether the group descriptor is all-zero (ext2fs_open2() should be changed
>> to use ext2fs_get_array_zero(..., &fs->group_desc)) and if so read the whole
>> descriptor block into the array and optionally swab it.
>> 
>> Cheers, Andreas
>> 
>>> ---
>>> lib/ext2fs/openfs.c |    6 ++++++
>>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
>>> index ba501e6..f158b0a 100644
>>> --- a/lib/ext2fs/openfs.c
>>> +++ b/lib/ext2fs/openfs.c
>>> @@ -399,6 +399,12 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
>>> #endif
>>> 		dest += fs->blocksize*first_meta_bg;
>>> 	}
>>> +
>>> +	for (i = first_meta_bg ; i < fs->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++) {
>>> 		blk = ext2fs_descriptor_block_loc2(fs, group_block, i);
>>> 		retval = io_channel_read_blk64(fs->io, blk, 1, dest);
>>> --
>>> 1.7.1
>>> 
>> 
>> 
>> Cheers, Andreas
>> 
>> 
>> 
>> 
>> 
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)