| Message ID | 93c382a002210831e1051456cdc5c44dbcef4562.1680185833.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: some trivial updates for delayed ref space reservation | expand |
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on kdave/for-next]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/fdmanana-kernel-org/btrfs-make-btrfs_block_rsv_full-check-more-boolean-when-starting-transaction/20230330-224056
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link: https://lore.kernel.org/r/93c382a002210831e1051456cdc5c44dbcef4562.1680185833.git.fdmanana%40suse.com
patch subject: [PATCH 2/2] btrfs: correctly calculate delayed ref bytes when starting transaction
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20230331/202303310221.Zjv7RAZT-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/7f30c221a11d9dc6fad3f763b3df7ecd0b6d966c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review fdmanana-kernel-org/btrfs-make-btrfs_block_rsv_full-check-more-boolean-when-starting-transaction/20230330-224056
git checkout 7f30c221a11d9dc6fad3f763b3df7ecd0b6d966c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash fs/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303310221.Zjv7RAZT-lkp@intel.com/
All errors (new ones prefixed by >>):
fs/btrfs/transaction.c: In function 'start_transaction':
>> fs/btrfs/transaction.c:611:46: error: implicit declaration of function 'btrfs_calc_delayed_ref_bytes'; did you mean 'btrfs_run_delayed_refs'? [-Werror=implicit-function-declaration]
611 | delayed_refs_bytes = btrfs_calc_delayed_ref_bytes(fs_info,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
| btrfs_run_delayed_refs
cc1: some warnings being treated as errors
vim +611 fs/btrfs/transaction.c
558
559 static struct btrfs_trans_handle *
560 start_transaction(struct btrfs_root *root, unsigned int num_items,
561 unsigned int type, enum btrfs_reserve_flush_enum flush,
562 bool enforce_qgroups)
563 {
564 struct btrfs_fs_info *fs_info = root->fs_info;
565 struct btrfs_block_rsv *delayed_refs_rsv = &fs_info->delayed_refs_rsv;
566 struct btrfs_trans_handle *h;
567 struct btrfs_transaction *cur_trans;
568 u64 num_bytes = 0;
569 u64 qgroup_reserved = 0;
570 bool reloc_reserved = false;
571 bool do_chunk_alloc = false;
572 int ret;
573
574 if (BTRFS_FS_ERROR(fs_info))
575 return ERR_PTR(-EROFS);
576
577 if (current->journal_info) {
578 WARN_ON(type & TRANS_EXTWRITERS);
579 h = current->journal_info;
580 refcount_inc(&h->use_count);
581 WARN_ON(refcount_read(&h->use_count) > 2);
582 h->orig_rsv = h->block_rsv;
583 h->block_rsv = NULL;
584 goto got_it;
585 }
586
587 /*
588 * Do the reservation before we join the transaction so we can do all
589 * the appropriate flushing if need be.
590 */
591 if (num_items && root != fs_info->chunk_root) {
592 struct btrfs_block_rsv *rsv = &fs_info->trans_block_rsv;
593 u64 delayed_refs_bytes = 0;
594
595 qgroup_reserved = num_items * fs_info->nodesize;
596 ret = btrfs_qgroup_reserve_meta_pertrans(root, qgroup_reserved,
597 enforce_qgroups);
598 if (ret)
599 return ERR_PTR(ret);
600
601 /*
602 * We want to reserve all the bytes we may need all at once, so
603 * we only do 1 enospc flushing cycle per transaction start. We
604 * accomplish this by simply assuming we'll do num_items worth
605 * of delayed refs updates in this trans handle, and refill that
606 * amount for whatever is missing in the reserve.
607 */
608 num_bytes = btrfs_calc_insert_metadata_size(fs_info, num_items);
609 if (flush == BTRFS_RESERVE_FLUSH_ALL &&
610 !btrfs_block_rsv_full(delayed_refs_rsv)) {
> 611 delayed_refs_bytes = btrfs_calc_delayed_ref_bytes(fs_info,
612 num_items);
613 num_bytes += delayed_refs_bytes;
614 }
615
616 /*
617 * Do the reservation for the relocation root creation
618 */
619 if (need_reserve_reloc_root(root)) {
620 num_bytes += fs_info->nodesize;
621 reloc_reserved = true;
622 }
623
624 ret = btrfs_block_rsv_add(fs_info, rsv, num_bytes, flush);
625 if (ret)
626 goto reserve_fail;
627 if (delayed_refs_bytes) {
628 btrfs_migrate_to_delayed_refs_rsv(fs_info, rsv,
629 delayed_refs_bytes);
630 num_bytes -= delayed_refs_bytes;
631 }
632
633 if (rsv->space_info->force_alloc)
634 do_chunk_alloc = true;
635 } else if (num_items == 0 && flush == BTRFS_RESERVE_FLUSH_ALL &&
636 !btrfs_block_rsv_full(delayed_refs_rsv)) {
637 /*
638 * Some people call with btrfs_start_transaction(root, 0)
639 * because they can be throttled, but have some other mechanism
640 * for reserving space. We still want these guys to refill the
641 * delayed block_rsv so just add 1 items worth of reservation
642 * here.
643 */
644 ret = btrfs_delayed_refs_rsv_refill(fs_info, flush);
645 if (ret)
646 goto reserve_fail;
647 }
648 again:
649 h = kmem_cache_zalloc(btrfs_trans_handle_cachep, GFP_NOFS);
650 if (!h) {
651 ret = -ENOMEM;
652 goto alloc_fail;
653 }
654
655 /*
656 * If we are JOIN_NOLOCK we're already committing a transaction and
657 * waiting on this guy, so we don't need to do the sb_start_intwrite
658 * because we're already holding a ref. We need this because we could
659 * have raced in and did an fsync() on a file which can kick a commit
660 * and then we deadlock with somebody doing a freeze.
661 *
662 * If we are ATTACH, it means we just want to catch the current
663 * transaction and commit it, so we needn't do sb_start_intwrite().
664 */
665 if (type & __TRANS_FREEZABLE)
666 sb_start_intwrite(fs_info->sb);
667
668 if (may_wait_transaction(fs_info, type))
669 wait_current_trans(fs_info);
670
671 do {
672 ret = join_transaction(fs_info, type);
673 if (ret == -EBUSY) {
674 wait_current_trans(fs_info);
675 if (unlikely(type == TRANS_ATTACH ||
676 type == TRANS_JOIN_NOSTART))
677 ret = -ENOENT;
678 }
679 } while (ret == -EBUSY);
680
681 if (ret < 0)
682 goto join_fail;
683
684 cur_trans = fs_info->running_transaction;
685
686 h->transid = cur_trans->transid;
687 h->transaction = cur_trans;
688 refcount_set(&h->use_count, 1);
689 h->fs_info = root->fs_info;
690
691 h->type = type;
692 INIT_LIST_HEAD(&h->new_bgs);
693
694 smp_mb();
695 if (cur_trans->state >= TRANS_STATE_COMMIT_START &&
696 may_wait_transaction(fs_info, type)) {
697 current->journal_info = h;
698 btrfs_commit_transaction(h);
699 goto again;
700 }
701
702 if (num_bytes) {
703 trace_btrfs_space_reservation(fs_info, "transaction",
704 h->transid, num_bytes, 1);
705 h->block_rsv = &fs_info->trans_block_rsv;
706 h->bytes_reserved = num_bytes;
707 h->reloc_reserved = reloc_reserved;
708 }
709
710 got_it:
711 if (!current->journal_info)
712 current->journal_info = h;
713
714 /*
715 * If the space_info is marked ALLOC_FORCE then we'll get upgraded to
716 * ALLOC_FORCE the first run through, and then we won't allocate for
717 * anybody else who races in later. We don't care about the return
718 * value here.
719 */
720 if (do_chunk_alloc && num_bytes) {
721 u64 flags = h->block_rsv->space_info->flags;
722
723 btrfs_chunk_alloc(h, btrfs_get_alloc_profile(fs_info, flags),
724 CHUNK_ALLOC_NO_FORCE);
725 }
726
727 /*
728 * btrfs_record_root_in_trans() needs to alloc new extents, and may
729 * call btrfs_join_transaction() while we're also starting a
730 * transaction.
731 *
732 * Thus it need to be called after current->journal_info initialized,
733 * or we can deadlock.
734 */
735 ret = btrfs_record_root_in_trans(h, root);
736 if (ret) {
737 /*
738 * The transaction handle is fully initialized and linked with
739 * other structures so it needs to be ended in case of errors,
740 * not just freed.
741 */
742 btrfs_end_transaction(h);
743 return ERR_PTR(ret);
744 }
745
746 return h;
747
748 join_fail:
749 if (type & __TRANS_FREEZABLE)
750 sb_end_intwrite(fs_info->sb);
751 kmem_cache_free(btrfs_trans_handle_cachep, h);
752 alloc_fail:
753 if (num_bytes)
754 btrfs_block_rsv_release(fs_info, &fs_info->trans_block_rsv,
755 num_bytes, NULL);
756 reserve_fail:
757 btrfs_qgroup_free_meta_pertrans(root, qgroup_reserved);
758 return ERR_PTR(ret);
759 }
760
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index a54a5c5a5db3..9e7ba07a35e8 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -601,15 +601,16 @@ start_transaction(struct btrfs_root *root, unsigned int num_items, /* * We want to reserve all the bytes we may need all at once, so * we only do 1 enospc flushing cycle per transaction start. We - * accomplish this by simply assuming we'll do 2 x num_items - * worth of delayed refs updates in this trans handle, and - * refill that amount for whatever is missing in the reserve. + * accomplish this by simply assuming we'll do num_items worth + * of delayed refs updates in this trans handle, and refill that + * amount for whatever is missing in the reserve. */ num_bytes = btrfs_calc_insert_metadata_size(fs_info, num_items); if (flush == BTRFS_RESERVE_FLUSH_ALL && !btrfs_block_rsv_full(delayed_refs_rsv)) { - delayed_refs_bytes = num_bytes; - num_bytes <<= 1; + delayed_refs_bytes = btrfs_calc_delayed_ref_bytes(fs_info, + num_items); + num_bytes += delayed_refs_bytes; } /*