All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: quota: Introduce new flag to cancel quota rescan
@ 2021-06-17 12:34 Marcos Paulo de Souza
  2021-06-17 12:44 ` Qu Wenruo
  0 siblings, 1 reply; 4+ messages in thread
From: Marcos Paulo de Souza @ 2021-06-17 12:34 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba, wqu, Marcos Paulo de Souza

Until now, there wasn't a way to cancel a currently running quota
rescan. The QUOTA_CTL ioctl has only commands to enable and disable,
thus, no way to cancel and restart the rescan. This functionality can be
useful to applications like snapper, that can create and delete
snapshots, and thus restarting the quota scan can be used instead of
waiting for the current scan to finish.

The new command BTRFS_QUOTA_CTL_CANCEL_RESCAN can now be used in
QUOTA_CTL ioctl to reset the progress of the rescan and stopping
btrfs_qgroup_rescan_worker.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---

 Can someone check if locking only qgrop_ioctl_lock is enough for this case, or
 if I missed something? Thanks!

 fs/btrfs/ctree.h           |  1 +
 fs/btrfs/ioctl.c           |  3 +++
 fs/btrfs/qgroup.c          | 29 +++++++++++++++++++++++++++++
 fs/btrfs/qgroup.h          |  1 +
 include/uapi/linux/btrfs.h |  1 +
 5 files changed, 35 insertions(+)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 6131b58f779f..e1f2153d4a42 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -933,6 +933,7 @@ struct btrfs_fs_info {
 
 	/* qgroup rescan items */
 	struct mutex qgroup_rescan_lock; /* protects the progress item */
+	bool qgroup_cancel_rescan;
 	struct btrfs_key qgroup_rescan_progress;
 	struct btrfs_workqueue *qgroup_rescan_workers;
 	struct completion qgroup_rescan_completion;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 0ba98e08a029..b39b6ff4650f 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4206,6 +4206,9 @@ static long btrfs_ioctl_quota_ctl(struct file *file, void __user *arg)
 	case BTRFS_QUOTA_CTL_DISABLE:
 		ret = btrfs_quota_disable(fs_info);
 		break;
+	case BTRFS_QUOTA_CTL_CANCEL_RESCAN:
+		ret = btrfs_quota_cancel_rescan(fs_info);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index d72885903b8c..077021fc63c8 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1233,6 +1233,21 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
 	return ret;
 }
 
+int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info)
+{
+	int ret;
+
+	mutex_lock(&fs_info->qgroup_ioctl_lock);
+	fs_info->qgroup_cancel_rescan = true;
+
+	ret = btrfs_qgroup_wait_for_completion(fs_info, false);
+
+	fs_info->qgroup_cancel_rescan = false;
+	mutex_unlock(&fs_info->qgroup_ioctl_lock);
+
+	return ret;
+}
+
 static void qgroup_dirty(struct btrfs_fs_info *fs_info,
 			 struct btrfs_qgroup *qgroup)
 {
@@ -3214,6 +3229,7 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 	int err = -ENOMEM;
 	int ret = 0;
 	bool stopped = false;
+	bool canceled = false;
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -3234,6 +3250,9 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 		}
 		if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)) {
 			err = -EINTR;
+		} else if (fs_info->qgroup_cancel_rescan) {
+			canceled = true;
+			err = -ECANCELED;
 		} else {
 			err = qgroup_rescan_leaf(trans, path);
 		}
@@ -3280,6 +3299,14 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 		}
 	}
 	fs_info->qgroup_rescan_running = false;
+
+	/*
+	 * If we cancel the current rescan, set progress to zero to start over
+	 * on next rescan.
+	 */
+	if (canceled)
+		fs_info->qgroup_rescan_progress.objectid = 0;
+
 	complete_all(&fs_info->qgroup_rescan_completion);
 	mutex_unlock(&fs_info->qgroup_rescan_lock);
 
@@ -3290,6 +3317,8 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 
 	if (stopped) {
 		btrfs_info(fs_info, "qgroup scan paused");
+	} else if (canceled) {
+		btrfs_info(fs_info, "qgroup scan canceled");
 	} else if (err >= 0) {
 		btrfs_info(fs_info, "qgroup scan completed%s",
 			err > 0 ? " (inconsistency flag cleared)" : "");
diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
index 7283e4f549af..ae6a42312ab8 100644
--- a/fs/btrfs/qgroup.h
+++ b/fs/btrfs/qgroup.h
@@ -245,6 +245,7 @@ static inline u64 btrfs_qgroup_subvolid(u64 qgroupid)
 
 int btrfs_quota_enable(struct btrfs_fs_info *fs_info);
 int btrfs_quota_disable(struct btrfs_fs_info *fs_info);
+int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info);
 int btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info);
 void btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info);
 int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info *fs_info,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 22cd037123fa..29a66dd01df7 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -714,6 +714,7 @@ struct btrfs_ioctl_get_dev_stats {
 #define BTRFS_QUOTA_CTL_ENABLE	1
 #define BTRFS_QUOTA_CTL_DISABLE	2
 #define BTRFS_QUOTA_CTL_RESCAN__NOTUSED	3
+#define BTRFS_QUOTA_CTL_CANCEL_RESCAN 4
 struct btrfs_ioctl_quota_ctl_args {
 	__u64 cmd;
 	__u64 status;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: quota: Introduce new flag to cancel quota rescan
  2021-06-17 12:34 [PATCH] btrfs: quota: Introduce new flag to cancel quota rescan Marcos Paulo de Souza
@ 2021-06-17 12:44 ` Qu Wenruo
  2021-07-28 16:49   ` Marcos Paulo de Souza
  0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2021-06-17 12:44 UTC (permalink / raw)
  To: Marcos Paulo de Souza, linux-btrfs; +Cc: dsterba, wqu



On 2021/6/17 下午8:34, Marcos Paulo de Souza wrote:
> Until now, there wasn't a way to cancel a currently running quota
> rescan. The QUOTA_CTL ioctl has only commands to enable and disable,
> thus, no way to cancel and restart the rescan. This functionality can be
> useful to applications like snapper, that can create and delete
> snapshots, and thus restarting the quota scan can be used instead of
> waiting for the current scan to finish.

This implies there are some operations which would make qgroup
accounting go crazy when doing rescan.

My first idea is doing qgroup inherit during rescan.

As we can't handle it now, we will make the fs with INCONSISTENT bit,
and forget about it.

But if a rescan is running, then when it finishes, the INCONSISTENT bit
get cleared, giving a false view of the qgroup numbers.

I hope the commit message can have better background on this part.

>
> The new command BTRFS_QUOTA_CTL_CANCEL_RESCAN can now be used in
> QUOTA_CTL ioctl to reset the progress of the rescan and stopping
> btrfs_qgroup_rescan_worker.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>
>   Can someone check if locking only qgrop_ioctl_lock is enough for this case, or
>   if I missed something? Thanks!
>
>   fs/btrfs/ctree.h           |  1 +
>   fs/btrfs/ioctl.c           |  3 +++
>   fs/btrfs/qgroup.c          | 29 +++++++++++++++++++++++++++++
>   fs/btrfs/qgroup.h          |  1 +
>   include/uapi/linux/btrfs.h |  1 +
>   5 files changed, 35 insertions(+)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 6131b58f779f..e1f2153d4a42 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -933,6 +933,7 @@ struct btrfs_fs_info {
>
>   	/* qgroup rescan items */
>   	struct mutex qgroup_rescan_lock; /* protects the progress item */
> +	bool qgroup_cancel_rescan;
>   	struct btrfs_key qgroup_rescan_progress;
>   	struct btrfs_workqueue *qgroup_rescan_workers;
>   	struct completion qgroup_rescan_completion;
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 0ba98e08a029..b39b6ff4650f 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4206,6 +4206,9 @@ static long btrfs_ioctl_quota_ctl(struct file *file, void __user *arg)
>   	case BTRFS_QUOTA_CTL_DISABLE:
>   		ret = btrfs_quota_disable(fs_info);
>   		break;
> +	case BTRFS_QUOTA_CTL_CANCEL_RESCAN:
> +		ret = btrfs_quota_cancel_rescan(fs_info);
> +		break;
>   	default:
>   		ret = -EINVAL;
>   		break;
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index d72885903b8c..077021fc63c8 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1233,6 +1233,21 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
>   	return ret;
>   }
>
> +int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info)
> +{
> +	int ret;
> +
> +	mutex_lock(&fs_info->qgroup_ioctl_lock);
> +	fs_info->qgroup_cancel_rescan = true;
> +
> +	ret = btrfs_qgroup_wait_for_completion(fs_info, false);
> +
> +	fs_info->qgroup_cancel_rescan = false;
> +	mutex_unlock(&fs_info->qgroup_ioctl_lock);
> +
> +	return ret;
> +}
> +
>   static void qgroup_dirty(struct btrfs_fs_info *fs_info,
>   			 struct btrfs_qgroup *qgroup)
>   {
> @@ -3214,6 +3229,7 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
>   	int err = -ENOMEM;
>   	int ret = 0;
>   	bool stopped = false;
> +	bool canceled = false;
>
>   	path = btrfs_alloc_path();
>   	if (!path)
> @@ -3234,6 +3250,9 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
>   		}
>   		if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)) {
>   			err = -EINTR;
> +		} else if (fs_info->qgroup_cancel_rescan) {
> +			canceled = true;
> +			err = -ECANCELED;
>   		} else {
>   			err = qgroup_rescan_leaf(trans, path);
>   		}
> @@ -3280,6 +3299,14 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
>   		}
>   	}
>   	fs_info->qgroup_rescan_running = false;
> +
> +	/*
> +	 * If we cancel the current rescan, set progress to zero to start over
> +	 * on next rescan.
> +	 */
> +	if (canceled)
> +		fs_info->qgroup_rescan_progress.objectid = 0;
> +

Not sure if this is needed.

>   	complete_all(&fs_info->qgroup_rescan_completion);
>   	mutex_unlock(&fs_info->qgroup_rescan_lock);
>
> @@ -3290,6 +3317,8 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
>
>   	if (stopped) {
>   		btrfs_info(fs_info, "qgroup scan paused");
> +	} else if (canceled) {
> +		btrfs_info(fs_info, "qgroup scan canceled");
>   	} else if (err >= 0) {
>   		btrfs_info(fs_info, "qgroup scan completed%s",
>   			err > 0 ? " (inconsistency flag cleared)" : "");
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index 7283e4f549af..ae6a42312ab8 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -245,6 +245,7 @@ static inline u64 btrfs_qgroup_subvolid(u64 qgroupid)
>
>   int btrfs_quota_enable(struct btrfs_fs_info *fs_info);
>   int btrfs_quota_disable(struct btrfs_fs_info *fs_info);
> +int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info);
>   int btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info);
>   void btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info);
>   int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info *fs_info,
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 22cd037123fa..29a66dd01df7 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -714,6 +714,7 @@ struct btrfs_ioctl_get_dev_stats {
>   #define BTRFS_QUOTA_CTL_ENABLE	1
>   #define BTRFS_QUOTA_CTL_DISABLE	2
>   #define BTRFS_QUOTA_CTL_RESCAN__NOTUSED	3
> +#define BTRFS_QUOTA_CTL_CANCEL_RESCAN 4

Well, this needs to be determined by David.

Despite that, it looks OK to me.

Thanks,
Qu

>   struct btrfs_ioctl_quota_ctl_args {
>   	__u64 cmd;
>   	__u64 status;
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: quota: Introduce new flag to cancel quota rescan
  2021-06-17 12:44 ` Qu Wenruo
@ 2021-07-28 16:49   ` Marcos Paulo de Souza
  2021-08-11 13:03     ` Marcos Paulo de Souza
  0 siblings, 1 reply; 4+ messages in thread
From: Marcos Paulo de Souza @ 2021-07-28 16:49 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

On Thu, 2021-06-17 at 20:44 +0800, Qu Wenruo wrote:
> 
> On 2021/6/17 下午8:34, Marcos Paulo de Souza wrote:
> > Until now, there wasn't a way to cancel a currently running quota
> > rescan. The QUOTA_CTL ioctl has only commands to enable and
> > disable,
> > thus, no way to cancel and restart the rescan. This functionality
> > can be
> > useful to applications like snapper, that can create and delete
> > snapshots, and thus restarting the quota scan can be used instead
> > of
> > waiting for the current scan to finish.
> 
> This implies there are some operations which would make qgroup
> accounting go crazy when doing rescan.
> 
> My first idea is doing qgroup inherit during rescan.
> 
> As we can't handle it now, we will make the fs with INCONSISTENT bit,
> and forget about it.
> 
> But if a rescan is running, then when it finishes, the INCONSISTENT
> bit
> get cleared, giving a false view of the qgroup numbers.
> 
> I hope the commit message can have better background on this part.
> 
> > The new command BTRFS_QUOTA_CTL_CANCEL_RESCAN can now be used in
> > QUOTA_CTL ioctl to reset the progress of the rescan and stopping
> > btrfs_qgroup_rescan_worker.
> > 
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> > 
> >   Can someone check if locking only qgrop_ioctl_lock is enough for
> > this case, or
> >   if I missed something? Thanks!
> > 
> >   fs/btrfs/ctree.h           |  1 +
> >   fs/btrfs/ioctl.c           |  3 +++
> >   fs/btrfs/qgroup.c          | 29 +++++++++++++++++++++++++++++
> >   fs/btrfs/qgroup.h          |  1 +
> >   include/uapi/linux/btrfs.h |  1 +
> >   5 files changed, 35 insertions(+)
> > 
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index 6131b58f779f..e1f2153d4a42 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -933,6 +933,7 @@ struct btrfs_fs_info {
> > 
> >   	/* qgroup rescan items */
> >   	struct mutex qgroup_rescan_lock; /* protects the progress item
> > */
> > +	bool qgroup_cancel_rescan;
> >   	struct btrfs_key qgroup_rescan_progress;
> >   	struct btrfs_workqueue *qgroup_rescan_workers;
> >   	struct completion qgroup_rescan_completion;
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index 0ba98e08a029..b39b6ff4650f 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -4206,6 +4206,9 @@ static long btrfs_ioctl_quota_ctl(struct file
> > *file, void __user *arg)
> >   	case BTRFS_QUOTA_CTL_DISABLE:
> >   		ret = btrfs_quota_disable(fs_info);
> >   		break;
> > +	case BTRFS_QUOTA_CTL_CANCEL_RESCAN:
> > +		ret = btrfs_quota_cancel_rescan(fs_info);
> > +		break;
> >   	default:
> >   		ret = -EINVAL;
> >   		break;
> > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> > index d72885903b8c..077021fc63c8 100644
> > --- a/fs/btrfs/qgroup.c
> > +++ b/fs/btrfs/qgroup.c
> > @@ -1233,6 +1233,21 @@ int btrfs_quota_disable(struct btrfs_fs_info
> > *fs_info)
> >   	return ret;
> >   }
> > 
> > +int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&fs_info->qgroup_ioctl_lock);
> > +	fs_info->qgroup_cancel_rescan = true;
> > +
> > +	ret = btrfs_qgroup_wait_for_completion(fs_info, false);
> > +
> > +	fs_info->qgroup_cancel_rescan = false;
> > +	mutex_unlock(&fs_info->qgroup_ioctl_lock);
> > +
> > +	return ret;
> > +}
> > +
> >   static void qgroup_dirty(struct btrfs_fs_info *fs_info,
> >   			 struct btrfs_qgroup *qgroup)
> >   {
> > @@ -3214,6 +3229,7 @@ static void btrfs_qgroup_rescan_worker(struct
> > btrfs_work *work)
> >   	int err = -ENOMEM;
> >   	int ret = 0;
> >   	bool stopped = false;
> > +	bool canceled = false;
> > 
> >   	path = btrfs_alloc_path();
> >   	if (!path)
> > @@ -3234,6 +3250,9 @@ static void btrfs_qgroup_rescan_worker(struct
> > btrfs_work *work)
> >   		}
> >   		if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)) 
> > {
> >   			err = -EINTR;
> > +		} else if (fs_info->qgroup_cancel_rescan) {
> > +			canceled = true;
> > +			err = -ECANCELED;
> >   		} else {
> >   			err = qgroup_rescan_leaf(trans, path);
> >   		}
> > @@ -3280,6 +3299,14 @@ static void
> > btrfs_qgroup_rescan_worker(struct btrfs_work *work)
> >   		}
> >   	}
> >   	fs_info->qgroup_rescan_running = false;
> > +
> > +	/*
> > +	 * If we cancel the current rescan, set progress to zero to
> > start over
> > +	 * on next rescan.
> > +	 */
> > +	if (canceled)
> > +		fs_info->qgroup_rescan_progress.objectid = 0;
> > +
> 
> Not sure if this is needed.
> 
> >   	complete_all(&fs_info->qgroup_rescan_completion);
> >   	mutex_unlock(&fs_info->qgroup_rescan_lock);
> > 
> > @@ -3290,6 +3317,8 @@ static void btrfs_qgroup_rescan_worker(struct
> > btrfs_work *work)
> > 
> >   	if (stopped) {
> >   		btrfs_info(fs_info, "qgroup scan paused");
> > +	} else if (canceled) {
> > +		btrfs_info(fs_info, "qgroup scan canceled");
> >   	} else if (err >= 0) {
> >   		btrfs_info(fs_info, "qgroup scan completed%s",
> >   			err > 0 ? " (inconsistency flag cleared)" :
> > "");
> > diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> > index 7283e4f549af..ae6a42312ab8 100644
> > --- a/fs/btrfs/qgroup.h
> > +++ b/fs/btrfs/qgroup.h
> > @@ -245,6 +245,7 @@ static inline u64 btrfs_qgroup_subvolid(u64
> > qgroupid)
> > 
> >   int btrfs_quota_enable(struct btrfs_fs_info *fs_info);
> >   int btrfs_quota_disable(struct btrfs_fs_info *fs_info);
> > +int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info);
> >   int btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info);
> >   void btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info);
> >   int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info
> > *fs_info,
> > diff --git a/include/uapi/linux/btrfs.h
> > b/include/uapi/linux/btrfs.h
> > index 22cd037123fa..29a66dd01df7 100644
> > --- a/include/uapi/linux/btrfs.h
> > +++ b/include/uapi/linux/btrfs.h
> > @@ -714,6 +714,7 @@ struct btrfs_ioctl_get_dev_stats {
> >   #define BTRFS_QUOTA_CTL_ENABLE	1
> >   #define BTRFS_QUOTA_CTL_DISABLE	2
> >   #define BTRFS_QUOTA_CTL_RESCAN__NOTUSED	3
> > +#define BTRFS_QUOTA_CTL_CANCEL_RESCAN 4
> 
> Well, this needs to be determined by David.

Ping David :)

> 
> Despite that, it looks OK to me.
> 
> Thanks,
> Qu
> 
> >   struct btrfs_ioctl_quota_ctl_args {
> >   	__u64 cmd;
> >   	__u64 status;
> > 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: quota: Introduce new flag to cancel quota rescan
  2021-07-28 16:49   ` Marcos Paulo de Souza
@ 2021-08-11 13:03     ` Marcos Paulo de Souza
  0 siblings, 0 replies; 4+ messages in thread
From: Marcos Paulo de Souza @ 2021-08-11 13:03 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

On Wed, 2021-07-28 at 13:49 -0300, Marcos Paulo de Souza wrote:
> On Thu, 2021-06-17 at 20:44 +0800, Qu Wenruo wrote:
> > On 2021/6/17 下午8:34, Marcos Paulo de Souza wrote:
> > > Until now, there wasn't a way to cancel a currently running quota
> > > rescan. The QUOTA_CTL ioctl has only commands to enable and
> > > disable,
> > > thus, no way to cancel and restart the rescan. This functionality
> > > can be
> > > useful to applications like snapper, that can create and delete
> > > snapshots, and thus restarting the quota scan can be used instead
> > > of
> > > waiting for the current scan to finish.
> > 
> > This implies there are some operations which would make qgroup
> > accounting go crazy when doing rescan.
> > 
> > My first idea is doing qgroup inherit during rescan.
> > 
> > As we can't handle it now, we will make the fs with INCONSISTENT
> > bit,
> > and forget about it.
> > 
> > But if a rescan is running, then when it finishes, the INCONSISTENT
> > bit
> > get cleared, giving a false view of the qgroup numbers.
> > 
> > I hope the commit message can have better background on this part.
> > 
> > > The new command BTRFS_QUOTA_CTL_CANCEL_RESCAN can now be used in
> > > QUOTA_CTL ioctl to reset the progress of the rescan and stopping
> > > btrfs_qgroup_rescan_worker.
> > > 
> > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > > ---
> > > 
> > >   Can someone check if locking only qgrop_ioctl_lock is enough
> > > for
> > > this case, or
> > >   if I missed something? Thanks!
> > > 
> > >   fs/btrfs/ctree.h           |  1 +
> > >   fs/btrfs/ioctl.c           |  3 +++
> > >   fs/btrfs/qgroup.c          | 29 +++++++++++++++++++++++++++++
> > >   fs/btrfs/qgroup.h          |  1 +
> > >   include/uapi/linux/btrfs.h |  1 +
> > >   5 files changed, 35 insertions(+)
> > > 
> > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > > index 6131b58f779f..e1f2153d4a42 100644
> > > --- a/fs/btrfs/ctree.h
> > > +++ b/fs/btrfs/ctree.h
> > > @@ -933,6 +933,7 @@ struct btrfs_fs_info {
> > > 
> > >   	/* qgroup rescan items */
> > >   	struct mutex qgroup_rescan_lock; /* protects the
> > > progress item
> > > */
> > > +	bool qgroup_cancel_rescan;
> > >   	struct btrfs_key qgroup_rescan_progress;
> > >   	struct btrfs_workqueue *qgroup_rescan_workers;
> > >   	struct completion qgroup_rescan_completion;
> > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > > index 0ba98e08a029..b39b6ff4650f 100644
> > > --- a/fs/btrfs/ioctl.c
> > > +++ b/fs/btrfs/ioctl.c
> > > @@ -4206,6 +4206,9 @@ static long btrfs_ioctl_quota_ctl(struct
> > > file
> > > *file, void __user *arg)
> > >   	case BTRFS_QUOTA_CTL_DISABLE:
> > >   		ret = btrfs_quota_disable(fs_info);
> > >   		break;
> > > +	case BTRFS_QUOTA_CTL_CANCEL_RESCAN:
> > > +		ret = btrfs_quota_cancel_rescan(fs_info);
> > > +		break;
> > >   	default:
> > >   		ret = -EINVAL;
> > >   		break;
> > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> > > index d72885903b8c..077021fc63c8 100644
> > > --- a/fs/btrfs/qgroup.c
> > > +++ b/fs/btrfs/qgroup.c
> > > @@ -1233,6 +1233,21 @@ int btrfs_quota_disable(struct
> > > btrfs_fs_info
> > > *fs_info)
> > >   	return ret;
> > >   }
> > > 
> > > +int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info)
> > > +{
> > > +	int ret;
> > > +
> > > +	mutex_lock(&fs_info->qgroup_ioctl_lock);
> > > +	fs_info->qgroup_cancel_rescan = true;
> > > +
> > > +	ret = btrfs_qgroup_wait_for_completion(fs_info, false);
> > > +
> > > +	fs_info->qgroup_cancel_rescan = false;
> > > +	mutex_unlock(&fs_info->qgroup_ioctl_lock);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >   static void qgroup_dirty(struct btrfs_fs_info *fs_info,
> > >   			 struct btrfs_qgroup *qgroup)
> > >   {
> > > @@ -3214,6 +3229,7 @@ static void
> > > btrfs_qgroup_rescan_worker(struct
> > > btrfs_work *work)
> > >   	int err = -ENOMEM;
> > >   	int ret = 0;
> > >   	bool stopped = false;
> > > +	bool canceled = false;
> > > 
> > >   	path = btrfs_alloc_path();
> > >   	if (!path)
> > > @@ -3234,6 +3250,9 @@ static void
> > > btrfs_qgroup_rescan_worker(struct
> > > btrfs_work *work)
> > >   		}
> > >   		if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info-
> > > >flags)) 
> > > {
> > >   			err = -EINTR;
> > > +		} else if (fs_info->qgroup_cancel_rescan) {
> > > +			canceled = true;
> > > +			err = -ECANCELED;
> > >   		} else {
> > >   			err = qgroup_rescan_leaf(trans, path);
> > >   		}
> > > @@ -3280,6 +3299,14 @@ static void
> > > btrfs_qgroup_rescan_worker(struct btrfs_work *work)
> > >   		}
> > >   	}
> > >   	fs_info->qgroup_rescan_running = false;
> > > +
> > > +	/*
> > > +	 * If we cancel the current rescan, set progress to zero to
> > > start over
> > > +	 * on next rescan.
> > > +	 */
> > > +	if (canceled)
> > > +		fs_info->qgroup_rescan_progress.objectid = 0;
> > > +
> > 
> > Not sure if this is needed.
> > 
> > >   	complete_all(&fs_info->qgroup_rescan_completion);
> > >   	mutex_unlock(&fs_info->qgroup_rescan_lock);
> > > 
> > > @@ -3290,6 +3317,8 @@ static void
> > > btrfs_qgroup_rescan_worker(struct
> > > btrfs_work *work)
> > > 
> > >   	if (stopped) {
> > >   		btrfs_info(fs_info, "qgroup scan paused");
> > > +	} else if (canceled) {
> > > +		btrfs_info(fs_info, "qgroup scan canceled");
> > >   	} else if (err >= 0) {
> > >   		btrfs_info(fs_info, "qgroup scan completed%s",
> > >   			err > 0 ? " (inconsistency flag
> > > cleared)" :
> > > "");
> > > diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> > > index 7283e4f549af..ae6a42312ab8 100644
> > > --- a/fs/btrfs/qgroup.h
> > > +++ b/fs/btrfs/qgroup.h
> > > @@ -245,6 +245,7 @@ static inline u64 btrfs_qgroup_subvolid(u64
> > > qgroupid)
> > > 
> > >   int btrfs_quota_enable(struct btrfs_fs_info *fs_info);
> > >   int btrfs_quota_disable(struct btrfs_fs_info *fs_info);
> > > +int btrfs_quota_cancel_rescan(struct btrfs_fs_info *fs_info);
> > >   int btrfs_qgroup_rescan(struct btrfs_fs_info *fs_info);
> > >   void btrfs_qgroup_rescan_resume(struct btrfs_fs_info *fs_info);
> > >   int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info
> > > *fs_info,
> > > diff --git a/include/uapi/linux/btrfs.h
> > > b/include/uapi/linux/btrfs.h
> > > index 22cd037123fa..29a66dd01df7 100644
> > > --- a/include/uapi/linux/btrfs.h
> > > +++ b/include/uapi/linux/btrfs.h
> > > @@ -714,6 +714,7 @@ struct btrfs_ioctl_get_dev_stats {
> > >   #define BTRFS_QUOTA_CTL_ENABLE	1
> > >   #define BTRFS_QUOTA_CTL_DISABLE	2
> > >   #define BTRFS_QUOTA_CTL_RESCAN__NOTUSED	3
> > > +#define BTRFS_QUOTA_CTL_CANCEL_RESCAN 4
> > 
> > Well, this needs to be determined by David.
> 
> Ping David :)

Ping v2 :)

> 
> > Despite that, it looks OK to me.
> > 
> > Thanks,
> > Qu
> > 
> > >   struct btrfs_ioctl_quota_ctl_args {
> > >   	__u64 cmd;
> > >   	__u64 status;
> > > 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-11 13:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 12:34 [PATCH] btrfs: quota: Introduce new flag to cancel quota rescan Marcos Paulo de Souza
2021-06-17 12:44 ` Qu Wenruo
2021-07-28 16:49   ` Marcos Paulo de Souza
2021-08-11 13:03     ` Marcos Paulo de Souza

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.