Re: [PATCH v4] jbd2: avoid transaction reuse after reformatting

[Jan Kara <jack@suse.cz>]

On Fri 09-10-20 19:06:41, 常凤楠 wrote:
> Hi Jan:
> Thank you for your suggestions，I tested the new version of
> the patch，I think there still have some prblems.  1. Looks
> like you think jbd2_has_feature_checksum can determine that CRC is
> enabled，but this is different from jbd2_journal_has_csum_v2or3. so when
> csum is v2 or v3, this is still have problem.  2. This patch
> looks fixed the situations of descriptor and revoke block, commit block
> is not considered. Maybe it’s because my previous modification was
> problematic，I have a new idea, how about check crc first and compare
> timestap,if check crc is failed, then compare timestap, this way the risk
> will be much smaller. What do you think?

Hum, you're right that commit block checking will not work with v2/v3
checksums. Thanks for catching that! I like the order of checks you propose
to fix the problem, I'll update the patch. Thanks!

								Honza

> ------------------ Original ------------------
> From:                                                                                                                        "Theodore Y. Ts'o"                                                                                    <tytso@mit.edu&gt;;
> Date:&nbsp;Fri, Oct 9, 2020 10:16 AM
> To:&nbsp;"Andreas Dilger"<adilger@dilger.ca&gt;;
> Cc:&nbsp;"Jan Kara"<jack@suse.cz&gt;;"linux-ext4"<linux-ext4@vger.kernel.org&gt;;"Fengnan Chang"<changfengnan@hikvision.com&gt;;"常凤楠"<fengnanchang@foxmail.com&gt;;
> Subject:&nbsp;Re: [PATCH v4] jbd2: avoid transaction reuse after reformatting
> 
> 
> 
> On Thu, Oct 08, 2020 at 02:13:02PM -0600, Andreas Dilger wrote:
> &gt; On Oct 7, 2020, at 2:13 AM, Jan Kara <jack@suse.cz&gt; wrote:
> &gt; &gt; 
> &gt; &gt; From: changfengnan <fengnanchang@foxmail.com&gt;
> &gt; &gt; 
> &gt; &gt; When ext4 is formatted with lazy_journal_init=1 and transactions from
> &gt; &gt; the previous filesystem are still on disk, it is possible that they are
> &gt; &gt; considered during a recovery after a crash. Because the checksum seed
> &gt; &gt; has changed, the CRC check will fail, and the journal recovery fails
> &gt; &gt; with checksum error although the journal is otherwise perfectly valid.
> &gt; &gt; Fix the problem by checking commit block time stamps to determine
> &gt; &gt; whether the data in the journal block is just stale or whether it is
> &gt; &gt; indeed corrupt.
> &gt; &gt; 
> &gt; &gt; Reported-by: kernel test robot <lkp@intel.com&gt;
> &gt; &gt; Signed-off-by: Fengnan Chang <changfengnan@hikvision.com&gt;
> &gt; &gt; Signed-off-by: Jan Kara <jack@suse.cz&gt;
> &gt; 
> &gt; Reviewed-by: Andreas Dilger <adilger@dilger.ca&gt;
> &gt; 
> &gt; NB: one trivial formatting cleanup if patch is refreshed
> &gt;
> 
> Applied, thanks.&nbsp; I fixed the trivial format cleanup you pointed out,
> plus a whitespace fix pointed out by checkpatch.
> 
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 		&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 	&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 	&nbsp;&nbsp;&nbsp;&nbsp; - Ted
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR