diff mbox

[1/2] Btrfs-progs: make convert to allocate space from the desired type of block group

Message ID 1464312410-10999-1-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Bo May 27, 2016, 1:26 a.m. UTC
During btrfs-convert, it can allocate space from METADATA block
group for data, which is not supposed to be correct, although it
doesn't cause any serious problem except eating METADATA space
 more quickly.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 btrfs-convert.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Qu Wenruo May 27, 2016, 1:38 a.m. UTC | #1
Hi Liu,

Since btrfs-convert has been reworked to use a completely new method to 
do such allocation, now it doesn't need any custom_alloc_extent() function.

And its new chunk lay out is designed to only put metadata chunk into 
large enough unused space, and data chunks will cover all used space.

So there is no need for such patch AFAIK.

And the new convert is already in devel branch.

Thanks,
Qu

Liu Bo wrote on 2016/05/26 18:26 -0700:
> During btrfs-convert, it can allocate space from METADATA block
> group for data, which is not supposed to be correct, although it
> doesn't cause any serious problem except eating METADATA space
>  more quickly.
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
>  btrfs-convert.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/btrfs-convert.c b/btrfs-convert.c
> index b49775c..550aa8f 100644
> --- a/btrfs-convert.c
> +++ b/btrfs-convert.c
> @@ -364,6 +364,12 @@ static int custom_alloc_extent(struct btrfs_root *root, u64 num_bytes,
>  			continue;
>  		}
>
> +		if ((!!metadata) !=
> +		    (!!(cache->flags & BTRFS_BLOCK_GROUP_METADATA))) {
> +			last = cache->key.objectid + cache->key.offset;
> +			continue;
> +		}
> +
>  		if (metadata) {
>  			BUG_ON(num_bytes != root->nodesize);
>  			if (check_crossing_stripes(start, num_bytes)) {
>


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba May 27, 2016, 10:55 a.m. UTC | #2
On Fri, May 27, 2016 at 09:38:09AM +0800, Qu Wenruo wrote:
> Since btrfs-convert has been reworked to use a completely new method to 
> do such allocation, now it doesn't need any custom_alloc_extent() function.
> 
> And its new chunk lay out is designed to only put metadata chunk into 
> large enough unused space, and data chunks will cover all used space.
> 
> So there is no need for such patch AFAIK.

I agree, and I think that the motivation for the convert rewrite was the
same problem that these patches address.

> And the new convert is already in devel branch.

The plan is to release 4.6 with this patchset, I'm still not decided
about the low-mem mode for checker.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Qu Wenruo May 27, 2016, 11:04 a.m. UTC | #3
On 05/27/2016 06:55 PM, David Sterba wrote:
> On Fri, May 27, 2016 at 09:38:09AM +0800, Qu Wenruo wrote:
>> Since btrfs-convert has been reworked to use a completely new method to
>> do such allocation, now it doesn't need any custom_alloc_extent() function.
>>
>> And its new chunk lay out is designed to only put metadata chunk into
>> large enough unused space, and data chunks will cover all used space.
>>
>> So there is no need for such patch AFAIK.
>
> I agree, and I think that the motivation for the convert rewrite was the
> same problem that these patches address.
>
>> And the new convert is already in devel branch.
>
> The plan is to release 4.6 with this patchset, I'm still not decided
> about the low-mem mode for checker.

For low memory mode, at least we're not that eager to see it merged into 
4.6.

Although if it could be merged, it would make us free from rebasing it 
again and again.

But the problem is, for end-user, we are cheating them, as the low 
memory mode is not really that low memory.
Since fs tree is still eating tons of memory.

Thanks,
Qu

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba May 27, 2016, 11:15 a.m. UTC | #4
On Fri, May 27, 2016 at 07:04:42PM +0800, Qu Wenruo wrote:
> > The plan is to release 4.6 with this patchset, I'm still not decided
> > about the low-mem mode for checker.
> 
> For low memory mode, at least we're not that eager to see it merged into 
> 4.6.

I still need to test it on some large filesystems.

> Although if it could be merged, it would make us free from rebasing it 
> again and again.

As it's in devel now, it's on me to rebase and refresh it. It's going to
stay here, a release branch is usually a set of cherry-picked patches
from devel.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/btrfs-convert.c b/btrfs-convert.c
index b49775c..550aa8f 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -364,6 +364,12 @@  static int custom_alloc_extent(struct btrfs_root *root, u64 num_bytes,
 			continue;
 		}
 
+		if ((!!metadata) !=
+		    (!!(cache->flags & BTRFS_BLOCK_GROUP_METADATA))) {
+			last = cache->key.objectid + cache->key.offset;
+			continue;
+		}
+
 		if (metadata) {
 			BUG_ON(num_bytes != root->nodesize);
 			if (check_crossing_stripes(start, num_bytes)) {