Re: [PATCH] Btrfs: disable snapshot aware defrag for now

2014-02-04 Thread Josef Bacik


On 02/03/2014 10:19 PM, Roger Binns wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 03/02/14 09:27, Josef Bacik wrote:

It is so totally broken that I don't want it being turned on by anybody
who can't edit this and change it themselves.

The symptoms I saw are huge amounts of kernel memory consumption, possibly
till exhaustion of swap.  Are there other ways in which is it broken (eg
corruption)?

Also is this patch making its way to the various stables?
No corruption, and frankly if you had just small files and a small 
number of snapshots it worked just fine, but otherwise it would use all 
of your ram.  I cc'ed stable@ so it should make it back to all the 
stables.  Thanks,


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


Re: [PATCH] Btrfs: disable snapshot aware defrag for now

2014-02-03 Thread David Sterba
On Wed, Jan 29, 2014 at 04:05:30PM -0500, Josef Bacik wrote:
 It's just broken and it's taking a lot of effort to fix it, so for now just
 disable it so people can defrag in peace.  Thanks,
 
 Cc: sta...@vger.kernel.org
 Signed-off-by: Josef Bacik jba...@fb.com
 ---
  fs/btrfs/inode.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
 index 3b65987..8c0bc31 100644
 --- a/fs/btrfs/inode.c
 +++ b/fs/btrfs/inode.c
 @@ -2628,7 +2628,7 @@ static int btrfs_finish_ordered_io(struct 
 btrfs_ordered_extent *ordered_extent)
   EXTENT_DEFRAG, 1, cached_state);
   if (ret) {
   u64 last_snapshot = btrfs_root_last_snapshot(root-root_item);
 - if (last_snapshot = BTRFS_I(inode)-generation)
 + if (0  last_snapshot = BTRFS_I(inode)-generation)

That's not very flexible, how are we supposed to test that in the
meantime? Editing sources is not the peferred way.

I was thinking about adding a config option that would cover any
experimental/broken features, this one be the first, as we currently
have no other way to disable it. I'd rather avoid adding a temporary
mount option.
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: disable snapshot aware defrag for now

2014-02-03 Thread Josef Bacik


On 02/03/2014 09:48 AM, David Sterba wrote:

On Wed, Jan 29, 2014 at 04:05:30PM -0500, Josef Bacik wrote:

It's just broken and it's taking a lot of effort to fix it, so for now just
disable it so people can defrag in peace.  Thanks,

Cc: sta...@vger.kernel.org
Signed-off-by: Josef Bacik jba...@fb.com
---
  fs/btrfs/inode.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3b65987..8c0bc31 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2628,7 +2628,7 @@ static int btrfs_finish_ordered_io(struct 
btrfs_ordered_extent *ordered_extent)
EXTENT_DEFRAG, 1, cached_state);
if (ret) {
u64 last_snapshot = btrfs_root_last_snapshot(root-root_item);
-   if (last_snapshot = BTRFS_I(inode)-generation)
+   if (0  last_snapshot = BTRFS_I(inode)-generation)

That's not very flexible, how are we supposed to test that in the
meantime? Editing sources is not the peferred way.


Well since I'm the only one currently working on fixing it I'm not 
worried about it.  If anybody else wants to fix it they can easily 
change it themselves.  It is so totally broken that I don't want it 
being turned on by anybody who can't edit this and change it 
themselves.  Thanks,


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


Re: [PATCH] Btrfs: disable snapshot aware defrag for now

2014-02-03 Thread Roger Binns
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 03/02/14 09:27, Josef Bacik wrote:
 It is so totally broken that I don't want it being turned on by anybody
 who can't edit this and change it themselves.

The symptoms I saw are huge amounts of kernel memory consumption, possibly
till exhaustion of swap.  Are there other ways in which is it broken (eg
corruption)?

Also is this patch making its way to the various stables?

Roger

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.14 (GNU/Linux)

iEYEARECAAYFAlLwXE8ACgkQmOOfHg372QRLngCgpc445lPvM7YhGUxVdlU2O4vN
1CUAoM2NmeGPOeYxOji4yL4VRysBnTxg
=sQ3M
-END PGP SIGNATURE-

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


[PATCH] Btrfs: disable snapshot aware defrag for now

2014-01-29 Thread Josef Bacik
It's just broken and it's taking a lot of effort to fix it, so for now just
disable it so people can defrag in peace.  Thanks,

Cc: sta...@vger.kernel.org
Signed-off-by: Josef Bacik jba...@fb.com
---
 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3b65987..8c0bc31 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2628,7 +2628,7 @@ static int btrfs_finish_ordered_io(struct 
btrfs_ordered_extent *ordered_extent)
EXTENT_DEFRAG, 1, cached_state);
if (ret) {
u64 last_snapshot = btrfs_root_last_snapshot(root-root_item);
-   if (last_snapshot = BTRFS_I(inode)-generation)
+   if (0  last_snapshot = BTRFS_I(inode)-generation)
/* the inode is shared */
new = record_old_file_extents(inode, ordered_extent);
 
-- 
1.8.3.1

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