* Re: [PATCH, realtime-rmap branch] xfs: lock the RT bitmap inode in xfs_efi_item_recover
2023-10-31 9:50 [PATCH, realtime-rmap branch] xfs: lock the RT bitmap inode in xfs_efi_item_recover Christoph Hellwig
@ 2023-10-31 16:47 ` kernel test robot
2023-10-31 16:53 ` Darrick J. Wong
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2023-10-31 16:47 UTC (permalink / raw)
To: Christoph Hellwig, djwong; +Cc: oe-kbuild-all, linux-xfs
Hi Christoph,
kernel test robot noticed the following build errors:
[auto build test ERROR on v6.6]
[also build test ERROR on linus/master next-20231031]
[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/Christoph-Hellwig/xfs-lock-the-RT-bitmap-inode-in-xfs_efi_item_recover/20231031-175736
base: v6.6
patch link: https://lore.kernel.org/r/20231031095038.1559309-1-hch%40lst.de
patch subject: [PATCH, realtime-rmap branch] xfs: lock the RT bitmap inode in xfs_efi_item_recover
config: loongarch-randconfig-002-20231031 (https://download.01.org/0day-ci/archive/20231101/202311010017.ZOItqKn9-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231101/202311010017.ZOItqKn9-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311010017.ZOItqKn9-lkp@intel.com/
All errors (new ones prefixed by >>):
fs/xfs/xfs_extfree_item.c: In function 'xfs_efi_item_recover':
>> fs/xfs/xfs_extfree_item.c:707:29: error: implicit declaration of function 'xfs_efi_is_realtime'; did you mean 'xfs_has_realtime'? [-Werror=implicit-function-declaration]
707 | if (xfs_efi_is_realtime(&fake))
| ^~~~~~~~~~~~~~~~~~~
| xfs_has_realtime
>> fs/xfs/xfs_extfree_item.c:708:33: error: implicit declaration of function 'xfs_rtbitmap_lock' [-Werror=implicit-function-declaration]
708 | xfs_rtbitmap_lock(tp, mp);
| ^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +707 fs/xfs/xfs_extfree_item.c
653
654 /*
655 * Process an extent free intent item that was recovered from
656 * the log. We need to free the extents that it describes.
657 */
658 STATIC int
659 xfs_efi_item_recover(
660 struct xfs_log_item *lip,
661 struct list_head *capture_list)
662 {
663 struct xfs_trans_res resv;
664 struct xfs_efi_log_item *efip = EFI_ITEM(lip);
665 struct xfs_mount *mp = lip->li_log->l_mp;
666 struct xfs_efd_log_item *efdp;
667 struct xfs_trans *tp;
668 int i;
669 int error = 0;
670 bool requeue_only = false;
671
672 /*
673 * First check the validity of the extents described by the
674 * EFI. If any are bad, then assume that all are bad and
675 * just toss the EFI.
676 */
677 for (i = 0; i < efip->efi_format.efi_nextents; i++) {
678 if (!xfs_efi_validate_ext(mp,
679 &efip->efi_format.efi_extents[i])) {
680 XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
681 &efip->efi_format,
682 sizeof(efip->efi_format));
683 return -EFSCORRUPTED;
684 }
685 }
686
687 resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
688 error = xfs_trans_alloc(mp, &resv, 0, 0, 0, &tp);
689 if (error)
690 return error;
691 efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
692
693 for (i = 0; i < efip->efi_format.efi_nextents; i++) {
694 struct xfs_extent_free_item fake = {
695 .xefi_owner = XFS_RMAP_OWN_UNKNOWN,
696 .xefi_agresv = XFS_AG_RESV_NONE,
697 };
698 struct xfs_extent *extp;
699
700 extp = &efip->efi_format.efi_extents[i];
701
702 fake.xefi_startblock = extp->ext_start;
703 fake.xefi_blockcount = extp->ext_len;
704
705 if (!requeue_only) {
706 xfs_extent_free_get_group(mp, &fake);
> 707 if (xfs_efi_is_realtime(&fake))
> 708 xfs_rtbitmap_lock(tp, mp);
709 error = xfs_trans_free_extent(tp, efdp, &fake);
710 xfs_extent_free_put_group(&fake);
711 }
712
713 /*
714 * If we can't free the extent without potentially deadlocking,
715 * requeue the rest of the extents to a new so that they get
716 * run again later with a new transaction context.
717 */
718 if (error == -EAGAIN || requeue_only) {
719 error = xfs_free_extent_later(tp, fake.xefi_startblock,
720 fake.xefi_blockcount,
721 &XFS_RMAP_OINFO_ANY_OWNER,
722 fake.xefi_agresv);
723 if (!error) {
724 requeue_only = true;
725 continue;
726 }
727 }
728
729 if (error == -EFSCORRUPTED)
730 XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
731 extp, sizeof(*extp));
732 if (error)
733 goto abort_error;
734
735 }
736
737 return xfs_defer_ops_capture_and_commit(tp, capture_list);
738
739 abort_error:
740 xfs_trans_cancel(tp);
741 return error;
742 }
743
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH, realtime-rmap branch] xfs: lock the RT bitmap inode in xfs_efi_item_recover
2023-10-31 9:50 [PATCH, realtime-rmap branch] xfs: lock the RT bitmap inode in xfs_efi_item_recover Christoph Hellwig
2023-10-31 16:47 ` kernel test robot
@ 2023-10-31 16:53 ` Darrick J. Wong
2023-10-31 17:14 ` Christoph Hellwig
1 sibling, 1 reply; 4+ messages in thread
From: Darrick J. Wong @ 2023-10-31 16:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Tue, Oct 31, 2023 at 10:50:38AM +0100, Christoph Hellwig wrote:
> xfs_trans_free_extent expects the rtbitmap and rtsum inodes to be locked.
> Ensure that is the case during log recovery as well.
>
> Fixes: 3ea32f5cc5f9 ("xfs: support logging EFIs for realtime extents")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>
> I found this when testing the realtime-rmap branch with additional
> patches on top. Probably makes sense to just fold it in for the next
> rebase of that branch.
>
> fs/xfs/xfs_extfree_item.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 5b3e7dca4e1ba0..070070b6401d66 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -768,6 +768,8 @@ xfs_efi_item_recover(
>
> if (!requeue_only) {
> xfs_extent_free_get_group(mp, &fake);
> + if (xfs_efi_is_realtime(&fake))
> + xfs_rtbitmap_lock(tp, mp);
Curious, I thought that patch "xfs: support logging EFIs for realtime
extents" already locked the rt bitmap?
resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
error = xfs_trans_alloc(mp, &resv, 0, 0, 0, &tp);
if (error)
return error;
efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
/* Lock the rt bitmap if we've any realtime extents to free. */
for (i = 0; i < efip->efi_format.efi_nextents; i++) {
struct xfs_extent *extp;
extp = &efip->efi_format.efi_extents[i];
if (extp->ext_len & XFS_EFI_EXTLEN_REALTIME_EXT) {
xfs_rtbitmap_lock(tp, mp);
Here ^^^^^^^^^
break;
}
}
for (i = 0; i < efip->efi_format.efi_nextents; i++) {
struct xfs_extent_free_item fake = {
.xefi_owner = XFS_RMAP_OWN_UNKNOWN,
.xefi_agresv = XFS_AG_RESV_NONE,
};
struct xfs_extent *extp;
unsigned int len;
extp = &efip->efi_format.efi_extents[i];
fake.xefi_startblock = extp->ext_start;
len = extp->ext_len;
if (len & XFS_EFI_EXTLEN_REALTIME_EXT) {
len &= ~XFS_EFI_EXTLEN_REALTIME_EXT;
fake.xefi_flags |= XFS_EFI_REALTIME;
}
fake.xefi_blockcount = len;
if (!requeue_only) {
xfs_extent_free_get_group(mp, &fake);
error = xfs_trans_free_extent(tp, efdp, &fake);
xfs_extent_free_put_group(&fake);
}
So that by the time we get to this part of the loop, the rtbitmap will
be locked already?
What sort of error message did you get?
<confused>
--D
> error = xfs_trans_free_extent(tp, efdp, &fake);
> xfs_extent_free_put_group(&fake);
> }
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread