From: Alexey Lyashkov <[email protected]>
This patchset provide a various fixes for debugfs tool.
All fixes related to the logdump command which able to handle large FS
and journal checksums version 1. It fixes a panic with journal super block
flags print.
Alexey Lyashkov (3):
block number output fix.
Fix panic with journal superblock flags printing.
add support to check a journal checksum v1 while journal dump.
debugfs/logdump.c | 103 +++++++++++++++++++++++++++++++++++-----------
e2fsck/jfs_user.h | 5 +++
2 files changed, 84 insertions(+), 24 deletions(-)
--
2.18.0
journal checksum v1 cover an all blocks in journal descriptor.
This checksum stored into commit block and check with commit
block reading.
Signed-off-by: Alexey Lyashkov <[email protected]>
---
debugfs/logdump.c | 69 ++++++++++++++++++++++++++++++++++++++---------
e2fsck/jfs_user.h | 5 ++++
2 files changed, 62 insertions(+), 12 deletions(-)
1024 is jbd superblock size, and this constant widely used over e2fsprogs code.
from this particular case it’s clear bug as jsb_buffer defined as.
>>
char jsb_buffer[1024];
>>
> 22 нояб. 2018 г., в 2:54, Theodore Y. Ts'o <[email protected]> написал(а):
>
> The summary really isn't right; "panic" is something that kernels do,
> not userspace programs.
>
>> - if (dump_super) {
>> + if (dump_all || dump_super) {
>> e2p_list_journal_super(out_file, jsb_buffer,
>> - current_fs->blocksize, 0);
>> + 1024, 0);
>
> This makes no sense to me. Why is hard-coding the expected blocksize
> to be 1024 a good thing to do here?
>
> - Ted
On Mon, Nov 19, 2018 at 12:16:50PM +0300, [email protected] wrote:
>
> journal checksum v1 cover an all blocks in journal descriptor.
> This checksum stored into commit block and check with commit
> block reading.
I'm really curious --- why do you care about journal_checksum v1 at
all? It rarely used, and the journal_checksum v3 is much superior.
- Ted
On Mon, Nov 19, 2018 at 12:16:48PM +0300, [email protected] wrote:
>
> debugfs lack a support journal tag v3 and block numbers over 2^32
> fix it.
This file is missing a Signed-off-by trailer.
Also please use a subject prefix of "debugfs: " for these patches,
since they are related to debugfs. I also prefer not having a
trailing period, and use a more descriptive summary so that it's more
understandable what the commit will do when using "git log --oneline".
So, something like:
debugfs: fix logdump to support 64-bit block numbers with v3 journal tags
Thanks,
- Ted
debugfs lack a support journal tag v3 and block numbers over 2^32
fix it.
---
debugfs/logdump.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
Signed-off-by: Alexey Lyashkov <[email protected]>
---
debugfs/logdump.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
On Thu, Nov 22, 2018 at 07:24:17AM +0300, Alexey Lyashkov wrote:
> 1024 is jbd superblock size, and this constant widely used over e2fsprogs code.
> from this particular case it’s clear bug as jsb_buffer defined as.
> >>
> char jsb_buffer[1024];
The jbd superblock size is 1024 bytes --- just as the ext4 superblock
is only 1024 bytes. It may be *stored* in a 4k block, and the size of
the super block has nothingm to do with the size of the file system
block size.
It gets used here:
if (exp_block_size != (int) ntohl(jsb->s_blocksize))
fprintf(f, "Journal block size: %u\n",
(unsigned int)ntohl(jsb->s_blocksize));
The normal case is when the journal superblock size is the same as the
file system block size, and in that case, there's no reason to print
the journal block size. It *can* happen where the journal block size
can be different from the file system block size. The most likely
case is the one where an external journal is in use, and the external
journal is shared between two file systems, one which uses (say) a 4k
block size, and the other uses (say) a 1k block size. The data
structures support this mode, but what we don't have is e2fsck support
for handling a journal replay where the journal needs to be replayed
to two different file systems --- especially if one of the block
device is temporarily unavailable.
So that's why this is there, and in fact it's intentional that it's
done this way.
Cheers,
- Ted
it’s typical option for Sonexion storages.
journal checksum is enough to cover a corruptions for external journal,
but provide a less overhead than full metadata checksums.
In our’s workload - any checksums is source of huge latency, so minimal option had enabled.
from other sides it option can enabled without disk format change.
I was have plan to add v2/v3 checksums to patch series but images need to prepared for it.
In this particular usage it was used to find a bug in jbd2 code, with back porting commit
de92c8caf16ca84926fa31b7a5590c0fb9c0d5ca
jbd2: speedup jbd2_journal_get_[write|undo]_access()
to rhel7 code.
that commits introduce a race window while frozen buffer had modified in parallel thread.
this race likely to be fixed by
ee57aba159a5c329dc78c181a3ae0549e59f0925
jbd2: simplify code flow in do_get_write_access()
which described as cleanup.
> 22 нояб. 2018 г., в 3:07, Theodore Y. Ts'o <[email protected]> написал(а):
>
> On Mon, Nov 19, 2018 at 12:16:50PM +0300, [email protected] wrote:
>>
>> journal checksum v1 cover an all blocks in journal descriptor.
>> This checksum stored into commit block and check with commit
>> block reading.
>
> I'm really curious --- why do you care about journal_checksum v1 at
> all? It rarely used, and the journal_checksum v3 is much superior.
>
> - Ted
>
The summary really isn't right; "panic" is something that kernels do,
not userspace programs.
> - if (dump_super) {
> + if (dump_all || dump_super) {
> e2p_list_journal_super(out_file, jsb_buffer,
> - current_fs->blocksize, 0);
> + 1024, 0);
This makes no sense to me. Why is hard-coding the expected blocksize
to be 1024 a good thing to do here?
- Ted