linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Filipe Manana <fdmanana@suse.com>,
	Qu Wenruo <wqu@suse.com>, David Sterba <dsterba@suse.com>,
	Anand Jain <anand.jain@oracle.com>
Subject: [PATCH 5.4 25/27] btrfs: qgroup: dont commit transaction when we already hold the handle
Date: Fri, 13 Aug 2021 17:07:23 +0200	[thread overview]
Message-ID: <20210813150524.221417812@linuxfoundation.org> (raw)
In-Reply-To: <20210813150523.364549385@linuxfoundation.org>

From: Qu Wenruo <wqu@suse.com>

commit 6f23277a49e68f8a9355385c846939ad0b1261e7 upstream

[BUG]
When running the following script, btrfs will trigger an ASSERT():

  #/bin/bash
  mkfs.btrfs -f $dev
  mount $dev $mnt
  xfs_io -f -c "pwrite 0 1G" $mnt/file
  sync
  btrfs quota enable $mnt
  btrfs quota rescan -w $mnt

  # Manually set the limit below current usage
  btrfs qgroup limit 512M $mnt $mnt

  # Crash happens
  touch $mnt/file

The dmesg looks like this:

  assertion failed: refcount_read(&trans->use_count) == 1, in fs/btrfs/transaction.c:2022
  ------------[ cut here ]------------
  kernel BUG at fs/btrfs/ctree.h:3230!
  invalid opcode: 0000 [#1] SMP PTI
  RIP: 0010:assertfail.constprop.0+0x18/0x1a [btrfs]
   btrfs_commit_transaction.cold+0x11/0x5d [btrfs]
   try_flush_qgroup+0x67/0x100 [btrfs]
   __btrfs_qgroup_reserve_meta+0x3a/0x60 [btrfs]
   btrfs_delayed_update_inode+0xaa/0x350 [btrfs]
   btrfs_update_inode+0x9d/0x110 [btrfs]
   btrfs_dirty_inode+0x5d/0xd0 [btrfs]
   touch_atime+0xb5/0x100
   iterate_dir+0xf1/0x1b0
   __x64_sys_getdents64+0x78/0x110
   do_syscall_64+0x33/0x80
   entry_SYSCALL_64_after_hwframe+0x44/0xa9
  RIP: 0033:0x7fb5afe588db

[CAUSE]
In try_flush_qgroup(), we assume we don't hold a transaction handle at
all.  This is true for data reservation and mostly true for metadata.
Since data space reservation always happens before we start a
transaction, and for most metadata operation we reserve space in
start_transaction().

But there is an exception, btrfs_delayed_inode_reserve_metadata().
It holds a transaction handle, while still trying to reserve extra
metadata space.

When we hit EDQUOT inside btrfs_delayed_inode_reserve_metadata(), we
will join current transaction and commit, while we still have
transaction handle from qgroup code.

[FIX]
Let's check current->journal before we join the transaction.

If current->journal is unset or BTRFS_SEND_TRANS_STUB, it means
we are not holding a transaction, thus are able to join and then commit
transaction.

If current->journal is a valid transaction handle, we avoid committing
transaction and just end it

This is less effective than committing current transaction, as it won't
free metadata reserved space, but we may still free some data space
before new data writes.

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1178634
Fixes: c53e9653605d ("btrfs: qgroup: try to flush qgroup space when we get -EDQUOT")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/btrfs/qgroup.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -3502,6 +3502,7 @@ static int try_flush_qgroup(struct btrfs
 {
 	struct btrfs_trans_handle *trans;
 	int ret;
+	bool can_commit = true;
 
 	/*
 	 * We don't want to run flush again and again, so if there is a running
@@ -3513,6 +3514,20 @@ static int try_flush_qgroup(struct btrfs
 		return 0;
 	}
 
+	/*
+	 * If current process holds a transaction, we shouldn't flush, as we
+	 * assume all space reservation happens before a transaction handle is
+	 * held.
+	 *
+	 * But there are cases like btrfs_delayed_item_reserve_metadata() where
+	 * we try to reserve space with one transction handle already held.
+	 * In that case we can't commit transaction, but at least try to end it
+	 * and hope the started data writes can free some space.
+	 */
+	if (current->journal_info &&
+	    current->journal_info != BTRFS_SEND_TRANS_STUB)
+		can_commit = false;
+
 	ret = btrfs_start_delalloc_snapshot(root);
 	if (ret < 0)
 		goto out;
@@ -3524,7 +3539,10 @@ static int try_flush_qgroup(struct btrfs
 		goto out;
 	}
 
-	ret = btrfs_commit_transaction(trans);
+	if (can_commit)
+		ret = btrfs_commit_transaction(trans);
+	else
+		ret = btrfs_end_transaction(trans);
 out:
 	clear_bit(BTRFS_ROOT_QGROUP_FLUSHING, &root->state);
 	wake_up(&root->qgroup_flush_wait);



  parent reply	other threads:[~2021-08-13 15:15 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-13 15:06 [PATCH 5.4 00/27] 5.4.141-rc1 review Greg Kroah-Hartman
2021-08-13 15:06 ` [PATCH 5.4 01/27] KVM: SVM: Fix off-by-one indexing when nullifying last used SEV VMCB Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 02/27] tee: Correct inappropriate usage of TEE_SHM_DMA_BUF flag Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 03/27] media: v4l2-mem2mem: always consider OUTPUT queue during poll Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 04/27] tracing: Reject string operand in the histogram expression Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 05/27] usb: dwc3: Stop active transfers before halting the controller Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 06/27] usb: dwc3: gadget: Allow runtime suspend if UDC unbinded Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 07/27] usb: dwc3: gadget: Restart DWC3 gadget when enabling pullup Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 08/27] usb: dwc3: gadget: Prevent EP queuing while stopping transfers Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 09/27] usb: dwc3: gadget: Clear DEP flags after stop transfers in ep disable Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 10/27] usb: dwc3: gadget: Disable gadget IRQ during pullup disable Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 11/27] usb: dwc3: gadget: Avoid runtime resume if disabling pullup Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 12/27] KVM: X86: MMU: Use the correct inherited permissions to get shadow page Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 13/27] USB:ehci:fix Kunpeng920 ehci hardware problem Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 14/27] ALSA: hda: Add quirk for ASUS Flow x13 Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 15/27] ppp: Fix generating ppp unit id when ifname is not specified Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 16/27] ovl: prevent private clone if bind mount is not allowed Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 17/27] btrfs: make qgroup_free_reserved_data take btrfs_inode Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 18/27] btrfs: make btrfs_qgroup_reserve_data " Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 19/27] btrfs: qgroup: allow to unreserve range without releasing other ranges Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 20/27] btrfs: qgroup: try to flush qgroup space when we get -EDQUOT Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 21/27] btrfs: transaction: Cleanup unused TRANS_STATE_BLOCKED Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 22/27] btrfs: qgroup: remove ASYNC_COMMIT mechanism in favor of reserve retry-after-EDQUOT Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 23/27] btrfs: fix lockdep splat when enabling and disabling qgroups Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 24/27] net: xilinx_emaclite: Do not print real IOMEM pointer Greg Kroah-Hartman
2021-08-13 15:07 ` Greg Kroah-Hartman [this message]
2021-08-13 15:07 ` [PATCH 5.4 26/27] btrfs: export and rename qgroup_reserve_meta Greg Kroah-Hartman
2021-08-13 15:07 ` [PATCH 5.4 27/27] btrfs: dont flush from btrfs_delayed_inode_reserve_metadata Greg Kroah-Hartman
2021-08-13 23:24 ` [PATCH 5.4 00/27] 5.4.141-rc1 review Shuah Khan
2021-08-14 11:11 ` Sudip Mukherjee
2021-08-14 11:39 ` Naresh Kamboju
2021-08-14 18:15 ` Guenter Roeck
2021-08-16  3:02 ` Samuel Zou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210813150524.221417812@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=anand.jain@oracle.com \
    --cc=dsterba@suse.com \
    --cc=fdmanana@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).