linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: dsterba@suse.cz, jeffm@suse.com
Subject: [PATCH 14/14] Revert "btrfs: qgroups: Retry after commit on getting EDQUOT"
Date: Tue, 12 Dec 2017 15:34:36 +0800	[thread overview]
Message-ID: <20171212073436.16447-15-wqu@suse.com> (raw)
In-Reply-To: <20171212073436.16447-1-wqu@suse.com>

This reverts commit 48a89bc4f2ceab87bc858a8eb189636b09c846a7.

The idea to commit transaction and free some space after hitting qgroup
limit is good, although the problem is it will easily cause deadlocks.

One deadlock example is caused by trying to flush data while still
holding it:
Call Trace:
 __schedule+0x49d/0x10f0
 schedule+0xc6/0x290
 schedule_timeout+0x187/0x1c0
 wait_for_completion+0x204/0x3a0
 btrfs_wait_ordered_extents+0xa40/0xaf0 [btrfs]
 qgroup_reserve+0x913/0xa10 [btrfs]
 btrfs_qgroup_reserve_data+0x3ef/0x580 [btrfs]
 btrfs_check_data_free_space+0x96/0xd0 [btrfs]
 __btrfs_buffered_write+0x3ac/0xd40 [btrfs]
 btrfs_file_write_iter+0x62a/0xba0 [btrfs]
 __vfs_write+0x320/0x430
 vfs_write+0x107/0x270
 SyS_write+0xbf/0x150
 do_syscall_64+0x1b0/0x3d0
 entry_SYSCALL64_slow_path+0x25/0x25

Another case can be caused by trying to commit one transaction while
nesting with trans handler hold by ourselves:
btrfs_start_transaction()
|- btrfs_qgroup_reserve_meta_pertrans()
   |- qgroup_reserve()
      |- btrfs_join_transaction()
      |- btrfs_commit_transaction()

The retry is causing more problem than expectation when limit is
enabled.
At least graceful EDQUOT is way better than kernel deadlock.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index ee5b05dd10a9..02effd919452 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2416,7 +2416,6 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce,
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	u64 ref_root = root->root_key.objectid;
 	int ret = 0;
-	int retried = 0;
 	struct ulist_node *unode;
 	struct ulist_iterator uiter;
 
@@ -2457,27 +2456,6 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce,
 		qg = unode_aux_to_qgroup(unode);
 
 		if (enforce && !qgroup_check_limits(qg, num_bytes)) {
-			/*
-			 * Commit the tree and retry, since we may have
-			 * deletions which would free up space.
-			 */
-			if (!retried && qgroup_rsv_total(qg) > 0) {
-				struct btrfs_trans_handle *trans;
-
-				spin_unlock(&fs_info->qgroup_lock);
-				ret = btrfs_start_delalloc_inodes(root, 0);
-				if (ret)
-					return ret;
-				btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
-				trans = btrfs_join_transaction(root);
-				if (IS_ERR(trans))
-					return PTR_ERR(trans);
-				ret = btrfs_commit_transaction(trans);
-				if (ret)
-					return ret;
-				retried++;
-				goto retry;
-			}
 			ret = -EDQUOT;
 			goto out;
 		}
-- 
2.15.1


  parent reply	other threads:[~2017-12-12  7:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-12  7:34 [PATCH 00/14] Qgroup metadata reservation rework Qu Wenruo
2017-12-12  7:34 ` [PATCH 01/14] btrfs: qgroup: Skeleton to support separate qgroup reservation type Qu Wenruo
2017-12-12  7:34 ` [PATCH 02/14] btrfs: qgroup: Introduce helpers to update and access new qgroup rsv Qu Wenruo
2017-12-21 15:23   ` Nikolay Borisov
2017-12-12  7:34 ` [PATCH 03/14] btrfs: qgroup: Make qgroup_reserve and its callers to use separate reservation type Qu Wenruo
2017-12-12  7:34 ` [PATCH 04/14] btrfs: qgroup: Fix wrong qgroup reservation update for relationship modification Qu Wenruo
2017-12-12  7:34 ` [PATCH 05/14] btrfs: qgroup: Update trace events to use new separate rsv types Qu Wenruo
2017-12-12  7:34 ` [PATCH 06/14] btrfs: qgroup: Cleanup the remaining old reservation counters Qu Wenruo
2017-12-12  7:34 ` [PATCH 07/14] btrfs: qgroup: Split meta rsv type into meta_prealloc and meta_pertrans Qu Wenruo
2017-12-12  7:34 ` [PATCH 08/14] btrfs: qgroup: Don't use root->qgroup_meta_rsv for qgroup Qu Wenruo
2017-12-12  7:34 ` [PATCH 09/14] btrfs: qgroup: Introduce function to convert META_PREALLOC into META_PERTRANS Qu Wenruo
2017-12-12  7:34 ` [PATCH 10/14] btrfs: qgroup: Use separate meta reservation type for delalloc Qu Wenruo
2017-12-12  7:34 ` [PATCH 11/14] btrfs: delayed-inode: Use new qgroup meta rsv for delayed inode and item Qu Wenruo
2017-12-12  7:34 ` [PATCH 12/14] btrfs: qgroup: Use root->qgroup_meta_rsv_* to record qgroup meta reserved space Qu Wenruo
2017-12-12  7:34 ` [PATCH 13/14] btrfs: qgroup: Update trace events for metadata reservation Qu Wenruo
2017-12-12  7:34 ` Qu Wenruo [this message]
2017-12-12 14:16 ` [PATCH 00/14] Qgroup metadata reservation rework Nikolay Borisov
2017-12-12 18:01   ` David Sterba
2017-12-13  0:54     ` Qu Wenruo
2017-12-12 21:12 ` David Sterba
2017-12-13  0:55   ` Qu Wenruo
2018-03-26 14:10     ` David Sterba
2018-03-26 23:49       ` Qu Wenruo
2018-03-27 15:23         ` David Sterba
2018-03-27 18:00           ` Filipe Manana
2018-03-27 16:30         ` David Sterba

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=20171212073436.16447-15-wqu@suse.com \
    --to=wqu@suse.com \
    --cc=dsterba@suse.cz \
    --cc=jeffm@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    /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).