Re: [PATCH] Btrfs-progs: fix infinite loop in find_free_extent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





At 06/27/2017 02:02 AM, Liu Bo wrote:
On Mon, Jun 26, 2017 at 04:09:53PM +0200, David Sterba wrote:
On Fri, Jun 23, 2017 at 10:28:31PM -0600, Liu Bo wrote:
From: Liu Bo <boliu@localhost.localdomain>

Ah, my From was broken again.


%search_start is calculated in a wrong way, and if %ins is a cross-stripe
  one, it'll search the same block group forever.

That's a bit terse description, so please check if my understanding is right:
search_start advances by at least one stripe len, but the math would be wrong
as using bg_offset would not move us to the next stripe. bg_cache->key.objectid
is the full length so this will reach the next stripe and will not loop forever.

Yes, it's correct, the code's logic is like, now that the returned %ins is a
cross-stripe one, it then calculates a BTRFS_STRIPE_LEN aligned one as the new
%search_start and see if there is any free block matching %search_start.  The
current code is using a wrong offset, the offset really should be the start
position of a block group.


Do you happen to have a test for that?

Unfortunately it's not a test with vanilla progs.

I found this when mkfs.btrfs with a 12K nodesize, but now kernel has a
power_of_2 limitation for nodesize and progs code is using a weird IS_ALIGNED()

Yes, btrfs_check_nodesize() is using (nodesize & (sectorsize - 1)) to check if it's aligned, but it's only correct if sectorsize is power of 2.

It should also be fixed for btrfs-progs.

Thanks,
Qu

which has the same effect with power_of_2(), mkfs.btrfs -n 12K is not allowed.
I changed IS_ALIGNED() to (blocksize % nodesize != 0) and got the above loop.


Signed-off-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
---
  extent-tree.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/extent-tree.c b/extent-tree.c
index b12ee29..5e09274 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -2614,8 +2614,9 @@ check_failed:
  				goto no_bg_cache;
  			bg_offset = ins->objectid - bg_cache->key.objectid;
- search_start = round_up(bg_offset + num_bytes,
-						BTRFS_STRIPE_LEN) + bg_offset;
+			search_start = round_up(
+				bg_offset + num_bytes, BTRFS_STRIPE_LEN) +
+				bg_cache->key.object;

extent-tree.c: In function ‘find_free_extent’:
extent-tree.c:2617:18: error: ‘struct btrfs_key’ has no member named ‘object’; did you mean ‘objectid’?
      bg_cache->key.object;
                   ^

Ouch, that's right, it's %objectid.

I'll send a updated one, thanks for the comments.

-liubo
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux