Skip to contentSkip to conversation

Btrfs: fix infinite loop in btrfs_shrink_device()

PATCH In review Ilya Dryomov 2 participants 3 messages Feb 25, 2011 – Feb 26, 2011 Btrfs file system

lore
Share & tools
lore

Ilya Dryomov

author
#0 lore raw

PATCH 1/1 Btrfs: fix infinite loop in btrfs_shrink_device()

In case of an ENOSPC error from btrfs_relocate_chunk() (line 2202) while relocating a block group with offset 0 we end up endlessly looping. This happens because key.offset -= 1 statement then unconditionally brings us back to the beginnig of the loop (key.offset == (u64)-1).

Signed-off-by Ilya Dryomov <[email protected]>
---
 fs/btrfs/volumes.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
fs/btrfs/volumes.c +2 -1
index dd13eb8..0cb94ce 100644
@@ -2212,7 +2212,8 @@ again:
 			goto done;
 		if (ret == -ENOSPC)
 			failed++;
-		key.offset -= 1;
+		if (--key.offset == -1)
+			break;
 	}
 
 	if (failed && !retried) {
Signature · 1 line
1.7.2.3
#1 lore raw

On Sat, Feb 26, 2011 at 7:43 AM, Ilya Dryomov <[email protected]> wrot= e:

Quoted 1 line

In case of an ENOSPC error from btrfs_relocate_chunk() (line 2202) wh=

ile

Quoted 3 lines

relocating a block group with offset 0 we end up endlessly looping. This happens because key.offset -=3D 1 statement then unconditionally brings us back to the beginnig of the loop (key.offset =3D=3D (u64)-1=

).

Quoted 11 lines
Signed-off-by Ilya Dryomov <[email protected]>

---
=A0fs/btrfs/volumes.c | =A0 =A03 ++-
=A01 files changed, 2 insertions(+), 1 deletions(-)

fs/btrfs/volumes.c +0 -0
index dd13eb8..0cb94ce 100644

fs/btrfs/volumes.c again: · line 2217

Quoted 7 lines
fs/btrfs/volumes.c +2 -1
@@ -2212,7 +2212,8 @@ again:
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto done;
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (ret =3D=3D -ENOSPC)
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0failed++;
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 key.offset -=3D 1;
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (--key.offset =3D=3D -1)
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
 =A0 =A0 =A0 =A0}

it should be if (--key.offset =3D=3D (u64) -1)

Quoted 7 lines
fs/btrfs/volumes.c +0 -1
 =A0 =A0 =A0 =A0if (failed && !retried) {
--

1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs=

" in

Quoted 3 lines

the body of a message to [email protected]
More majordomo info at =A0http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#2 lore raw
Quoted 1 line · Zheng

On Sat, Feb 26, 2011 at 7:43 AM, Ilya Dryomov <[email protected]> wr=

ote:

Quoted 1 line
Quoted 1 line

In case of an ENOSPC error from btrfs_relocate_chunk() (line 2202) =

while

Quoted 2 lines
Quoted 2 lines

relocating a block group with offset 0 we end up endlessly looping. This happens because key.offset -=3D 1 statement then unconditional=

ly

Quoted 1 line
Quoted 1 line

brings us back to the beginnig of the loop (key.offset =3D=3D (u64)=

-1).

Quoted 6 lines
Quoted 6 lines
Signed-off-by Ilya Dryomov <[email protected]>

---
=C2=A0fs/btrfs/volumes.c | =C2=A0 =C2=A03 ++- =C2=A01 files changed, 2 insertions(+), 1 deletions(-)

Quoted 6 lines
Quoted 6 lines
fs/btrfs/volumes.c +0 -0
index dd13eb8..0cb94ce 100644
@@ -2212,7 +2212,8 @@ again:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 =C2=A0goto done;

Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret =3D=3D=

-ENOSPC)

Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 =C2=A0failed++;

Quoted 1 line
Quoted 1 line

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 key.offset -=3D =

1;

Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (--key.offset=

=3D=3D -1)

Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 break;

Quoted 11 lines
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0}

=20
it should be if (--key.offset =3D=3D (u64) -1)
=20

Quoted 7 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0if (failed && !retried) { --
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-btr=

fs" in

Quoted 2 lines
Quoted 2 lines

the body of a message to [email protected]
More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.=

html

Quoted 1 line
Quoted 1 line

I wonder how I managed to screw that up, I got it right in the commit description.. Sorry for the noise, here is an updated version.

---

=46rom: Ilya Dryomov <[email protected]>
Date: Sat, 26 Feb 2011 01:29:25 +0200
Subject: [PATCH] Btrfs: fix infinite loop in btrfs_shrink_device()

In case of an ENOSPC error from btrfs_relocate_chunk() (line 2202) whil= e
relocating a block group with offset 0 we end up endlessly looping. This happens because key.offset -=3D 1 statement then unconditionally brings us back to the beginning of the loop (key.offset =3D=3D (u64)-1)= =2E

Signed-off-by Ilya Dryomov <[email protected]>
---
 fs/btrfs/volumes.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
fs/btrfs/volumes.c +2 -1
index dd13eb8..cd334ae 100644
@@ -2212,7 +2212,8 @@ again:
 			goto done;
 		if (ret =3D=3D -ENOSPC)
 			failed++;
-		key.offset -=3D 1;
+		if (--key.offset =3D=3D (u64)-1)
+			break;
 	}

=20
if (failed && !retried) {
--=20
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html