* [BUG] Tune2fs and fuse2fs do not change j_tail_sequence in journal superblock
@ 2022-08-02 11:23 zhanchengbin
2022-08-03 4:52 ` Alexey Lyahkov
2022-08-04 10:33 ` [PATCH] tune2fs: " zhanchengbin
0 siblings, 2 replies; 6+ messages in thread
From: zhanchengbin @ 2022-08-02 11:23 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, linfeilong, liuzhiqiang26, liangyun2
There are two recover_ext3_journal functions in e2fsprogs, the
recover_ext3_journal
function in debugfs/journal.c is called when the programs tune2fs and
fuse2fs do
log replay, but in this recover_ext3_journal function, after the log
replay is over,
the j_tail_sequence in journal superblock is not changed to the value of
the last
transaction sequence, this will cause subsequent log commitids to count
from the
commitid in last time.
```
e2fsck/journal.c
static errcode_t recover_ext3_journal(e2fsck_t ctx)
{
... ...
journal->j_superblock->s_errno = -EINVAL;
mark_buffer_dirty(journal->j_sb_buffer);
}
journal->j_tail_sequence = journal->j_transaction_sequence;
errout:
journal_destroy_revoke(journal);
... ...
}
```
```
debugfs/journal.c
static errcode_t recover_ext3_journal(ext2_filsys fs)
{
... ...
journal->j_superblock->s_errno = -EINVAL;
mark_buffer_dirty(journal->j_sb_buffer);
}
errout:
journal_destroy_revoke(journal);
... ...
}
```
result:
After tune2fs -e, the log commitid is counted from the commitid in last
time, if
the log ID of the current operation overlaps with that of the last
operation, this
will cause logs that were previously replayed by tune2fs to be replayed
here. The
code used by tune2fs to determine whether to replay the transaction is
as follows:
```
static int do_one_pass(journal_t *journal,
struct recovery_info *info, enum passtype pass)
{
... ...
while (1) {
... ...
if (sequence != next_commit_ID) {
brelse(bh);
break;
}
... ...
next_commit_ID++;
}
... ...
}
```
This moment, commitid is "34 c7", commit time stamp is "62 e0 f7 a5"
004aa000 c0 3b 39 98 00 00 00 02 00 00 34 c7 00 00 00 00
|.;9.......4.....|
004aa010 4e 93 2f fb 00 00 00 00 00 00 00 00 00 00 00 00
|N./.............|
004aa020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|................|
004aa030 00 00 00 00 62 e0 f7 a5 0a 16 56 20 00 00 00 00
|....b.....V ....|
--
This moment, commitid is "34 c8", commit time stamp is "62 e0 e7 1e"
004ad000 c0 3b 39 98 00 00 00 02 00 00 34 c8 00 00 00 00
|.;9.......4.....|
004ad010 75 a6 12 01 00 00 00 00 00 00 00 00 00 00 00 00
|u...............|
004ad020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|................|
004ad030 00 00 00 00 62 e0 e7 1e 18 32 cf 18 00 00 00 00
|....b....2......|
--
The probability of this happening is very small, but we do, and I think
it's a problem.
There are two existing solutions:
1) Add "journal->j_tail_sequence = journal->j_transaction_sequence" in
to the
recover_ext3_journal in debugfs/journal.c.
2) There is a timestamp in the commit block, so we can add timestamp
check when
the log is replayed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] Tune2fs and fuse2fs do not change j_tail_sequence in journal superblock
2022-08-02 11:23 [BUG] Tune2fs and fuse2fs do not change j_tail_sequence in journal superblock zhanchengbin
@ 2022-08-03 4:52 ` Alexey Lyahkov
2022-08-04 8:30 ` zhanchengbin
2022-08-04 10:33 ` [PATCH] tune2fs: " zhanchengbin
1 sibling, 1 reply; 6+ messages in thread
From: Alexey Lyahkov @ 2022-08-03 4:52 UTC (permalink / raw)
To: zhanchengbin
Cc: Theodore Ts'o, linux-ext4, linfeilong, liuzhiqiang26, liangyun2
Hi
Thanks for you report.
Problem much bigger than it. Debugs based code lack many parts of journal handling, including fast commit.
Journal tag v3, and other.
Alex
> On 2 Aug 2022, at 14:23, zhanchengbin <zhanchengbin1@huawei.com> wrote:
>
> There are two recover_ext3_journal functions in e2fsprogs, the recover_ext3_journal
> function in debugfs/journal.c is called when the programs tune2fs and fuse2fs do
> log replay, but in this recover_ext3_journal function, after the log replay is over,
> the j_tail_sequence in journal superblock is not changed to the value of the last
> transaction sequence, this will cause subsequent log commitids to count from the
> commitid in last time.
> ```
> e2fsck/journal.c
> static errcode_t recover_ext3_journal(e2fsck_t ctx)
> {
> ... ...
> journal->j_superblock->s_errno = -EINVAL;
> mark_buffer_dirty(journal->j_sb_buffer);
> }
>
> journal->j_tail_sequence = journal->j_transaction_sequence;
>
> errout:
> journal_destroy_revoke(journal);
> ... ...
> }
> ```
> ```
> debugfs/journal.c
> static errcode_t recover_ext3_journal(ext2_filsys fs)
> {
> ... ...
> journal->j_superblock->s_errno = -EINVAL;
> mark_buffer_dirty(journal->j_sb_buffer);
> }
>
> errout:
> journal_destroy_revoke(journal);
> ... ...
> }
> ```
> result:
> After tune2fs -e, the log commitid is counted from the commitid in last time, if
> the log ID of the current operation overlaps with that of the last operation, this
> will cause logs that were previously replayed by tune2fs to be replayed here. The
> code used by tune2fs to determine whether to replay the transaction is as follows:
> ```
> static int do_one_pass(journal_t *journal,
> struct recovery_info *info, enum passtype pass)
> {
> ... ...
> while (1) {
> ... ...
> if (sequence != next_commit_ID) {
> brelse(bh);
> break;
> }
> ... ...
> next_commit_ID++;
> }
> ... ...
> }
> ```
> This moment, commitid is "34 c7", commit time stamp is "62 e0 f7 a5"
> 004aa000 c0 3b 39 98 00 00 00 02 00 00 34 c7 00 00 00 00 |.;9.......4.....|
> 004aa010 4e 93 2f fb 00 00 00 00 00 00 00 00 00 00 00 00 |N./.............|
> 004aa020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
> 004aa030 00 00 00 00 62 e0 f7 a5 0a 16 56 20 00 00 00 00 |....b.....V ....|
> --
> This moment, commitid is "34 c8", commit time stamp is "62 e0 e7 1e"
> 004ad000 c0 3b 39 98 00 00 00 02 00 00 34 c8 00 00 00 00 |.;9.......4.....|
> 004ad010 75 a6 12 01 00 00 00 00 00 00 00 00 00 00 00 00 |u...............|
> 004ad020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
> 004ad030 00 00 00 00 62 e0 e7 1e 18 32 cf 18 00 00 00 00 |....b....2......|
> --
> The probability of this happening is very small, but we do, and I think it's a problem.
>
> There are two existing solutions:
> 1) Add "journal->j_tail_sequence = journal->j_transaction_sequence" in to the
> recover_ext3_journal in debugfs/journal.c.
> 2) There is a timestamp in the commit block, so we can add timestamp check when
> the log is replayed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] Tune2fs and fuse2fs do not change j_tail_sequence in journal superblock
2022-08-03 4:52 ` Alexey Lyahkov
@ 2022-08-04 8:30 ` zhanchengbin
0 siblings, 0 replies; 6+ messages in thread
From: zhanchengbin @ 2022-08-04 8:30 UTC (permalink / raw)
To: Alexey Lyahkov
Cc: Theodore Ts'o, linux-ext4, linfeilong, liuzhiqiang26, liangyun2
> Hi
>
> Thanks for you report.
> Problem much bigger than it. Debugs based code lack many parts of journal handling, including fast commit.
> Journal tag v3, and other.
I think we can solve the current problem first.
>
> Alex
>
>> On 2 Aug 2022, at 14:23, zhanchengbin <zhanchengbin1@huawei.com> wrote:
<snip>
>> There are two existing solutions:
>> 1) Add "journal->j_tail_sequence = journal->j_transaction_sequence" in to the
>> recover_ext3_journal in debugfs/journal.c.
I want to use the first solution, do you have any other solutions?
Thanks.
>> 2) There is a timestamp in the commit block, so we can add timestamp check when
>> the log is replayed.
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] tune2fs: do not change j_tail_sequence in journal superblock
2022-08-02 11:23 [BUG] Tune2fs and fuse2fs do not change j_tail_sequence in journal superblock zhanchengbin
2022-08-03 4:52 ` Alexey Lyahkov
@ 2022-08-04 10:33 ` zhanchengbin
2022-08-09 1:31 ` zhanchengbin
2022-08-13 1:12 ` Theodore Ts'o
1 sibling, 2 replies; 6+ messages in thread
From: zhanchengbin @ 2022-08-04 10:33 UTC (permalink / raw)
To: Theodore Ts'o
Cc: linux-ext4, linfeilong, liuzhiqiang26, liangyun2, Alexey Lyahkov
The function recover_ext3_journal in debugfs/journal.c, if the log
replay is over,
the j_tail_sequence in journal superblock is not changed to the value of
the last
transaction sequence, this will cause subsequent log commitids to count
from the
commitid in last time.
After tune2fs -e, the log commitid is counted from the commitid in last
time, if
the log ID of the current operation overlaps with that of the last
operation, this
will cause logs that were previously replayed by tune2fs to be replayed
here.
Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: liangyun <liangyun2@huawei.com>
---
debugfs/journal.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/debugfs/journal.c b/debugfs/journal.c
index 095fff00..5bac0d3b 100644
--- a/debugfs/journal.c
+++ b/debugfs/journal.c
@@ -769,6 +769,8 @@ static errcode_t recover_ext3_journal(ext2_filsys fs)
mark_buffer_dirty(journal->j_sb_buffer);
}
+ journal->j_tail_sequence = journal->j_transaction_sequence;
+
errout:
jbd2_journal_destroy_revoke(journal);
jbd2_journal_destroy_revoke_record_cache();
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tune2fs: do not change j_tail_sequence in journal superblock
2022-08-04 10:33 ` [PATCH] tune2fs: " zhanchengbin
@ 2022-08-09 1:31 ` zhanchengbin
2022-08-13 1:12 ` Theodore Ts'o
1 sibling, 0 replies; 6+ messages in thread
From: zhanchengbin @ 2022-08-09 1:31 UTC (permalink / raw)
To: Theodore Ts'o
Cc: linux-ext4, linfeilong, liuzhiqiang26, liangyun2, Alexey Lyahkov
Friendly ping...
On 2022/8/4 18:33, zhanchengbin wrote:
> The function recover_ext3_journal in debugfs/journal.c, if the log
> replay is over,
> the j_tail_sequence in journal superblock is not changed to the value of
> the last
> transaction sequence, this will cause subsequent log commitids to count
> from the
> commitid in last time.
> After tune2fs -e, the log commitid is counted from the commitid in last
> time, if
> the log ID of the current operation overlaps with that of the last
> operation, this
> will cause logs that were previously replayed by tune2fs to be replayed
> here.
>
> Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: liangyun <liangyun2@huawei.com>
> ---
> debugfs/journal.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/debugfs/journal.c b/debugfs/journal.c
> index 095fff00..5bac0d3b 100644
> --- a/debugfs/journal.c
> +++ b/debugfs/journal.c
> @@ -769,6 +769,8 @@ static errcode_t recover_ext3_journal(ext2_filsys fs)
> mark_buffer_dirty(journal->j_sb_buffer);
> }
>
> + journal->j_tail_sequence = journal->j_transaction_sequence;
> +
> errout:
> jbd2_journal_destroy_revoke(journal);
> jbd2_journal_destroy_revoke_record_cache();
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tune2fs: do not change j_tail_sequence in journal superblock
2022-08-04 10:33 ` [PATCH] tune2fs: " zhanchengbin
2022-08-09 1:31 ` zhanchengbin
@ 2022-08-13 1:12 ` Theodore Ts'o
1 sibling, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2022-08-13 1:12 UTC (permalink / raw)
To: zhanchengbin
Cc: linux-ext4, linfeilong, liuzhiqiang26, liangyun2, Alexey Lyahkov
On Thu, Aug 04, 2022 at 06:33:39PM +0800, zhanchengbin wrote:
> The function recover_ext3_journal in debugfs/journal.c, if the log replay is
> over,
> the j_tail_sequence in journal superblock is not changed to the value of the
> last
> transaction sequence, this will cause subsequent log commitids to count from
> the
> commitid in last time.
> After tune2fs -e, the log commitid is counted from the commitid in last
> time, if
> the log ID of the current operation overlaps with that of the last
> operation, this
> will cause logs that were previously replayed by tune2fs to be replayed
> here.
>
> Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: liangyun <liangyun2@huawei.com>
Applied, thanks!
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-13 1:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 11:23 [BUG] Tune2fs and fuse2fs do not change j_tail_sequence in journal superblock zhanchengbin
2022-08-03 4:52 ` Alexey Lyahkov
2022-08-04 8:30 ` zhanchengbin
2022-08-04 10:33 ` [PATCH] tune2fs: " zhanchengbin
2022-08-09 1:31 ` zhanchengbin
2022-08-13 1:12 ` Theodore Ts'o
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.