All of lore.kernel.org
 help / color / mirror / Atom feed
* Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
@ 2019-05-29 11:23 David Sterba
  2019-05-29 11:33 ` Greg Kroah-Hartman
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: David Sterba @ 2019-05-29 11:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: stable

Hi,

upon closer inspection we found a problem with the patch

"btrfs: Honour FITRIM range constraints during free space trim"

that has been merged to 5.1.4. This could happen with ranged FITRIM
where the range is not 'honoured' as it was supposed to.

Please revert it and push in the next stable release so the buggy
version is not in the wild for too long.

Affected trees:

5.0.18
5.1.4
4.9.179
4.19.45
4.14.122

Master branch will have the revert too. Thanks.

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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-05-29 11:23 Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees David Sterba
@ 2019-05-29 11:33 ` Greg Kroah-Hartman
  2019-05-29 11:57   ` David Sterba
  2019-05-29 16:57   ` David Sterba
  2019-05-29 17:25 ` [PATCH for 4.14.x] Revert "btrfs: Honour FITRIM range constraints during free space trim" David Sterba
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-05-29 11:33 UTC (permalink / raw)
  To: David Sterba; +Cc: stable

On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> Hi,
> 
> upon closer inspection we found a problem with the patch
> 
> "btrfs: Honour FITRIM range constraints during free space trim"
> 
> that has been merged to 5.1.4. This could happen with ranged FITRIM
> where the range is not 'honoured' as it was supposed to.
> 
> Please revert it and push in the next stable release so the buggy
> version is not in the wild for too long.
> 
> Affected trees:
> 
> 5.0.18
> 5.1.4
> 4.9.179
> 4.19.45
> 4.14.122
> 
> Master branch will have the revert too. Thanks.

What is the git commit id of the revert in Linus's tree?

thanks,

greg k-h

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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-05-29 11:33 ` Greg Kroah-Hartman
@ 2019-05-29 11:57   ` David Sterba
  2019-06-04  8:19     ` Greg Kroah-Hartman
  2019-05-29 16:57   ` David Sterba
  1 sibling, 1 reply; 14+ messages in thread
From: David Sterba @ 2019-05-29 11:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: stable

On Wed, May 29, 2019 at 04:33:00AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> > Hi,
> > 
> > upon closer inspection we found a problem with the patch
> > 
> > "btrfs: Honour FITRIM range constraints during free space trim"
> > 
> > that has been merged to 5.1.4. This could happen with ranged FITRIM
> > where the range is not 'honoured' as it was supposed to.
> > 
> > Please revert it and push in the next stable release so the buggy
> > version is not in the wild for too long.
> > 
> > Affected trees:
> > 
> > 5.0.18
> > 5.1.4
> > 4.9.179
> > 4.19.45
> > 4.14.122
> > 
> > Master branch will have the revert too. Thanks.
> 
> What is the git commit id of the revert in Linus's tree?

The commit is not there yet, I'm going to send it with the next update
in a few days for 5.2-rc2.

To shorthen the delay I hope it's possible to revert the patches without
the corresponding master commit but if you insist on that I'll send the
pull request today and will let you know the commit id.

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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-05-29 11:33 ` Greg Kroah-Hartman
  2019-05-29 11:57   ` David Sterba
@ 2019-05-29 16:57   ` David Sterba
  2019-05-29 18:36     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 14+ messages in thread
From: David Sterba @ 2019-05-29 16:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: stable

On Wed, May 29, 2019 at 04:33:00AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> > Hi,
> > 
> > upon closer inspection we found a problem with the patch
> > 
> > "btrfs: Honour FITRIM range constraints during free space trim"
> > 
> > that has been merged to 5.1.4. This could happen with ranged FITRIM
> > where the range is not 'honoured' as it was supposed to.
> > 
> > Please revert it and push in the next stable release so the buggy
> > version is not in the wild for too long.
> > 
> > Affected trees:
> > 
> > 5.0.18
> > 5.1.4
> > 4.9.179
> > 4.19.45
> > 4.14.122
> > 
> > Master branch will have the revert too. Thanks.
> 
> What is the git commit id of the revert in Linus's tree?

Due to further changes in the code, a revert that will be in Linus'
branch can't be backported and stable-specific patches would have to be
provided anyway. There's a slight change in logic and handling of the
trimmed ranges, the upstream revert removes code and updates calls to
functions that are not in the stable branches.

So I'm going to send you patches for all the affected branches.

After analyzing the situation, the conclusion is that the patch should
have never been tagged for stable.  The patch is in 5.2-rc ie. an
unreleased kernel and the bug would be handled as a regression before
5.20 final.

The backport to stable happened before we knew that so the reverts are
IMO the best solution we have now. I hope you understand that and sorry
for the trouble.

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

* [PATCH for 4.14.x] Revert "btrfs: Honour FITRIM range constraints during free space trim"
  2019-05-29 11:23 Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees David Sterba
  2019-05-29 11:33 ` Greg Kroah-Hartman
@ 2019-05-29 17:25 ` David Sterba
  2019-05-29 17:25 ` [PATCH for 4.19.x] " David Sterba
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2019-05-29 17:25 UTC (permalink / raw)
  To: stable; +Cc: David Sterba

This reverts commit b327ff8a9b5767ce39db650d468fb124b48974a5.

There is currently no corresponding patch in master due to additional
changes that would be significantly different from plain revert in the
respective stable branch.

The range argument was not handled correctly and could cause trim to
overlap allocated areas or reach beyond the end of the device. The
address space that fitrim normally operates on is in logical
coordinates, while the discards are done on the physical device extents.
This distinction cannot be made with the current ioctl interface and
caused the confusion.

The bug depends on the layout of block groups and does not always
happen. The whole-fs trim (run by default by the fstrim tool) is not
affected.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 54bb5d79723a..83791d13c204 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -11058,9 +11058,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
  * transaction.
  */
 static int btrfs_trim_free_extents(struct btrfs_device *device,
-				   struct fstrim_range *range, u64 *trimmed)
+				   u64 minlen, u64 *trimmed)
 {
-	u64 start = range->start, len = 0;
+	u64 start = 0, len = 0;
 	int ret;
 
 	*trimmed = 0;
@@ -11096,8 +11096,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 			refcount_inc(&trans->use_count);
 		spin_unlock(&fs_info->trans_lock);
 
-		ret = find_free_dev_extent_start(trans, device, range->minlen,
-						 start, &start, &len);
+		ret = find_free_dev_extent_start(trans, device, minlen, start,
+						 &start, &len);
 		if (trans)
 			btrfs_put_transaction(trans);
 
@@ -11109,16 +11109,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 			break;
 		}
 
-		/* If we are out of the passed range break */
-		if (start > range->start + range->len - 1) {
-			mutex_unlock(&fs_info->chunk_mutex);
-			ret = 0;
-			break;
-		}
-
-		start = max(range->start, start);
-		len = min(range->len, len);
-
 		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
 		up_read(&fs_info->commit_root_sem);
 		mutex_unlock(&fs_info->chunk_mutex);
@@ -11129,10 +11119,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		start += len;
 		*trimmed += bytes;
 
-		/* We've trimmed enough */
-		if (*trimmed >= range->len)
-			break;
-
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;
 			break;
@@ -11216,7 +11202,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	devices = &fs_info->fs_devices->devices;
 	list_for_each_entry(device, devices, dev_list) {
-		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
+		ret = btrfs_trim_free_extents(device, range->minlen,
+					      &group_trimmed);
 		if (ret) {
 			dev_failed++;
 			dev_ret = ret;
-- 
2.21.0


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

* [PATCH for 4.19.x] Revert "btrfs: Honour FITRIM range constraints during free space trim"
  2019-05-29 11:23 Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees David Sterba
  2019-05-29 11:33 ` Greg Kroah-Hartman
  2019-05-29 17:25 ` [PATCH for 4.14.x] Revert "btrfs: Honour FITRIM range constraints during free space trim" David Sterba
@ 2019-05-29 17:25 ` David Sterba
  2019-05-29 17:25 ` [PATCH for 4.9.x] " David Sterba
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2019-05-29 17:25 UTC (permalink / raw)
  To: stable; +Cc: David Sterba

This reverts commit 8b13bb911f0c0c77d41e5ddc41ad3c127c356b8a.

There is currently no corresponding patch in master due to additional
changes that would be significantly different from plain revert in the
respective stable branch.

The range argument was not handled correctly and could cause trim to
overlap allocated areas or reach beyond the end of the device. The
address space that fitrim normally operates on is in logical
coordinates, while the discards are done on the physical device extents.
This distinction cannot be made with the current ioctl interface and
caused the confusion.

The bug depends on the layout of block groups and does not always
happen. The whole-fs trim (run by default by the fstrim tool) is not
affected.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 809c2c307c64..c0db7785cede 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -10789,9 +10789,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
  * held back allocations.
  */
 static int btrfs_trim_free_extents(struct btrfs_device *device,
-				   struct fstrim_range *range, u64 *trimmed)
+				   u64 minlen, u64 *trimmed)
 {
-	u64 start = range->start, len = 0;
+	u64 start = 0, len = 0;
 	int ret;
 
 	*trimmed = 0;
@@ -10834,8 +10834,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		if (!trans)
 			up_read(&fs_info->commit_root_sem);
 
-		ret = find_free_dev_extent_start(trans, device, range->minlen,
-						 start, &start, &len);
+		ret = find_free_dev_extent_start(trans, device, minlen, start,
+						 &start, &len);
 		if (trans) {
 			up_read(&fs_info->commit_root_sem);
 			btrfs_put_transaction(trans);
@@ -10848,16 +10848,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 			break;
 		}
 
-		/* If we are out of the passed range break */
-		if (start > range->start + range->len - 1) {
-			mutex_unlock(&fs_info->chunk_mutex);
-			ret = 0;
-			break;
-		}
-
-		start = max(range->start, start);
-		len = min(range->len, len);
-
 		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
 		mutex_unlock(&fs_info->chunk_mutex);
 
@@ -10867,10 +10857,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		start += len;
 		*trimmed += bytes;
 
-		/* We've trimmed enough */
-		if (*trimmed >= range->len)
-			break;
-
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;
 			break;
@@ -10954,7 +10940,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	devices = &fs_info->fs_devices->devices;
 	list_for_each_entry(device, devices, dev_list) {
-		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
+		ret = btrfs_trim_free_extents(device, range->minlen,
+					      &group_trimmed);
 		if (ret) {
 			dev_failed++;
 			dev_ret = ret;
-- 
2.21.0


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

* [PATCH for 4.9.x] Revert "btrfs: Honour FITRIM range constraints during free space trim"
  2019-05-29 11:23 Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees David Sterba
                   ` (2 preceding siblings ...)
  2019-05-29 17:25 ` [PATCH for 4.19.x] " David Sterba
@ 2019-05-29 17:25 ` David Sterba
  2019-05-29 17:25 ` [PATCH for 5.0.x] " David Sterba
  2019-05-29 17:25 ` [PATCH for 5.1.x] " David Sterba
  5 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2019-05-29 17:25 UTC (permalink / raw)
  To: stable; +Cc: David Sterba

This reverts commit 038ec2c13e0d1f7b9d45a081786f18f75b65f11b.

There is currently no corresponding patch in master due to additional
changes that would be significantly different from plain revert in the
respective stable branch.

The range argument was not handled correctly and could cause trim to
overlap allocated areas or reach beyond the end of the device. The
address space that fitrim normally operates on is in logical
coordinates, while the discards are done on the physical device extents.
This distinction cannot be made with the current ioctl interface and
caused the confusion.

The bug depends on the layout of block groups and does not always
happen. The whole-fs trim (run by default by the fstrim tool) is not
affected.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 6b29165f766f..7938c48c72ff 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -11150,9 +11150,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_root *root, u64 start, u64 end)
  * transaction.
  */
 static int btrfs_trim_free_extents(struct btrfs_device *device,
-				   struct fstrim_range *range, u64 *trimmed)
+				   u64 minlen, u64 *trimmed)
 {
-	u64 start = range->start, len = 0;
+	u64 start = 0, len = 0;
 	int ret;
 
 	*trimmed = 0;
@@ -11188,8 +11188,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 			atomic_inc(&trans->use_count);
 		spin_unlock(&fs_info->trans_lock);
 
-		ret = find_free_dev_extent_start(trans, device, range->minlen,
-						 start, &start, &len);
+		ret = find_free_dev_extent_start(trans, device, minlen, start,
+						 &start, &len);
 		if (trans)
 			btrfs_put_transaction(trans);
 
@@ -11201,16 +11201,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 			break;
 		}
 
-		/* If we are out of the passed range break */
-		if (start > range->start + range->len - 1) {
-			mutex_unlock(&fs_info->chunk_mutex);
-			ret = 0;
-			break;
-		}
-
-		start = max(range->start, start);
-		len = min(range->len, len);
-
 		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
 		up_read(&fs_info->commit_root_sem);
 		mutex_unlock(&fs_info->chunk_mutex);
@@ -11221,10 +11211,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		start += len;
 		*trimmed += bytes;
 
-		/* We've trimmed enough */
-		if (*trimmed >= range->len)
-			break;
-
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;
 			break;
@@ -11309,7 +11295,8 @@ int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range)
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	devices = &fs_info->fs_devices->devices;
 	list_for_each_entry(device, devices, dev_list) {
-		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
+		ret = btrfs_trim_free_extents(device, range->minlen,
+					      &group_trimmed);
 		if (ret) {
 			dev_failed++;
 			dev_ret = ret;
-- 
2.21.0


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

* [PATCH for 5.0.x] Revert "btrfs: Honour FITRIM range constraints during free space trim"
  2019-05-29 11:23 Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees David Sterba
                   ` (3 preceding siblings ...)
  2019-05-29 17:25 ` [PATCH for 4.9.x] " David Sterba
@ 2019-05-29 17:25 ` David Sterba
  2019-05-29 17:25 ` [PATCH for 5.1.x] " David Sterba
  5 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2019-05-29 17:25 UTC (permalink / raw)
  To: stable; +Cc: David Sterba

This reverts commit b9ee627187491547791aacf96d4dd8f4d9afbf1c.

There is currently no corresponding patch in master due to additional
changes that would be significantly different from plain revert in the
respective stable branch.

The range argument was not handled correctly and could cause trim to
overlap allocated areas or reach beyond the end of the device. The
address space that fitrim normally operates on is in logical
coordinates, while the discards are done on the physical device extents.
This distinction cannot be made with the current ioctl interface and
caused the confusion.

The bug depends on the layout of block groups and does not always
happen. The whole-fs trim (run by default by the fstrim tool) is not
affected.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a19bbfce449e..1b68700bc1c5 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -11192,9 +11192,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
  * held back allocations.
  */
 static int btrfs_trim_free_extents(struct btrfs_device *device,
-				   struct fstrim_range *range, u64 *trimmed)
+				   u64 minlen, u64 *trimmed)
 {
-	u64 start = range->start, len = 0;
+	u64 start = 0, len = 0;
 	int ret;
 
 	*trimmed = 0;
@@ -11237,8 +11237,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		if (!trans)
 			up_read(&fs_info->commit_root_sem);
 
-		ret = find_free_dev_extent_start(trans, device, range->minlen,
-						 start, &start, &len);
+		ret = find_free_dev_extent_start(trans, device, minlen, start,
+						 &start, &len);
 		if (trans) {
 			up_read(&fs_info->commit_root_sem);
 			btrfs_put_transaction(trans);
@@ -11251,16 +11251,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 			break;
 		}
 
-		/* If we are out of the passed range break */
-		if (start > range->start + range->len - 1) {
-			mutex_unlock(&fs_info->chunk_mutex);
-			ret = 0;
-			break;
-		}
-
-		start = max(range->start, start);
-		len = min(range->len, len);
-
 		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
 		mutex_unlock(&fs_info->chunk_mutex);
 
@@ -11270,10 +11260,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		start += len;
 		*trimmed += bytes;
 
-		/* We've trimmed enough */
-		if (*trimmed >= range->len)
-			break;
-
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;
 			break;
@@ -11357,7 +11343,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	devices = &fs_info->fs_devices->devices;
 	list_for_each_entry(device, devices, dev_list) {
-		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
+		ret = btrfs_trim_free_extents(device, range->minlen,
+					      &group_trimmed);
 		if (ret) {
 			dev_failed++;
 			dev_ret = ret;
-- 
2.21.0


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

* [PATCH for 5.1.x] Revert "btrfs: Honour FITRIM range constraints during free space trim"
  2019-05-29 11:23 Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees David Sterba
                   ` (4 preceding siblings ...)
  2019-05-29 17:25 ` [PATCH for 5.0.x] " David Sterba
@ 2019-05-29 17:25 ` David Sterba
  5 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2019-05-29 17:25 UTC (permalink / raw)
  To: stable; +Cc: David Sterba

This reverts commit eb432217d775a90c061681c0dfa3c7abfba75123.

There is currently no corresponding patch in master due to additional
changes that would be significantly different from plain revert in the
respective stable branch.

The range argument was not handled correctly and could cause trim to
overlap allocated areas or reach beyond the end of the device. The
address space that fitrim normally operates on is in logical
coordinates, while the discards are done on the physical device extents.
This distinction cannot be made with the current ioctl interface and
caused the confusion.

The bug depends on the layout of block groups and does not always
happen. The whole-fs trim (run by default by the fstrim tool) is not
affected.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index d789542edc5a..c5880329ae37 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -11315,9 +11315,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
  * held back allocations.
  */
 static int btrfs_trim_free_extents(struct btrfs_device *device,
-				   struct fstrim_range *range, u64 *trimmed)
+				   u64 minlen, u64 *trimmed)
 {
-	u64 start = range->start, len = 0;
+	u64 start = 0, len = 0;
 	int ret;
 
 	*trimmed = 0;
@@ -11360,8 +11360,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		if (!trans)
 			up_read(&fs_info->commit_root_sem);
 
-		ret = find_free_dev_extent_start(trans, device, range->minlen,
-						 start, &start, &len);
+		ret = find_free_dev_extent_start(trans, device, minlen, start,
+						 &start, &len);
 		if (trans) {
 			up_read(&fs_info->commit_root_sem);
 			btrfs_put_transaction(trans);
@@ -11374,16 +11374,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 			break;
 		}
 
-		/* If we are out of the passed range break */
-		if (start > range->start + range->len - 1) {
-			mutex_unlock(&fs_info->chunk_mutex);
-			ret = 0;
-			break;
-		}
-
-		start = max(range->start, start);
-		len = min(range->len, len);
-
 		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
 		mutex_unlock(&fs_info->chunk_mutex);
 
@@ -11393,10 +11383,6 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
 		start += len;
 		*trimmed += bytes;
 
-		/* We've trimmed enough */
-		if (*trimmed >= range->len)
-			break;
-
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;
 			break;
@@ -11480,7 +11466,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	devices = &fs_info->fs_devices->devices;
 	list_for_each_entry(device, devices, dev_list) {
-		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
+		ret = btrfs_trim_free_extents(device, range->minlen,
+					      &group_trimmed);
 		if (ret) {
 			dev_failed++;
 			dev_ret = ret;
-- 
2.21.0


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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-05-29 16:57   ` David Sterba
@ 2019-05-29 18:36     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-05-29 18:36 UTC (permalink / raw)
  To: David Sterba; +Cc: stable

On Wed, May 29, 2019 at 06:57:43PM +0200, David Sterba wrote:
> On Wed, May 29, 2019 at 04:33:00AM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> > > Hi,
> > > 
> > > upon closer inspection we found a problem with the patch
> > > 
> > > "btrfs: Honour FITRIM range constraints during free space trim"
> > > 
> > > that has been merged to 5.1.4. This could happen with ranged FITRIM
> > > where the range is not 'honoured' as it was supposed to.
> > > 
> > > Please revert it and push in the next stable release so the buggy
> > > version is not in the wild for too long.
> > > 
> > > Affected trees:
> > > 
> > > 5.0.18
> > > 5.1.4
> > > 4.9.179
> > > 4.19.45
> > > 4.14.122
> > > 
> > > Master branch will have the revert too. Thanks.
> > 
> > What is the git commit id of the revert in Linus's tree?
> 
> Due to further changes in the code, a revert that will be in Linus'
> branch can't be backported and stable-specific patches would have to be
> provided anyway. There's a slight change in logic and handling of the
> trimmed ranges, the upstream revert removes code and updates calls to
> functions that are not in the stable branches.
> 
> So I'm going to send you patches for all the affected branches.
> 
> After analyzing the situation, the conclusion is that the patch should
> have never been tagged for stable.  The patch is in 5.2-rc ie. an
> unreleased kernel and the bug would be handled as a regression before
> 5.20 final.
> 
> The backport to stable happened before we knew that so the reverts are
> IMO the best solution we have now. I hope you understand that and sorry
> for the trouble.

No problem, reverts are all now queued up, thanks for doing them.

greg k-h

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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-05-29 11:57   ` David Sterba
@ 2019-06-04  8:19     ` Greg Kroah-Hartman
  2019-06-04 11:03       ` David Sterba
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-04  8:19 UTC (permalink / raw)
  To: David Sterba; +Cc: stable

On Wed, May 29, 2019 at 01:57:52PM +0200, David Sterba wrote:
> On Wed, May 29, 2019 at 04:33:00AM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> > > Hi,
> > > 
> > > upon closer inspection we found a problem with the patch
> > > 
> > > "btrfs: Honour FITRIM range constraints during free space trim"
> > > 
> > > that has been merged to 5.1.4. This could happen with ranged FITRIM
> > > where the range is not 'honoured' as it was supposed to.
> > > 
> > > Please revert it and push in the next stable release so the buggy
> > > version is not in the wild for too long.
> > > 
> > > Affected trees:
> > > 
> > > 5.0.18
> > > 5.1.4
> > > 4.9.179
> > > 4.19.45
> > > 4.14.122
> > > 
> > > Master branch will have the revert too. Thanks.
> > 
> > What is the git commit id of the revert in Linus's tree?
> 
> The commit is not there yet, I'm going to send it with the next update
> in a few days for 5.2-rc2.
> 
> To shorthen the delay I hope it's possible to revert the patches without
> the corresponding master commit but if you insist on that I'll send the
> pull request today and will let you know the commit id.

Did this ever get reverted in Linus's tree?  I can't seem to find it...

thanks,

greg k-h

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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-06-04  8:19     ` Greg Kroah-Hartman
@ 2019-06-04 11:03       ` David Sterba
  2019-06-04 11:34         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2019-06-04 11:03 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: stable

On Tue, Jun 04, 2019 at 10:19:20AM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 29, 2019 at 01:57:52PM +0200, David Sterba wrote:
> > On Wed, May 29, 2019 at 04:33:00AM -0700, Greg Kroah-Hartman wrote:
> > > On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> > > > Hi,
> > > > 
> > > > upon closer inspection we found a problem with the patch
> > > > 
> > > > "btrfs: Honour FITRIM range constraints during free space trim"
> > > > 
> > > > that has been merged to 5.1.4. This could happen with ranged FITRIM
> > > > where the range is not 'honoured' as it was supposed to.
> > > > 
> > > > Please revert it and push in the next stable release so the buggy
> > > > version is not in the wild for too long.
> > > > 
> > > > Affected trees:
> > > > 
> > > > 5.0.18
> > > > 5.1.4
> > > > 4.9.179
> > > > 4.19.45
> > > > 4.14.122
> > > > 
> > > > Master branch will have the revert too. Thanks.
> > > 
> > > What is the git commit id of the revert in Linus's tree?
> > 
> > The commit is not there yet, I'm going to send it with the next update
> > in a few days for 5.2-rc2.
> > 
> > To shorthen the delay I hope it's possible to revert the patches without
> > the corresponding master commit but if you insist on that I'll send the
> > pull request today and will let you know the commit id.
> 
> Did this ever get reverted in Linus's tree?  I can't seem to find it...

The patches 2 and 4 from this patchset

https://patchwork.kernel.org/project/linux-btrfs/list/?series=126297

will implement the equivalent change. Scheduled for merge to 5.2-rc4,
the patches were not available last week.

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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-06-04 11:03       ` David Sterba
@ 2019-06-04 11:34         ` Greg Kroah-Hartman
  2019-06-12 11:06           ` David Sterba
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-04 11:34 UTC (permalink / raw)
  To: David Sterba; +Cc: stable

On Tue, Jun 04, 2019 at 01:03:55PM +0200, David Sterba wrote:
> On Tue, Jun 04, 2019 at 10:19:20AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, May 29, 2019 at 01:57:52PM +0200, David Sterba wrote:
> > > On Wed, May 29, 2019 at 04:33:00AM -0700, Greg Kroah-Hartman wrote:
> > > > On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> > > > > Hi,
> > > > > 
> > > > > upon closer inspection we found a problem with the patch
> > > > > 
> > > > > "btrfs: Honour FITRIM range constraints during free space trim"
> > > > > 
> > > > > that has been merged to 5.1.4. This could happen with ranged FITRIM
> > > > > where the range is not 'honoured' as it was supposed to.
> > > > > 
> > > > > Please revert it and push in the next stable release so the buggy
> > > > > version is not in the wild for too long.
> > > > > 
> > > > > Affected trees:
> > > > > 
> > > > > 5.0.18
> > > > > 5.1.4
> > > > > 4.9.179
> > > > > 4.19.45
> > > > > 4.14.122
> > > > > 
> > > > > Master branch will have the revert too. Thanks.
> > > > 
> > > > What is the git commit id of the revert in Linus's tree?
> > > 
> > > The commit is not there yet, I'm going to send it with the next update
> > > in a few days for 5.2-rc2.
> > > 
> > > To shorthen the delay I hope it's possible to revert the patches without
> > > the corresponding master commit but if you insist on that I'll send the
> > > pull request today and will let you know the commit id.
> > 
> > Did this ever get reverted in Linus's tree?  I can't seem to find it...
> 
> The patches 2 and 4 from this patchset
> 
> https://patchwork.kernel.org/project/linux-btrfs/list/?series=126297
> 
> will implement the equivalent change. Scheduled for merge to 5.2-rc4,
> the patches were not available last week.

You might want to mark those for stable, if this is a big deal :)

thanks,

greg k-h

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

* Re: Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees
  2019-06-04 11:34         ` Greg Kroah-Hartman
@ 2019-06-12 11:06           ` David Sterba
  0 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2019-06-12 11:06 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: stable

On Tue, Jun 04, 2019 at 01:34:11PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 01:03:55PM +0200, David Sterba wrote:
> > On Tue, Jun 04, 2019 at 10:19:20AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, May 29, 2019 at 01:57:52PM +0200, David Sterba wrote:
> > > > On Wed, May 29, 2019 at 04:33:00AM -0700, Greg Kroah-Hartman wrote:
> > > > > On Wed, May 29, 2019 at 01:23:14PM +0200, David Sterba wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > upon closer inspection we found a problem with the patch
> > > > > > 
> > > > > > "btrfs: Honour FITRIM range constraints during free space trim"
> > > > > > 
> > > > > > that has been merged to 5.1.4. This could happen with ranged FITRIM
> > > > > > where the range is not 'honoured' as it was supposed to.
> > > > > > 
> > > > > > Please revert it and push in the next stable release so the buggy
> > > > > > version is not in the wild for too long.
> > > > > > 
> > > > > > Affected trees:
> > > > > > 
> > > > > > 5.0.18
> > > > > > 5.1.4
> > > > > > 4.9.179
> > > > > > 4.19.45
> > > > > > 4.14.122
> > > > > > 
> > > > > > Master branch will have the revert too. Thanks.
> > > > > 
> > > > > What is the git commit id of the revert in Linus's tree?
> > > > 
> > > > The commit is not there yet, I'm going to send it with the next update
> > > > in a few days for 5.2-rc2.
> > > > 
> > > > To shorthen the delay I hope it's possible to revert the patches without
> > > > the corresponding master commit but if you insist on that I'll send the
> > > > pull request today and will let you know the commit id.
> > > 
> > > Did this ever get reverted in Linus's tree?  I can't seem to find it...
> > 
> > The patches 2 and 4 from this patchset
> > 
> > https://patchwork.kernel.org/project/linux-btrfs/list/?series=126297
> > 
> > will implement the equivalent change. Scheduled for merge to 5.2-rc4,
> > the patches were not available last week.
> 
> You might want to mark those for stable, if this is a big deal :)

For the record, the mentioned upstream commit has been merged. Marking
it for stable does not make sense as the stable trees already have the
reverts.

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

end of thread, other threads:[~2019-06-12 11:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 11:23 Please revert "btrfs: Honour FITRIM range constraints during free space trim" from all stable trees David Sterba
2019-05-29 11:33 ` Greg Kroah-Hartman
2019-05-29 11:57   ` David Sterba
2019-06-04  8:19     ` Greg Kroah-Hartman
2019-06-04 11:03       ` David Sterba
2019-06-04 11:34         ` Greg Kroah-Hartman
2019-06-12 11:06           ` David Sterba
2019-05-29 16:57   ` David Sterba
2019-05-29 18:36     ` Greg Kroah-Hartman
2019-05-29 17:25 ` [PATCH for 4.14.x] Revert "btrfs: Honour FITRIM range constraints during free space trim" David Sterba
2019-05-29 17:25 ` [PATCH for 4.19.x] " David Sterba
2019-05-29 17:25 ` [PATCH for 4.9.x] " David Sterba
2019-05-29 17:25 ` [PATCH for 5.0.x] " David Sterba
2019-05-29 17:25 ` [PATCH for 5.1.x] " David Sterba

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.