On Tue 06-10-20 21:06:06, kernel test robot wrote: > Hi Jan, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on ext4/dev] > [also build test WARNING on linus/master v5.9-rc8 next-20201002] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] Thanks for report! I'll send v4 with sparse warning fixed up (it isn't a functional problem but it is good to have it fixed). Honza > > url: https://github.com/0day-ci/linux/commits/Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337 > base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev > config: i386-randconfig-s001-20201005 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > reproduce: > # apt-get install sparse > # sparse version: v0.6.2-201-g24bdaac6-dirty > # https://github.com/0day-ci/linux/commit/ca606dc0beaec78e8b2b6ec2f112b2192534d9e2 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Jan-Kara/jbd2-avoid-transaction-reuse-after-reformatting/20201006-183337 > git checkout ca606dc0beaec78e8b2b6ec2f112b2192534d9e2 > # save the attached .config to linux build tree > make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > echo > echo "sparse warnings: (new ones prefixed by >>)" > echo > >> fs/jbd2/recovery.c:713:41: sparse: sparse: incorrect type in initializer (different base types) @@ expected restricted __be64 [usertype] commit_time @@ got unsigned long long @@ > fs/jbd2/recovery.c:713:41: sparse: expected restricted __be64 [usertype] commit_time > >> fs/jbd2/recovery.c:713:41: sparse: got unsigned long long > fs/jbd2/recovery.c:730:45: sparse: sparse: restricted __be64 degrades to integer > fs/jbd2/recovery.c:730:60: sparse: sparse: restricted __be64 degrades to integer > > vim +713 fs/jbd2/recovery.c > > 415 > 416 static int do_one_pass(journal_t *journal, > 417 struct recovery_info *info, enum passtype pass) > 418 { > 419 unsigned int first_commit_ID, next_commit_ID; > 420 unsigned long next_log_block; > 421 int err, success = 0; > 422 journal_superblock_t * sb; > 423 journal_header_t * tmp; > 424 struct buffer_head * bh; > 425 unsigned int sequence; > 426 int blocktype; > 427 int tag_bytes = journal_tag_bytes(journal); > 428 __u32 crc32_sum = ~0; /* Transactional Checksums */ > 429 int descr_csum_size = 0; > 430 int block_error = 0; > 431 bool need_check_commit_time = false; > 432 __be64 last_trans_commit_time = 0; > 433 > 434 /* > 435 * First thing is to establish what we expect to find in the log > 436 * (in terms of transaction IDs), and where (in terms of log > 437 * block offsets): query the superblock. > 438 */ > 439 > 440 sb = journal->j_superblock; > 441 next_commit_ID = be32_to_cpu(sb->s_sequence); > 442 next_log_block = be32_to_cpu(sb->s_start); > 443 > 444 first_commit_ID = next_commit_ID; > 445 if (pass == PASS_SCAN) > 446 info->start_transaction = first_commit_ID; > 447 > 448 jbd_debug(1, "Starting recovery pass %d\n", pass); > 449 > 450 /* > 451 * Now we walk through the log, transaction by transaction, > 452 * making sure that each transaction has a commit block in the > 453 * expected place. Each complete transaction gets replayed back > 454 * into the main filesystem. > 455 */ > 456 > 457 while (1) { > 458 int flags; > 459 char * tagp; > 460 journal_block_tag_t * tag; > 461 struct buffer_head * obh; > 462 struct buffer_head * nbh; > 463 > 464 cond_resched(); > 465 > 466 /* If we already know where to stop the log traversal, > 467 * check right now that we haven't gone past the end of > 468 * the log. */ > 469 > 470 if (pass != PASS_SCAN) > 471 if (tid_geq(next_commit_ID, info->end_transaction)) > 472 break; > 473 > 474 jbd_debug(2, "Scanning for sequence ID %u at %lu/%lu\n", > 475 next_commit_ID, next_log_block, journal->j_last); > 476 > 477 /* Skip over each chunk of the transaction looking > 478 * either the next descriptor block or the final commit > 479 * record. */ > 480 > 481 jbd_debug(3, "JBD2: checking block %ld\n", next_log_block); > 482 err = jread(&bh, journal, next_log_block); > 483 if (err) > 484 goto failed; > 485 > 486 next_log_block++; > 487 wrap(journal, next_log_block); > 488 > 489 /* What kind of buffer is it? > 490 * > 491 * If it is a descriptor block, check that it has the > 492 * expected sequence number. Otherwise, we're all done > 493 * here. */ > 494 > 495 tmp = (journal_header_t *)bh->b_data; > 496 > 497 if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER)) { > 498 brelse(bh); > 499 break; > 500 } > 501 > 502 blocktype = be32_to_cpu(tmp->h_blocktype); > 503 sequence = be32_to_cpu(tmp->h_sequence); > 504 jbd_debug(3, "Found magic %d, sequence %d\n", > 505 blocktype, sequence); > 506 > 507 if (sequence != next_commit_ID) { > 508 brelse(bh); > 509 break; > 510 } > 511 > 512 /* OK, we have a valid descriptor block which matches > 513 * all of the sequence number checks. What are we going > 514 * to do with it? That depends on the pass... */ > 515 > 516 switch(blocktype) { > 517 case JBD2_DESCRIPTOR_BLOCK: > 518 /* Verify checksum first */ > 519 if (jbd2_journal_has_csum_v2or3(journal)) > 520 descr_csum_size = > 521 sizeof(struct jbd2_journal_block_tail); > 522 if (descr_csum_size > 0 && > 523 !jbd2_descriptor_block_csum_verify(journal, > 524 bh->b_data)) { > 525 /* > 526 * PASS_SCAN can see stale blocks due to lazy > 527 * journal init. Don't error out on those yet. > 528 */ > 529 if (pass != PASS_SCAN) { > 530 pr_err("JBD2: Invalid checksum " > 531 "recovering block %lu in log\n", > 532 next_log_block); > 533 err = -EFSBADCRC; > 534 brelse(bh); > 535 goto failed; > 536 } > 537 need_check_commit_time = true; > 538 jbd_debug(1, > 539 "invalid descriptor block found in %lu\n", > 540 next_log_block); > 541 } > 542 > 543 /* If it is a valid descriptor block, replay it > 544 * in pass REPLAY; if journal_checksums enabled, then > 545 * calculate checksums in PASS_SCAN, otherwise, > 546 * just skip over the blocks it describes. */ > 547 if (pass != PASS_REPLAY) { > 548 if (pass == PASS_SCAN && > 549 jbd2_has_feature_checksum(journal) && > 550 !need_check_commit_time && > 551 !info->end_transaction) { > 552 if (calc_chksums(journal, bh, > 553 &next_log_block, > 554 &crc32_sum)) { > 555 put_bh(bh); > 556 break; > 557 } > 558 put_bh(bh); > 559 continue; > 560 } > 561 next_log_block += count_tags(journal, bh); > 562 wrap(journal, next_log_block); > 563 put_bh(bh); > 564 continue; > 565 } > 566 > 567 /* A descriptor block: we can now write all of > 568 * the data blocks. Yay, useful work is finally > 569 * getting done here! */ > 570 > 571 tagp = &bh->b_data[sizeof(journal_header_t)]; > 572 while ((tagp - bh->b_data + tag_bytes) > 573 <= journal->j_blocksize - descr_csum_size) { > 574 unsigned long io_block; > 575 > 576 tag = (journal_block_tag_t *) tagp; > 577 flags = be16_to_cpu(tag->t_flags); > 578 > 579 io_block = next_log_block++; > 580 wrap(journal, next_log_block); > 581 err = jread(&obh, journal, io_block); > 582 if (err) { > 583 /* Recover what we can, but > 584 * report failure at the end. */ > 585 success = err; > 586 printk(KERN_ERR > 587 "JBD2: IO error %d recovering " > 588 "block %ld in log\n", > 589 err, io_block); > 590 } else { > 591 unsigned long long blocknr; > 592 > 593 J_ASSERT(obh != NULL); > 594 blocknr = read_tag_block(journal, > 595 tag); > 596 > 597 /* If the block has been > 598 * revoked, then we're all done > 599 * here. */ > 600 if (jbd2_journal_test_revoke > 601 (journal, blocknr, > 602 next_commit_ID)) { > 603 brelse(obh); > 604 ++info->nr_revoke_hits; > 605 goto skip_write; > 606 } > 607 > 608 /* Look for block corruption */ > 609 if (!jbd2_block_tag_csum_verify( > 610 journal, tag, obh->b_data, > 611 be32_to_cpu(tmp->h_sequence))) { > 612 brelse(obh); > 613 success = -EFSBADCRC; > 614 printk(KERN_ERR "JBD2: Invalid " > 615 "checksum recovering " > 616 "data block %llu in " > 617 "log\n", blocknr); > 618 block_error = 1; > 619 goto skip_write; > 620 } > 621 > 622 /* Find a buffer for the new > 623 * data being restored */ > 624 nbh = __getblk(journal->j_fs_dev, > 625 blocknr, > 626 journal->j_blocksize); > 627 if (nbh == NULL) { > 628 printk(KERN_ERR > 629 "JBD2: Out of memory " > 630 "during recovery.\n"); > 631 err = -ENOMEM; > 632 brelse(bh); > 633 brelse(obh); > 634 goto failed; > 635 } > 636 > 637 lock_buffer(nbh); > 638 memcpy(nbh->b_data, obh->b_data, > 639 journal->j_blocksize); > 640 if (flags & JBD2_FLAG_ESCAPE) { > 641 *((__be32 *)nbh->b_data) = > 642 cpu_to_be32(JBD2_MAGIC_NUMBER); > 643 } > 644 > 645 BUFFER_TRACE(nbh, "marking dirty"); > 646 set_buffer_uptodate(nbh); > 647 mark_buffer_dirty(nbh); > 648 BUFFER_TRACE(nbh, "marking uptodate"); > 649 ++info->nr_replays; > 650 /* ll_rw_block(WRITE, 1, &nbh); */ > 651 unlock_buffer(nbh); > 652 brelse(obh); > 653 brelse(nbh); > 654 } > 655 > 656 skip_write: > 657 tagp += tag_bytes; > 658 if (!(flags & JBD2_FLAG_SAME_UUID)) > 659 tagp += 16; > 660 > 661 if (flags & JBD2_FLAG_LAST_TAG) > 662 break; > 663 } > 664 > 665 brelse(bh); > 666 continue; > 667 > 668 case JBD2_COMMIT_BLOCK: > 669 /* How to differentiate between interrupted commit > 670 * and journal corruption ? > 671 * > 672 * {nth transaction} > 673 * Checksum Verification Failed > 674 * | > 675 * ____________________ > 676 * | | > 677 * async_commit sync_commit > 678 * | | > 679 * | GO TO NEXT "Journal Corruption" > 680 * | TRANSACTION > 681 * | > 682 * {(n+1)th transanction} > 683 * | > 684 * _______|______________ > 685 * | | > 686 * Commit block found Commit block not found > 687 * | | > 688 * "Journal Corruption" | > 689 * _____________|_________ > 690 * | | > 691 * nth trans corrupt OR nth trans > 692 * and (n+1)th interrupted interrupted > 693 * before commit block > 694 * could reach the disk. > 695 * (Cannot find the difference in above > 696 * mentioned conditions. Hence assume > 697 * "Interrupted Commit".) > 698 */ > 699 > 700 /* > 701 * Found an expected commit block: if checksums > 702 * are present, verify them in PASS_SCAN; else not > 703 * much to do other than move on to the next sequence > 704 * number. > 705 */ > 706 if (pass == PASS_SCAN && > 707 jbd2_has_feature_checksum(journal)) { > 708 struct commit_header *cbh = > 709 (struct commit_header *)bh->b_data; > 710 unsigned found_chksum = > 711 be32_to_cpu(cbh->h_chksum[0]); > 712 __be64 commit_time = > > 713 be64_to_cpu(cbh->h_commit_sec); > 714 > 715 if (info->end_transaction) { > 716 journal->j_failed_commit = > 717 info->end_transaction; > 718 brelse(bh); > 719 break; > 720 } > 721 > 722 /* > 723 * If need_check_commit_time is set, it means > 724 * csum verify failed before, if commit_time is > 725 * increasing, it's same journal, otherwise it > 726 * is stale journal block, just end this > 727 * recovery. > 728 */ > 729 if (need_check_commit_time) { > 730 if (commit_time >= last_trans_commit_time) { > 731 pr_err("JBD2: Invalid checksum found in transaction %u\n", > 732 next_commit_ID); > 733 err = -EFSBADCRC; > 734 brelse(bh); > 735 goto failed; > 736 } > 737 /* > 738 * It likely does not belong to same > 739 * journal, just end this recovery with > 740 * success. > 741 */ > 742 jbd_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n", > 743 next_commit_ID); > 744 err = 0; > 745 brelse(bh); > 746 goto done; > 747 } > 748 > 749 /* Neither checksum match nor unused? */ > 750 if (!((crc32_sum == found_chksum && > 751 cbh->h_chksum_type == > 752 JBD2_CRC32_CHKSUM && > 753 cbh->h_chksum_size == > 754 JBD2_CRC32_CHKSUM_SIZE) || > 755 (cbh->h_chksum_type == 0 && > 756 cbh->h_chksum_size == 0 && > 757 found_chksum == 0))) > 758 goto chksum_error; > 759 > 760 crc32_sum = ~0; > 761 last_trans_commit_time = commit_time; > 762 } > 763 if (pass == PASS_SCAN && > 764 !jbd2_commit_block_csum_verify(journal, > 765 bh->b_data)) { > 766 chksum_error: > 767 info->end_transaction = next_commit_ID; > 768 > 769 if (!jbd2_has_feature_async_commit(journal)) { > 770 journal->j_failed_commit = > 771 next_commit_ID; > 772 brelse(bh); > 773 break; > 774 } > 775 } > 776 brelse(bh); > 777 next_commit_ID++; > 778 continue; > 779 > 780 case JBD2_REVOKE_BLOCK: > 781 /* > 782 * Check revoke block crc in pass_scan, if csum verify > 783 * failed, check commit block time later. > 784 */ > 785 if (pass == PASS_SCAN && > 786 !jbd2_descriptor_block_csum_verify(journal, > 787 bh->b_data)) { > 788 jbd_debug(1, "JBD2: invalid revoke block found in %lu\n", > 789 next_log_block); > 790 need_check_commit_time = true; > 791 } > 792 /* If we aren't in the REVOKE pass, then we can > 793 * just skip over this block. */ > 794 if (pass != PASS_REVOKE) { > 795 brelse(bh); > 796 continue; > 797 } > 798 > 799 err = scan_revoke_records(journal, bh, > 800 next_commit_ID, info); > 801 brelse(bh); > 802 if (err) > 803 goto failed; > 804 continue; > 805 > 806 default: > 807 jbd_debug(3, "Unrecognised magic %d, end of scan.\n", > 808 blocktype); > 809 brelse(bh); > 810 goto done; > 811 } > 812 } > 813 > 814 done: > 815 /* > 816 * We broke out of the log scan loop: either we came to the > 817 * known end of the log or we found an unexpected block in the > 818 * log. If the latter happened, then we know that the "current" > 819 * transaction marks the end of the valid log. > 820 */ > 821 > 822 if (pass == PASS_SCAN) { > 823 if (!info->end_transaction) > 824 info->end_transaction = next_commit_ID; > 825 } else { > 826 /* It's really bad news if different passes end up at > 827 * different places (but possible due to IO errors). */ > 828 if (info->end_transaction != next_commit_ID) { > 829 printk(KERN_ERR "JBD2: recovery pass %d ended at " > 830 "transaction %u, expected %u\n", > 831 pass, next_commit_ID, info->end_transaction); > 832 if (!success) > 833 success = -EIO; > 834 } > 835 } > 836 if (block_error && success == 0) > 837 success = -EIO; > 838 return success; > 839 > 840 failed: > 841 return err; > 842 } > 843 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
