linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nikolay Borisov <nborisov@suse.com>
To: misono.tomohiro@jp.fujitsu.com
Cc: linux-btrfs@vger.kernel.org, Nikolay Borisov <nborisov@suse.com>
Subject: [PATCH] btrfs: qgroups: Move transaction managed inside btrfs_quota_enable
Date: Tue, 26 Jun 2018 10:09:58 +0300	[thread overview]
Message-ID: <1529996998-15451-1-git-send-email-nborisov@suse.com> (raw)
In-Reply-To: <0ced0e29-9ecc-01fb-b0a5-9de32966666e@jp.fujitsu.com>

Commit 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to
btrfs_quota_enable") not only resulted in an easier to follow code but
it also introduced a subtle bug. It changed the timing when the initial
transaction rescan was happening - before the commit it would happen
after transaction commit had occured but after the commit it might happen
before the transaction was committed. This results in failure to
correctly rescan the quota since there could be data which is still not
committed on disk.

This patch aims to fix this by movign the transaction creation/commit
inside btrfs_quota_enable, which allows to schedule the quota commit
after the transaction has been committed.

Fixes: 5d23515be669 ("btrfs: Move qgroup rescan on quota enable to btrfs_quota_enable")
Reported-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
Hi Misono, 

Care to test the following patch ? If you say it's ok I will do a similar one 
for the btrfs_quota_disable function. This will also allow me to get rid of 
the extra err variable in btrfs_ioctl_quota_ctl. Additionally I think the 
number of blocks (2) passed to the transaction for reservation might be 
wrong. 

 fs/btrfs/ioctl.c  |  2 +-
 fs/btrfs/qgroup.c | 17 ++++++++++++++---
 fs/btrfs/qgroup.h |  3 +--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a399750b9e41..bf99d7aae3ae 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5161,7 +5161,7 @@ static long btrfs_ioctl_quota_ctl(struct file *file, void __user *arg)
 
 	switch (sa->cmd) {
 	case BTRFS_QUOTA_CTL_ENABLE:
-		ret = btrfs_quota_enable(trans, fs_info);
+		ret = btrfs_quota_enable(fs_info);
 		break;
 	case BTRFS_QUOTA_CTL_DISABLE:
 		ret = btrfs_quota_disable(trans, fs_info);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 1874a6d2e6f5..91bb7e97c0d0 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -875,8 +875,7 @@ static int btrfs_clean_quota_tree(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-int btrfs_quota_enable(struct btrfs_trans_handle *trans,
-		       struct btrfs_fs_info *fs_info)
+int btrfs_quota_enable(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_root *quota_root;
 	struct btrfs_root *tree_root = fs_info->tree_root;
@@ -886,6 +885,7 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	struct btrfs_key found_key;
 	struct btrfs_qgroup *qgroup = NULL;
+	struct btrfs_trans_handle *trans = NULL;
 	int ret = 0;
 	int slot;
 
@@ -893,6 +893,12 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
 	if (fs_info->quota_root)
 		goto out;
 
+	trans = btrfs_start_transaction(tree_root, 2);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out;
+	}
+
 	fs_info->qgroup_ulist = ulist_alloc(GFP_KERNEL);
 	if (!fs_info->qgroup_ulist) {
 		ret = -ENOMEM;
@@ -987,6 +993,11 @@ int btrfs_quota_enable(struct btrfs_trans_handle *trans,
 	fs_info->quota_root = quota_root;
 	set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
 	spin_unlock(&fs_info->qgroup_lock);
+
+	ret = btrfs_commit_transaction(trans);
+	if (ret)
+		goto out_free_path;
+
 	ret = qgroup_rescan_init(fs_info, 0, 1);
 	if (!ret) {
 	        qgroup_rescan_zero_tracking(fs_info);
@@ -3061,7 +3072,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode,
 	if (free && reserved)
 		return qgroup_free_reserved_data(inode, reserved, start, len);
 	extent_changeset_init(&changeset);
-	ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, start, 
+	ret = clear_record_extent_bits(&BTRFS_I(inode)->io_tree, start,
 			start + len -1, EXTENT_QGROUP_RESERVED, &changeset);
 	if (ret < 0)
 		goto out;
diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
index d60dd06445ce..3900efab0e70 100644
--- a/fs/btrfs/qgroup.h
+++ b/fs/btrfs/qgroup.h
@@ -141,8 +141,7 @@ struct btrfs_qgroup {
 #define QGROUP_RELEASE		(1<<1)
 #define QGROUP_FREE		(1<<2)
 
-int btrfs_quota_enable(struct btrfs_trans_handle *trans,
-		       struct btrfs_fs_info *fs_info);
+int btrfs_quota_enable(struct btrfs_fs_info *fs_info);
 int btrfs_quota_disable(struct btrfs_trans_handle *trans,
 			struct btrfs_fs_info *fs_info);
 int btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info);
-- 
2.7.4


  parent reply	other threads:[~2018-06-26  7:10 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-26  6:00 Enabling quota may not correctly rescan on 4.17 Misono Tomohiro
2018-06-26  6:54 ` Nikolay Borisov
2018-06-26  7:09 ` Nikolay Borisov [this message]
2018-06-26  8:46   ` [PATCH] btrfs: qgroups: Move transaction managed inside btrfs_quota_enable Misono Tomohiro
2018-06-26  8:58     ` Nikolay Borisov
2018-06-27  8:09     ` Qu Wenruo
2018-06-27  7:40 ` Enabling quota may not correctly rescan on 4.17 Nikolay Borisov
2018-06-27  7:55   ` Misono Tomohiro
2018-06-27  8:04     ` Nikolay Borisov
2018-06-27  8:20       ` Misono Tomohiro
2018-06-27  8:22         ` Nikolay Borisov
2018-06-27  8:29           ` Misono Tomohiro
2018-06-27  8:10 ` Qu Wenruo
2018-06-27  8:25   ` Misono Tomohiro
2018-06-27  8:34     ` Qu Wenruo
2018-06-27  8:38       ` Qu Wenruo
2018-06-27  8:47         ` Nikolay Borisov
2018-06-27  8:57           ` Qu Wenruo
2018-06-27 10:12             ` Qu Wenruo
2018-06-28  7:12     ` Qu Wenruo
2018-06-28  8:10       ` Misono Tomohiro
2018-06-28  8:32         ` Nikolay Borisov
2018-07-02  8:15           ` Misono Tomohiro
  -- strict thread matches above, loose matches on Subject: below --
2018-07-02 11:00 [PATCH] btrfs: qgroups: Move transaction managed inside btrfs_quota_enable Nikolay Borisov
2018-07-02 11:02 ` Nikolay Borisov
2018-07-02 15:40 ` David Sterba
2018-07-03  8:54   ` Nikolay Borisov
2018-07-04 15:01     ` David Sterba
2018-07-03  6:27 ` Misono Tomohiro

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=1529996998-15451-1-git-send-email-nborisov@suse.com \
    --to=nborisov@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=misono.tomohiro@jp.fujitsu.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).