linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Btrfs: add support for mixed data+metadata block groups V3
@ 2010-10-07 11:51 Josef Bacik
  2010-10-21  4:21 ` Mitch Harder
  0 siblings, 1 reply; 9+ messages in thread
From: Josef Bacik @ 2010-10-07 11:51 UTC (permalink / raw)
  To: linux-btrfs

There are just a few things that need to be fixed in the kernel to support mixed
data+metadata block groups.  Mostly we just need to make sure that if we are
using mixed block groups that we continue to allocate mixed block groups as we
need them.  Also we need to make sure __find_space_info will find our space info
if we search for DATA or METADATA only.  Tested this with xfstests and it works
nicely.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
V2->V3: Add btrfs_mixed_space_info helper

V1->V2: In do_chunk_alloc I was changing flags to == space_info->flags, which
isn't right since space_info doesn't carry the RAID profiles anymore, so instead
check to see if the space info has DATA and METADATA set and if so set that in
the flags as well.
 fs/btrfs/ctree.h       |    6 ++++++
 fs/btrfs/extent-tree.c |   22 +++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1ecd8f6..86a4f4b 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2043,6 +2043,12 @@ static inline struct dentry *fdentry(struct file *file)
 	return file->f_path.dentry;
 }
 
+static inline bool btrfs_mixed_space_info(struct btrfs_space_info *space_info)
+{
+	return ((space_info->flags & BTRFS_BLOCK_GROUP_METADATA) &&
+		(space_info->flags & BTRFS_BLOCK_GROUP_DATA));
+}
+
 /* extent-tree.c */
 void btrfs_put_block_group(struct btrfs_block_group_cache *cache);
 int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 91a0a41..ed84271 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -547,7 +547,7 @@ static struct btrfs_space_info *__find_space_info(struct btrfs_fs_info *info,
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(found, head, list) {
-		if (found->flags == flags) {
+		if (found->flags & flags) {
 			rcu_read_unlock();
 			return found;
 		}
@@ -3267,6 +3267,13 @@ static int do_chunk_alloc(struct btrfs_trans_handle *trans,
 	spin_unlock(&space_info->lock);
 
 	/*
+	 * If we have mixed data/metadata chunks we want to make sure we keep
+	 * allocating mixed chunks instead of individual chunks.
+	 */
+	if (btrfs_mixed_space_info(space_info))
+		flags |= (BTRFS_BLOCK_GROUP_DATA | BTRFS_BLOCK_GROUP_METADATA);
+
+	/*
 	 * if we're doing a data chunk, go ahead and make sure that
 	 * we keep a reasonable number of metadata chunks allocated in the
 	 * FS as well.
@@ -4793,6 +4800,7 @@ static noinline int find_free_extent(struct btrfs_trans_handle *trans,
 	bool found_uncached_bg = false;
 	bool failed_cluster_refill = false;
 	bool failed_alloc = false;
+	bool use_cluster = true;
 	u64 ideal_cache_percent = 0;
 	u64 ideal_cache_offset = 0;
 
@@ -4807,16 +4815,24 @@ static noinline int find_free_extent(struct btrfs_trans_handle *trans,
 		return -ENOSPC;
 	}
 
+	/*
+	 * If the space info is for both data and metadata it means we have a
+	 * small filesystem and we can't use the clustering stuff.
+	 */
+	if (btrfs_mixed_space_info(space_info))
+		use_cluster = false;
+
 	if (orig_root->ref_cows || empty_size)
 		allowed_chunk_alloc = 1;
 
-	if (data & BTRFS_BLOCK_GROUP_METADATA) {
+	if (data & BTRFS_BLOCK_GROUP_METADATA && use_cluster) {
 		last_ptr = &root->fs_info->meta_alloc_cluster;
 		if (!btrfs_test_opt(root, SSD))
 			empty_cluster = 64 * 1024;
 	}
 
-	if ((data & BTRFS_BLOCK_GROUP_DATA) && btrfs_test_opt(root, SSD)) {
+	if ((data & BTRFS_BLOCK_GROUP_DATA) && use_cluster &&
+	    btrfs_test_opt(root, SSD)) {
 		last_ptr = &root->fs_info->data_alloc_cluster;
 	}
 
-- 
1.6.6.1


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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-07 11:51 [PATCH] Btrfs: add support for mixed data+metadata block groups V3 Josef Bacik
@ 2010-10-21  4:21 ` Mitch Harder
  2010-10-21 15:46   ` David Nicol
  0 siblings, 1 reply; 9+ messages in thread
From: Mitch Harder @ 2010-10-21  4:21 UTC (permalink / raw)
  To: linux-btrfs

I've been testing this patch (as well as the accompanying patch to btrfs-progs).

It seems to save a decent amount of space (maybe 10-20% according to
df in my testing, YMMV), but I was also noticing a performance penalty
of maybe 5-15%, depending on the application (in my case, I was timing
the untar-ing of data to a btrfs partition).

I also developed the perception that I was encountering some minor
latency issues when heavily using a drive formated for mixed data and
metadata (more keyboard and mouse hesitations than normal, difficult
stuff to quantify).

The main problem I encountered was a btrfs crash when booting with an
un-patched kernel, and letting the boot process attempt to mount the
drive with mixed data+metadata.  I wasn't entirely surprised by this
result, but it was unclear from the patch description whether a volume
formatted with mixed data+metadata would be incompatible with an
un-patched, older kernel.

Other than that, it seemed to perform well.  I did not encounter any
stability issues as long as I was using a patched kernel.

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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-21  4:21 ` Mitch Harder
@ 2010-10-21 15:46   ` David Nicol
  2010-10-21 22:09     ` Diego Calleja
  0 siblings, 1 reply; 9+ messages in thread
From: David Nicol @ 2010-10-21 15:46 UTC (permalink / raw)
  Cc: linux-btrfs

Does this mixing constitute a forbidden change of on-disk format, and
if not how not?


On Wed, Oct 20, 2010 at 11:21 PM, Mitch Harder
<mitch.harder@sabayonlinux.org> wrote:
> Other than that, it seemed to perform well. =C2=A0I did not encounter=
 any
> stability issues as long as I was using a patched kernel.
--
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

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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-21 15:46   ` David Nicol
@ 2010-10-21 22:09     ` Diego Calleja
  2010-10-21 22:21       ` Mitch Harder
  0 siblings, 1 reply; 9+ messages in thread
From: Diego Calleja @ 2010-10-21 22:09 UTC (permalink / raw)
  To: David Nicol, linux-btrfs

On Jueves, 21 de Octubre de 2010 17:46:58 David Nicol escribi=C3=B3:
> Does this mixing constitute a forbidden change of on-disk format, and
> if not how not?

It doesn't need a format change. The difference between a data and
a metadata block group is just an allocation hint AFAIK.
--
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

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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-21 22:09     ` Diego Calleja
@ 2010-10-21 22:21       ` Mitch Harder
  2010-10-22  1:05         ` Josef Bacik
  0 siblings, 1 reply; 9+ messages in thread
From: Mitch Harder @ 2010-10-21 22:21 UTC (permalink / raw)
  To: linux-btrfs

On Thu, Oct 21, 2010 at 5:09 PM, Diego Calleja <diegocg@gmail.com> wrot=
e:
> On Jueves, 21 de Octubre de 2010 17:46:58 David Nicol escribi=F3:
>> Does this mixing constitute a forbidden change of on-disk format, an=
d
>> if not how not?
>
> It doesn't need a format change. The difference between a data and
> a metadata block group is just an allocation hint AFAIK.
> --
> 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 =A0http://vger.kernel.org/majordomo-info.html
>

Let me know if the problems with an un-patched kernel were un-expected.

I can provide more information on the crash when booting an older kerne=
l.
--
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

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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-21 22:21       ` Mitch Harder
@ 2010-10-22  1:05         ` Josef Bacik
  2010-10-22  1:32           ` C Anthony Risinger
  0 siblings, 1 reply; 9+ messages in thread
From: Josef Bacik @ 2010-10-22  1:05 UTC (permalink / raw)
  To: Mitch Harder; +Cc: linux-btrfs

On Thu, Oct 21, 2010 at 05:21:06PM -0500, Mitch Harder wrote:
> On Thu, Oct 21, 2010 at 5:09 PM, Diego Calleja <diegocg@gmail.com> wr=
ote:
> > On Jueves, 21 de Octubre de 2010 17:46:58 David Nicol escribi=F3:
> >> Does this mixing constitute a forbidden change of on-disk format, =
and
> >> if not how not?
> >
> > It doesn't need a format change. The difference between a data and
> > a metadata block group is just an allocation hint AFAIK.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btr=
fs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at =A0http://vger.kernel.org/majordomo-info.htm=
l
> >
>=20
> Let me know if the problems with an un-patched kernel were un-expecte=
d.
>=20
> I can provide more information on the crash when booting an older ker=
nel.

Nope they are expected, it's not a disk format change, but older kernel=
s won't
deal with mixed block groups.  Thanks,

Josef
--
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

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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-22  1:05         ` Josef Bacik
@ 2010-10-22  1:32           ` C Anthony Risinger
  2010-10-22  1:37             ` Josef Bacik
  0 siblings, 1 reply; 9+ messages in thread
From: C Anthony Risinger @ 2010-10-22  1:32 UTC (permalink / raw)
  To: Josef Bacik; +Cc: Mitch Harder, linux-btrfs

On Thu, Oct 21, 2010 at 8:05 PM, Josef Bacik <josef@redhat.com> wrote:
> On Thu, Oct 21, 2010 at 05:21:06PM -0500, Mitch Harder wrote:
>> On Thu, Oct 21, 2010 at 5:09 PM, Diego Calleja <diegocg@gmail.com> w=
rote:
>> > On Jueves, 21 de Octubre de 2010 17:46:58 David Nicol escribi=F3:
>> >> Does this mixing constitute a forbidden change of on-disk format,=
 and
>> >> if not how not?
>> >
>> > It doesn't need a format change. The difference between a data and
>> > a metadata block group is just an allocation hint AFAIK.
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-bt=
rfs" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at =A0http://vger.kernel.org/majordomo-info.ht=
ml
>> >
>>
>> Let me know if the problems with an un-patched kernel were un-expect=
ed.
>>
>> I can provide more information on the crash when booting an older ke=
rnel.
>
> Nope they are expected, it's not a disk format change, but older kern=
els won't
> deal with mixed block groups.

When something like this goes mainline, is it used by default/automatic=
ally?

I ask because I maintain a btrfs-based rollback initramfs hook [1],
and am currently updating it for extlinux, enabling kernel-level
system rollbacks via `btrfs set-default` + reboot (or maybe
`kexec`)...

rolling back to an old kernel will then blow up my machine
(figuratively of course :-)?

C Anthony

[1] http://aur.archlinux.org/packages.php?ID=3D33376
--
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

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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-22  1:32           ` C Anthony Risinger
@ 2010-10-22  1:37             ` Josef Bacik
  2010-10-22  2:06               ` C Anthony Risinger
  0 siblings, 1 reply; 9+ messages in thread
From: Josef Bacik @ 2010-10-22  1:37 UTC (permalink / raw)
  To: C Anthony Risinger; +Cc: Josef Bacik, Mitch Harder, linux-btrfs

On Thu, Oct 21, 2010 at 08:32:12PM -0500, C Anthony Risinger wrote:
> On Thu, Oct 21, 2010 at 8:05 PM, Josef Bacik <josef@redhat.com> wrote=
:
> > On Thu, Oct 21, 2010 at 05:21:06PM -0500, Mitch Harder wrote:
> >> On Thu, Oct 21, 2010 at 5:09 PM, Diego Calleja <diegocg@gmail.com>=
 wrote:
> >> > On Jueves, 21 de Octubre de 2010 17:46:58 David Nicol escribi=F3=
:
> >> >> Does this mixing constitute a forbidden change of on-disk forma=
t, and
> >> >> if not how not?
> >> >
> >> > It doesn't need a format change. The difference between a data a=
nd
> >> > a metadata block group is just an allocation hint AFAIK.
> >> > --
> >> > 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 =A0http://vger.kernel.org/majordomo-info.=
html
> >> >
> >>
> >> Let me know if the problems with an un-patched kernel were un-expe=
cted.
> >>
> >> I can provide more information on the crash when booting an older =
kernel.
> >
> > Nope they are expected, it's not a disk format change, but older ke=
rnels won't
> > deal with mixed block groups.
>=20
> When something like this goes mainline, is it used by default/automat=
ically?
>=20
> I ask because I maintain a btrfs-based rollback initramfs hook [1],
> and am currently updating it for extlinux, enabling kernel-level
> system rollbacks via `btrfs set-default` + reboot (or maybe
> `kexec`)...
>=20
> rolling back to an old kernel will then blow up my machine
> (figuratively of course :-)?
>

The only way you get this feature is if you mkfs with the feature enabl=
ed, and
is only meant for small filesystems (1 gig or smaller).  Thanks,

Josef
--
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

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

* Re: [PATCH] Btrfs: add support for mixed data+metadata block groups V3
  2010-10-22  1:37             ` Josef Bacik
@ 2010-10-22  2:06               ` C Anthony Risinger
  0 siblings, 0 replies; 9+ messages in thread
From: C Anthony Risinger @ 2010-10-22  2:06 UTC (permalink / raw)
  To: Josef Bacik; +Cc: Mitch Harder, linux-btrfs

On Thu, Oct 21, 2010 at 8:37 PM, Josef Bacik <josef@redhat.com> wrote:
> On Thu, Oct 21, 2010 at 08:32:12PM -0500, C Anthony Risinger wrote:
>> On Thu, Oct 21, 2010 at 8:05 PM, Josef Bacik <josef@redhat.com> wrot=
e:
>> > On Thu, Oct 21, 2010 at 05:21:06PM -0500, Mitch Harder wrote:
>> >> On Thu, Oct 21, 2010 at 5:09 PM, Diego Calleja <diegocg@gmail.com=
> wrote:
>> >> > On Jueves, 21 de Octubre de 2010 17:46:58 David Nicol escribi=F3=
:
>> >> >> Does this mixing constitute a forbidden change of on-disk form=
at, and
>> >> >> if not how not?
>> >> >
>> >> > It doesn't need a format change. The difference between a data =
and
>> >> > a metadata block group is just an allocation hint AFAIK.
>> >> > --
>> >> > 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 =A0http://vger.kernel.org/majordomo-info=
=2Ehtml
>> >> >
>> >>
>> >> Let me know if the problems with an un-patched kernel were un-exp=
ected.
>> >>
>> >> I can provide more information on the crash when booting an older=
 kernel.
>> >
>> > Nope they are expected, it's not a disk format change, but older k=
ernels won't
>> > deal with mixed block groups.
>>
>> When something like this goes mainline, is it used by default/automa=
tically?
>>
>> I ask because I maintain a btrfs-based rollback initramfs hook [1],
>> and am currently updating it for extlinux, enabling kernel-level
>> system rollbacks via `btrfs set-default` + reboot (or maybe
>> `kexec`)...
>>
>> rolling back to an old kernel will then blow up my machine
>> (figuratively of course :-)?
>>
>
> The only way you get this feature is if you mkfs with the feature ena=
bled, and
> is only meant for small filesystems (1 gig or smaller).

Ah right :-), thanks

C Anthony
--
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

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

end of thread, other threads:[~2010-10-22  2:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-07 11:51 [PATCH] Btrfs: add support for mixed data+metadata block groups V3 Josef Bacik
2010-10-21  4:21 ` Mitch Harder
2010-10-21 15:46   ` David Nicol
2010-10-21 22:09     ` Diego Calleja
2010-10-21 22:21       ` Mitch Harder
2010-10-22  1:05         ` Josef Bacik
2010-10-22  1:32           ` C Anthony Risinger
2010-10-22  1:37             ` Josef Bacik
2010-10-22  2:06               ` C Anthony Risinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).