Re: [PATCH] btrfs: use list_for_each_entry_safe() when delete items

2013-07-29 Thread Azat Khuzhin
On Sat, Jul 27, 2013 at 2:12 PM, Azat Khuzhin  wrote:
> Replace list_for_each_entry() by list_for_each_entry_safe() in
> __btrfs_close_devices()
>
> There is another place that delete items lock_stripe_add(), but there we
> don't need safe version, because after deleting we exit from loop.
>
> Signed-off-by: Azat Khuzhin 
> ---
>  fs/btrfs/volumes.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 78b8717..1d1b595 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -616,13 +616,13 @@ static void free_device(struct rcu_head *head)
>
>  static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>  {
> -   struct btrfs_device *device;
> +   struct btrfs_device *device, *next;
>
> if (--fs_devices->opened > 0)
> return 0;
>
> mutex_lock(&fs_devices->device_list_mutex);
> -   list_for_each_entry(device, &fs_devices->devices, dev_list) {
> +   list_for_each_entry_safe(device, next, &fs_devices->devices, 
> dev_list) {
> struct btrfs_device *new_device;
> struct rcu_string *name;

There is "kfree(device);" at the end of loop, maybe there must "goto
again;" after it?
(instead of this patch)

>
> --
> 1.7.10.4
>



-- 
Respectfully
Azat Khuzhin
--
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: btrfs quota examples?

2013-07-29 Thread Tomasz Chmielewski
On Mon, 10 Jun 2013 09:41:39 +0200
Arne Jansen  wrote:

> > Now, my questions:
> > 
> > - what do both 104882176 104882176 numbers represent?
> 
> The first number represents the amount of data in that subvolume,
> regardless whether that data is shared with other subvolumes or
> not.
> The second number shows the amount of data that is unique to this
> subvolume and not shared with others, i.e. the amount of space
> that will get freed if you delete this subvolume.

I've played with qgroups for some time, but the results are rather
inconsistent.

I.e. here - what does a negative number represent in 0/1181 row?

# btrfs qgroup show /mnt/lxc2
0/260 151490953216 151490953216
0/261 180969472 180969472
0/262 17888 983040
0/377 180310016 25776128
0/378 304088072192 304088072192
0/535 571944960 417370112
0/536 68550987776 68550987776
0/642 247463936 92921856
0/1175 617213952 827392
0/1181 16112013312 -22184235008
0/1268 38296248320 0
0/1269 616386560 0
0/1270 4096 4096
0/1271 4096 4096


-- 
Tomasz Chmielewski
http://wpkg.org
--
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: Fwd: Cloning a Btrfs partition

2013-07-29 Thread Jan Schmidt
Hi BJ,

[original message rewrapped]

On Thu, July 25, 2013 at 18:32 (+0200), BJ Quinn wrote:
> (Apologies for the double post -- forgot to send as plain text the first time
> around, so the list rejected it.)
> 
> I see that there's now a btrfs send / receive and I've tried using it, but
> I'm getting the oops I've pasted below, after which the FS becomes
> unresponsive (no I/O to the drive, no CPU usage, but all attempts to access
> the FS results in a hang). I have an internal drive (single drive) that
> contains 82GB of compressed data with a couple hundred snapshots. I tried
> taking the first snapshot and making a read only copy (btrfs subvolume
> snapshot -r) and then I connected an external USB drive and ran btrfs send /
> receive to that external drive. It starts working and gets a couple of GB in
> (I'd expect the first snapshot to be about 20GB) and then gets the following
> error. I had to use the latest copy of btrfs-progs from git, because the
> package installed on my system (btrfs-progs-0.20-0.2.git91d9eec) simply
> returned "invalid argument" when trying to run btrfs send / receive. Thanks
> in advance for any info you may have.

The problem has been introduced with rbtree ulists in 3.10, commit

Btrfs: add a rb_tree to improve performance of ulist search

You should be safe to revert that commit, it's a performance optimization
attempt. Alternatively, you can apply the published fix

Btrfs: fix crash regarding to ulist_add_merge

It has not made it into 3.10 stable or 3.11, yet, but is contained in Josef's
btrfs-next

git://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git

Thanks,
-Jan
--
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 v3] Btrfs: fix crash regarding to ulist_add_merge

2013-07-29 Thread Jan Schmidt
On Fri, June 28, 2013 at 06:37 (+0200), Liu Bo wrote:
> Several users reported this crash of NULL pointer or general protection,
> the story is that we add a rbtree for speedup ulist iteration, and we
> use krealloc() to address ulist growth, and krealloc() use memcpy to copy
> old data to new memory area, so it's OK for an array as it doesn't use
> pointers while it's not OK for a rbtree as it uses pointers.
> 
> So krealloc() will mess up our rbtree and it ends up with crash.
> 
> Reviewed-by: Wang Shilong 
> Signed-off-by: Liu Bo 
> ---
> v3: fix a return value problem(Thanks Wang Shilong).
> v2: fix an use-after-free bug and a finger error(Thanks Zach and Josef).
> 
>  fs/btrfs/ulist.c |   15 +++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c
> index 7b417e2..b0a523b2 100644
> --- a/fs/btrfs/ulist.c
> +++ b/fs/btrfs/ulist.c
> @@ -205,6 +205,10 @@ int ulist_add_merge(struct ulist *ulist, u64 val, u64 
> aux,
>   u64 new_alloced = ulist->nodes_alloced + 128;
>   struct ulist_node *new_nodes;
>   void *old = NULL;
> + int i;
> +
> + for (i = 0; i < ulist->nnodes; i++)
> + rb_erase(&ulist->nodes[i].rb_node, &ulist->root);
>  
>   /*
>* if nodes_alloced == ULIST_SIZE no memory has been allocated
> @@ -224,6 +228,17 @@ int ulist_add_merge(struct ulist *ulist, u64 val, u64 
> aux,
>  
>   ulist->nodes = new_nodes;
>   ulist->nodes_alloced = new_alloced;
> +
> + /*
> +  * krealloc actually uses memcpy, which does not copy rb_node
> +  * pointers, so we have to do it ourselves.  Otherwise we may
> +  * be bitten by crashes.
> +  */
> + for (i = 0; i < ulist->nnodes; i++) {
> + ret = ulist_rbtree_insert(ulist, &ulist->nodes[i]);
> + if (ret < 0)
> + return ret;
> + }
>   }
>   ulist->nodes[ulist->nnodes].val = val;
>   ulist->nodes[ulist->nnodes].aux = aux;
> 

Reviewed-by: Jan Schmidt 

Josef, how about sending this one for the next 3.11 rc and to 3.10 stable? Any
objections?

-Jan
--
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: Recovering from btrfs error Couldn't read chunk root.

2013-07-29 Thread Wang Shilong

在 2013-7-29,上午2:12,Kyriakos  写道:

> Just tried it as you said with the -v option enabled
> This is my output:
> 
> http://bpaste.net/show/118112/
> 
> 
This is a *long* email, and seems that btrfs list refuse it.

>  Device extent: devid = 1, start = 1667558801408, len = 1073741824,
> chunk offset = 1663255445504
> Couldn't map the block 626309926912
> btrfs: volumes.c:1020: btrfs_num_copies: Assertion `!(ce->start >
> logical || ce->start + ce->size < logical)' failed.
> Aborted (core dumped)

Strange enough, we don't find any chunks during scanning process.

And seems this is unrecoverable ~_~


Wang,
> 
> 
> Any thoughts?
> 
> On 28 July 2013 08:17, Wang Shilong  wrote:
>> Hello,
>> 
>> It seems Btrfs Chunk Tree is damaged, so you can not mount Btrfs filesystem 
>> any more.
>> 
>> However, you can try the latest Btrfs-progs, Miao Xie implements chunk tree 
>> recover function.
>> 
>> The url is:
>> 
>>git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git
>> 
>> 
>> you can try it:
>>btrfs chunk-recover  -v 
>> 
>> This is Time-consuming, because it will scan the whole disk. And also,
>> please catch output of processing(this is helpful to us if the recovery 
>> fails, -v option
>> enable this).
>> 
>> Thanks,
>> Wang
>> 
>> 
>> 
>> 
>> 
>> 

--
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 v3] Btrfs: fix crash regarding to ulist_add_merge

2013-07-29 Thread Liu Bo
On Mon, Jul 29, 2013 at 10:23:34AM +0200, Jan Schmidt wrote:
> On Fri, June 28, 2013 at 06:37 (+0200), Liu Bo wrote:
> > Several users reported this crash of NULL pointer or general protection,
> > the story is that we add a rbtree for speedup ulist iteration, and we
> > use krealloc() to address ulist growth, and krealloc() use memcpy to copy
> > old data to new memory area, so it's OK for an array as it doesn't use
> > pointers while it's not OK for a rbtree as it uses pointers.
> > 
> > So krealloc() will mess up our rbtree and it ends up with crash.
> > 
> > Reviewed-by: Wang Shilong 
> > Signed-off-by: Liu Bo 
> > ---
> > v3: fix a return value problem(Thanks Wang Shilong).
> > v2: fix an use-after-free bug and a finger error(Thanks Zach and Josef).
> > 
> >  fs/btrfs/ulist.c |   15 +++
> >  1 files changed, 15 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c
> > index 7b417e2..b0a523b2 100644
> > --- a/fs/btrfs/ulist.c
> > +++ b/fs/btrfs/ulist.c
> > @@ -205,6 +205,10 @@ int ulist_add_merge(struct ulist *ulist, u64 val, u64 
> > aux,
> > u64 new_alloced = ulist->nodes_alloced + 128;
> > struct ulist_node *new_nodes;
> > void *old = NULL;
> > +   int i;
> > +
> > +   for (i = 0; i < ulist->nnodes; i++)
> > +   rb_erase(&ulist->nodes[i].rb_node, &ulist->root);
> >  
> > /*
> >  * if nodes_alloced == ULIST_SIZE no memory has been allocated
> > @@ -224,6 +228,17 @@ int ulist_add_merge(struct ulist *ulist, u64 val, u64 
> > aux,
> >  
> > ulist->nodes = new_nodes;
> > ulist->nodes_alloced = new_alloced;
> > +
> > +   /*
> > +* krealloc actually uses memcpy, which does not copy rb_node
> > +* pointers, so we have to do it ourselves.  Otherwise we may
> > +* be bitten by crashes.
> > +*/
> > +   for (i = 0; i < ulist->nnodes; i++) {
> > +   ret = ulist_rbtree_insert(ulist, &ulist->nodes[i]);
> > +   if (ret < 0)
> > +   return ret;
> > +   }
> > }
> > ulist->nodes[ulist->nnodes].val = val;
> > ulist->nodes[ulist->nnodes].aux = aux;
> > 
> 
> Reviewed-by: Jan Schmidt 
> 
> Josef, how about sending this one for the next 3.11 rc and to 3.10 stable? Any
> objections?

A good candidate for -stable, along with josef's 'last ref' fixes about
__tree_mod_log_rewind(), thanks,

-liubo
--
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: cleanup arguments to extent_clear_unlock_delalloc

2013-07-29 Thread Josef Bacik
This patch removes the io_tree argument for extent_clear_unlock_delalloc since
we always use &BTRFS_I(inode)->io_tree, and it separates out the extent tree
operations from the page operations.  This way we just pass in the extent bits
we want to clear and then pass in the operations we want done to the pages.
This is because I'm going to fix what extent bits we clear in some cases and
rather than add a bunch of new flags we'll just use the actual extent bits we
want to clear.  Thanks,

Signed-off-by: Josef Bacik 
---
 fs/btrfs/extent_io.c |   32 ---
 fs/btrfs/extent_io.h |   21 +++
 fs/btrfs/inode.c |  162 --
 3 files changed, 85 insertions(+), 130 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index cc685dd..deaea9c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1678,31 +1678,21 @@ out_failed:
return found;
 }
 
-int extent_clear_unlock_delalloc(struct inode *inode,
-   struct extent_io_tree *tree,
-   u64 start, u64 end, struct page *locked_page,
-   unsigned long op)
+int extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,
+struct page *locked_page,
+unsigned long clear_bits,
+unsigned long page_ops)
 {
+   struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
int ret;
struct page *pages[16];
unsigned long index = start >> PAGE_CACHE_SHIFT;
unsigned long end_index = end >> PAGE_CACHE_SHIFT;
unsigned long nr_pages = end_index - index + 1;
int i;
-   unsigned long clear_bits = 0;
-
-   if (op & EXTENT_CLEAR_UNLOCK)
-   clear_bits |= EXTENT_LOCKED;
-   if (op & EXTENT_CLEAR_DIRTY)
-   clear_bits |= EXTENT_DIRTY;
-
-   if (op & EXTENT_CLEAR_DELALLOC)
-   clear_bits |= EXTENT_DELALLOC;
 
clear_extent_bit(tree, start, end, clear_bits, 1, 0, NULL, GFP_NOFS);
-   if (!(op & (EXTENT_CLEAR_UNLOCK_PAGE | EXTENT_CLEAR_DIRTY |
-   EXTENT_SET_WRITEBACK | EXTENT_END_WRITEBACK |
-   EXTENT_SET_PRIVATE2)))
+   if (page_ops == 0)
return 0;
 
while (nr_pages > 0) {
@@ -1711,20 +1701,20 @@ int extent_clear_unlock_delalloc(struct inode *inode,
 nr_pages, ARRAY_SIZE(pages)), pages);
for (i = 0; i < ret; i++) {
 
-   if (op & EXTENT_SET_PRIVATE2)
+   if (page_ops & PAGE_SET_PRIVATE2)
SetPagePrivate2(pages[i]);
 
if (pages[i] == locked_page) {
page_cache_release(pages[i]);
continue;
}
-   if (op & EXTENT_CLEAR_DIRTY)
+   if (page_ops & PAGE_CLEAR_DIRTY)
clear_page_dirty_for_io(pages[i]);
-   if (op & EXTENT_SET_WRITEBACK)
+   if (page_ops & PAGE_SET_WRITEBACK)
set_page_writeback(pages[i]);
-   if (op & EXTENT_END_WRITEBACK)
+   if (page_ops & PAGE_END_WRITEBACK)
end_page_writeback(pages[i]);
-   if (op & EXTENT_CLEAR_UNLOCK_PAGE)
+   if (page_ops & PAGE_UNLOCK)
unlock_page(pages[i]);
page_cache_release(pages[i]);
}
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index f7544af..c450620 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -44,14 +44,11 @@
 #define EXTENT_BUFFER_DUMMY 9
 
 /* these are flags for extent_clear_unlock_delalloc */
-#define EXTENT_CLEAR_UNLOCK_PAGE 0x1
-#define EXTENT_CLEAR_UNLOCK 0x2
-#define EXTENT_CLEAR_DELALLOC   0x4
-#define EXTENT_CLEAR_DIRTY  0x8
-#define EXTENT_SET_WRITEBACK0x10
-#define EXTENT_END_WRITEBACK0x20
-#define EXTENT_SET_PRIVATE2 0x40
-#define EXTENT_CLEAR_ACCOUNTING  0x80
+#define PAGE_UNLOCK(1 << 0)
+#define PAGE_CLEAR_DIRTY   (1 << 1)
+#define PAGE_SET_WRITEBACK (1 << 2)
+#define PAGE_END_WRITEBACK (1 << 3)
+#define PAGE_SET_PRIVATE2  (1 << 4)
 
 /*
  * page->private values.  Every page that is controlled by the extent
@@ -328,10 +325,10 @@ int map_private_extent_buffer(struct extent_buffer *eb, 
unsigned long offset,
  unsigned long *map_len);
 int extent_range_clear_dirty_for_io(struct inode *inode, u64 start, u64 end);
 int extent_range_redirty_for_io(struct inode *inode, u64 start, u64 end);
-int extent_clear_unlock_delalloc(struct inode *inode,
-   struct extent_io_tree *tree,
-   u64 start, u64 end,

Re: Cloning a Btrfs partition

2013-07-29 Thread BJ Quinn
Thanks for the response!  Not sure I want to roll a custom kernel on this
particular system.  Any idea on when it might make it to 3.10 stable or 
3.11?  Or should I just revert back to 3.9?

Thanks!

-BJ

- Original Message - 

From: "Jan Schmidt"  
Sent: Monday, July 29, 2013 3:21:51 AM 

Hi BJ, 

[original message rewrapped] 

On Thu, July 25, 2013 at 18:32 (+0200), BJ Quinn wrote: 
> (Apologies for the double post -- forgot to send as plain text the first time 
> around, so the list rejected it.) 
> 
> I see that there's now a btrfs send / receive and I've tried using it, but 
> I'm getting the oops I've pasted below, after which the FS becomes 
> unresponsive (no I/O to the drive, no CPU usage, but all attempts to access 
> the FS results in a hang). I have an internal drive (single drive) that 
> contains 82GB of compressed data with a couple hundred snapshots. I tried 
> taking the first snapshot and making a read only copy (btrfs subvolume 
> snapshot -r) and then I connected an external USB drive and ran btrfs send / 
> receive to that external drive. It starts working and gets a couple of GB in 
> (I'd expect the first snapshot to be about 20GB) and then gets the following 
> error. I had to use the latest copy of btrfs-progs from git, because the 
> package installed on my system (btrfs-progs-0.20-0.2.git91d9eec) simply 
> returned "invalid argument" when trying to run btrfs send / receive. Thanks 
> in advance for any info you may have. 

The problem has been introduced with rbtree ulists in 3.10, commit 

Btrfs: add a rb_tree to improve performance of ulist search 

You should be safe to revert that commit, it's a performance optimization 
attempt. Alternatively, you can apply the published fix 

Btrfs: fix crash regarding to ulist_add_merge 

It has not made it into 3.10 stable or 3.11, yet, but is contained in Josef's 
btrfs-next 

git://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git 

Thanks, 
-Jan 
--
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


Online data deduplication

2013-07-29 Thread Hemanth Kumar
Hello,
I am willing to perform QA on online data deduplication. From where
can i download the patches?

-- 
Thanks,
Hemanth Kumar H C
--
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] xfstests: generic/315: add one more sync and more output

2013-07-29 Thread Josef Bacik
So df in btrfs is tricky at best, and relying on it for accurate information is
not great, but it's the best way to verify this test.  So we need to add another
sync to make sure the pinned blocks are all freed up and the df space is really
really accurate, otherwise we end up with this test failling because the df
after the test is slightly off (in my case it was like 36kb off).  With this
patch I'm not seeing random failures of this test.  Thanks,

Signed-off-by: Josef Bacik 
---
 tests/generic/315 |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tests/generic/315 b/tests/generic/315
index 7cfc40d..7c55b8a 100644
--- a/tests/generic/315
+++ b/tests/generic/315
@@ -70,10 +70,11 @@ fsize=`ls -l $TEST_DIR/testfile.$seq | awk '{print $5}'`
 # Truncate the file size back to 0
 truncate -s 0 $TEST_DIR/testfile.$seq
 sync
+sync
 
 # Preallocated disk space should be released
 avail_done=`df -P $TEST_DIR | awk 'END {print $4}'`
-[ "$avail_done" -eq "$avail_begin" ] || _fail "Available disk space 
($avail_done KiB)"
+[ "$avail_done" -eq "$avail_begin" ] || _fail "Available disk space 
($avail_done KiB) wanted ($avail_begin KiB)"
 
 # success, all done
 exit
-- 
1.7.7.6

--
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: fix what bits we clear when erroring out from delalloc

2013-07-29 Thread Josef Bacik
First of all we no longer set EXTENT_DIRTY when we dirty an extent so this patch
removes the clearing of EXTENT_DIRTY since it confuses me.  This patch also adds
clearing EXTENT_DEFRAG and also doing EXTENT_DO_ACCOUNTING when we have errors.
This is because if we are clearing delalloc without adding an ordered extent
then we need to make sure the enospc handling stuff is accounted for.  Also if
this range was DEFRAG we need to make sure that bit is cleared so we dont leak
it.  Thanks,

Signed-off-by: Josef Bacik 
---
 fs/btrfs/inode.c |   46 ++
 1 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a60be02..686ba5c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -490,8 +490,9 @@ cont:
 * Unlock and free up our temp pages.
 */
extent_clear_unlock_delalloc(inode, start, end, NULL,
-EXTENT_DIRTY |
-EXTENT_DELALLOC,
+EXTENT_DELALLOC |
+EXTENT_DO_ACCOUNTING |
+EXTENT_DEFRAG,
 PAGE_UNLOCK |
 PAGE_CLEAR_DIRTY |
 PAGE_SET_WRITEBACK |
@@ -593,9 +594,10 @@ free_pages_out:
 
 cleanup_and_out:
extent_clear_unlock_delalloc(inode, start, end, NULL,
-EXTENT_DIRTY | EXTENT_DELALLOC,
-PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
-PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
+EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING |
+EXTENT_DEFRAG, PAGE_UNLOCK |
+PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
+PAGE_END_WRITEBACK);
if (!trans || IS_ERR(trans))
btrfs_error(root->fs_info, ret, "Failed to join transaction");
else
@@ -770,8 +772,8 @@ retry:
extent_clear_unlock_delalloc(inode, async_extent->start,
async_extent->start +
async_extent->ram_size - 1,
-   NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
-   EXTENT_DIRTY, PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
+   NULL, EXTENT_LOCKED | EXTENT_DELALLOC,
+   PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
PAGE_SET_WRITEBACK);
ret = btrfs_submit_compressed_write(inode,
async_extent->start,
@@ -795,9 +797,9 @@ out_free:
 async_extent->start +
 async_extent->ram_size - 1,
 NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
-EXTENT_DIRTY, PAGE_UNLOCK |
-PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
-PAGE_END_WRITEBACK);
+EXTENT_DEFRAG | EXTENT_DO_ACCOUNTING,
+PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
+PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
kfree(async_extent);
goto again;
 }
@@ -884,7 +886,7 @@ static noinline int __cow_file_range(struct 
btrfs_trans_handle *trans,
if (ret == 0) {
extent_clear_unlock_delalloc(inode, start, end, NULL,
 EXTENT_LOCKED | EXTENT_DELALLOC |
-EXTENT_DIRTY, PAGE_UNLOCK |
+EXTENT_DEFRAG, PAGE_UNLOCK |
 PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
 PAGE_END_WRITEBACK);
 
@@ -995,10 +997,10 @@ out_reserve:
btrfs_free_reserved_extent(root, ins.objectid, ins.offset);
 out_unlock:
extent_clear_unlock_delalloc(inode, start, end, locked_page,
-EXTENT_LOCKED | EXTENT_DIRTY |
-EXTENT_DELALLOC, PAGE_UNLOCK |
-PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
-PAGE_END_WRITEBACK);
+EXTENT_LOCKED | EXTENT_DO_ACCOUNTING |
+EXTENT_DELALLOC | EXTENT_DEFRAG,
+PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
+PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
goto out;
 }
 
@@ -1016,7 +1018,8 @@ static 

Re: [PATCH] xfstests: generic/315: add one more sync and more output

2013-07-29 Thread Eric Sandeen
On 7/29/13 12:21 PM, Josef Bacik wrote:
> So df in btrfs is tricky at best, and relying on it for accurate information 
> is
> not great, but it's the best way to verify this test.  So we need to add 
> another
> sync to make sure the pinned blocks are all freed up and the df space is 
> really
> really accurate, otherwise we end up with this test failling because the df
> after the test is slightly off (in my case it was like 36kb off).  With this
> patch I'm not seeing random failures of this test.  Thanks,

Honest question: does one more sync make this deterministic, or is it a 
best-effort, um, hack?

-Eric

> Signed-off-by: Josef Bacik 
> ---
>  tests/generic/315 |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/generic/315 b/tests/generic/315
> index 7cfc40d..7c55b8a 100644
> --- a/tests/generic/315
> +++ b/tests/generic/315
> @@ -70,10 +70,11 @@ fsize=`ls -l $TEST_DIR/testfile.$seq | awk '{print $5}'`
>  # Truncate the file size back to 0
>  truncate -s 0 $TEST_DIR/testfile.$seq
>  sync
> +sync
>  
>  # Preallocated disk space should be released
>  avail_done=`df -P $TEST_DIR | awk 'END {print $4}'`
> -[ "$avail_done" -eq "$avail_begin" ] || _fail "Available disk space 
> ($avail_done KiB)"
> +[ "$avail_done" -eq "$avail_begin" ] || _fail "Available disk space 
> ($avail_done KiB) wanted ($avail_begin KiB)"
>  
>  # success, all done
>  exit
> 

--
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] xfstests: generic/315: add one more sync and more output

2013-07-29 Thread Eric Sandeen
On 7/29/13 12:31 PM, Eric Sandeen wrote:
> Honest question: does one more sync make this deterministic, or is it a 
> best-effort, um, hack?

I'm not quite sure why even 1 sync is needed.  :(

I'm not sure what bug this is trying to test; if you need 2 syncs for global 
space stats to accurately reflect the fact that you chopped off the end of a 
block, maybe that's ... still a bug?

Or if it's just the big-hammer question of "does the truncated space *ever* get 
freed?" then maybe umount/remount/check would tell you that more definitively.

-Eric
--
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 v2] Btrfs: optimize function btrfs_read_chunk_tree

2013-07-29 Thread Filipe David Borba Manana
After reading all device items from the chunk tree, don't
exit the loop and then navigate down the tree again to find
the chunk items. Instead just read all device items and
chunk items with a single tree search. This is possible
because all device items are found before any chunk item in
the chunks tree.

Signed-off-by: Filipe David Borba Manana 
---

V2: Simplified logic inside the loop
(suggested by Josef Bacik on irc).

 fs/btrfs/volumes.c |   30 ++
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 090f57c..45c5ec3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5676,14 +5676,13 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
mutex_lock(&uuid_mutex);
lock_chunks(root);
 
-   /* first we search for all of the device items, and then we
-* read in all of the chunk items.  This way we can create chunk
-* mappings that reference all of the devices that are afound
-*/
+   /* Read all device items, and then all the chunk items. All
+  device items are found before any chunk item (their object id
+  is smaller than the lowest possible object id for a chunk
+  item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). */
key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
key.offset = 0;
key.type = 0;
-again:
ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
if (ret < 0)
goto error;
@@ -5699,17 +5698,13 @@ again:
break;
}
btrfs_item_key_to_cpu(leaf, &found_key, slot);
-   if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
-   if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID)
-   break;
-   if (found_key.type == BTRFS_DEV_ITEM_KEY) {
-   struct btrfs_dev_item *dev_item;
-   dev_item = btrfs_item_ptr(leaf, slot,
+   if (found_key.type == BTRFS_DEV_ITEM_KEY) {
+   struct btrfs_dev_item *dev_item;
+   dev_item = btrfs_item_ptr(leaf, slot,
  struct btrfs_dev_item);
-   ret = read_one_dev(root, leaf, dev_item);
-   if (ret)
-   goto error;
-   }
+   ret = read_one_dev(root, leaf, dev_item);
+   if (ret)
+   goto error;
} else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) {
struct btrfs_chunk *chunk;
chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
@@ -5719,11 +5714,6 @@ again:
}
path->slots[0]++;
}
-   if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
-   key.objectid = 0;
-   btrfs_release_path(path);
-   goto again;
-   }
ret = 0;
 error:
unlock_chunks(root);
-- 
1.7.9.5

--
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] xfstests: generic/315: add one more sync and more output

2013-07-29 Thread Josef Bacik
On Mon, Jul 29, 2013 at 12:38:25PM -0500, Eric Sandeen wrote:
> On 7/29/13 12:31 PM, Eric Sandeen wrote:
> > Honest question: does one more sync make this deterministic, or is it a 
> > best-effort, um, hack?
> 
> I'm not quite sure why even 1 sync is needed.  :(
> 

Because of COW, we won't free up the data space until the transaction commits
because it is pinned, so doing the truncate and then immediately doing df will
show no difference.

> I'm not sure what bug this is trying to test; if you need 2 syncs for global 
> space stats to accurately reflect the fact that you chopped off the end of a 
> block, maybe that's ... still a bug?
> 

No, it's just COW for you, in this case we do our sync, stuff gets updated and
some metadata is cow'ed for once reason or another and now df doesn't quite
match up (in my case it was off by like 9 blocks), doing a second sync clears
these out and then df's match.

> Or if it's just the big-hammer question of "does the truncated space *ever* 
> get freed?" then maybe umount/remount/check would tell you that more 
> definitively.

Yeah but I think I'll do what you suggested on IRC and just use _within_range.
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


[PATCH v2] Btrfs-progs: optimize function btrfs_read_chunk_tree

2013-07-29 Thread Filipe David Borba Manana
After reading all device items from the chunk tree, don't
exit the loop and then navigate down the tree again to find
the chunk items. Instead just read all device items and
chunk items with a single tree search. This is possible
because all device items are found before any chunk item in
the chunks tree.

This is a port of the corresponding kernel patch to keep both
kernel and btrfs-progs identical:

https://patchwork.kernel.org/patch/2835105/

Signed-off-by: Filipe David Borba Manana 
---

V2: Simplified logic inside the loop
(suggested by Josef Bacik on irc).

 volumes.c |   28 +---
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/volumes.c b/volumes.c
index 0ff2283..2c69f28 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1718,14 +1718,13 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
if (!path)
return -ENOMEM;
 
-   /* first we search for all of the device items, and then we
-* read in all of the chunk items.  This way we can create chunk
-* mappings that reference all of the devices that are afound
-*/
+   /* Read all device items, and then all the chunk items. All
+  device items are found before any chunk item (their object id
+  is smaller than the lowest possible object id for a chunk
+  item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). */
key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
key.offset = 0;
key.type = 0;
-again:
ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
while(1) {
leaf = path->nodes[0];
@@ -1739,16 +1738,12 @@ again:
break;
}
btrfs_item_key_to_cpu(leaf, &found_key, slot);
-   if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
-   if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID)
-   break;
-   if (found_key.type == BTRFS_DEV_ITEM_KEY) {
-   struct btrfs_dev_item *dev_item;
-   dev_item = btrfs_item_ptr(leaf, slot,
+   if (found_key.type == BTRFS_DEV_ITEM_KEY) {
+   struct btrfs_dev_item *dev_item;
+   dev_item = btrfs_item_ptr(leaf, slot,
  struct btrfs_dev_item);
-   ret = read_one_dev(root, leaf, dev_item);
-   BUG_ON(ret);
-   }
+   ret = read_one_dev(root, leaf, dev_item);
+   BUG_ON(ret);
} else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) {
struct btrfs_chunk *chunk;
chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
@@ -1757,11 +1752,6 @@ again:
}
path->slots[0]++;
}
-   if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
-   key.objectid = 0;
-   btrfs_release_path(root, path);
-   goto again;
-   }
 
ret = 0;
 error:
-- 
1.7.9.5

--
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-progs: return immediately on tree search failure

2013-07-29 Thread Filipe David Borba Manana
If the chunk tree search failed in volumes.c:btrfs_read_chunk_tree()
return immediately, rather than looping and use the invalid contents
of the path structure, causing weird errors/crash at run time.

Signed-off-by: Filipe David Borba Manana 
---
 volumes.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/volumes.c b/volumes.c
index 0ff2283..79008de 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1727,6 +1727,8 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
key.type = 0;
 again:
ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+   if (ret < 0)
+   goto error;
while(1) {
leaf = path->nodes[0];
slot = path->slots[0];
-- 
1.7.9.5

--
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] xfstests: generic/315: allow a little tolerance for our used check

2013-07-29 Thread Josef Bacik
So df in btrfs is tricky at best, and relying on it for accurate information is
not great, but it's the best way to verify this test.  To get around btrfs being
inconsistent sometimes just use _within_tolerance to check our new df value to
make sure that our truncate did something.  With this patch I no longer see
transient failures of this test.  Thanks,

Signed-off-by: Josef Bacik 
---
 tests/generic/315 |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tests/generic/315 b/tests/generic/315
index 7cfc40d..9c01b5e 100644
--- a/tests/generic/315
+++ b/tests/generic/315
@@ -73,7 +73,8 @@ sync
 
 # Preallocated disk space should be released
 avail_done=`df -P $TEST_DIR | awk 'END {print $4}'`
-[ "$avail_done" -eq "$avail_begin" ] || _fail "Available disk space 
($avail_done KiB)"
+_within_tolerance "df" $avail_done $avail_begin 1%
+[ $? -eq 0 ] || _fail "Available disk space ($avail_done KiB) wanted 
($avail_begin KiB)"
 
 # success, all done
 exit
-- 
1.7.7.6

--
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: fix what bits we clear when erroring out from delalloc V2

2013-07-29 Thread Josef Bacik
First of all we no longer set EXTENT_DIRTY when we dirty an extent so this patch
removes the clearing of EXTENT_DIRTY since it confuses me.  This patch also adds
clearing EXTENT_DEFRAG and also doing EXTENT_DO_ACCOUNTING when we have errors.
This is because if we are clearing delalloc without adding an ordered extent
then we need to make sure the enospc handling stuff is accounted for.  Also if
this range was DEFRAG we need to make sure that bit is cleared so we dont leak
it.  Thanks,

Signed-off-by: Josef Bacik 
---
V1->V2:
- only do EXTENT_DO_ACCOUNTING in compress_file_range if we had an error

 fs/btrfs/inode.c |   49 -
 1 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a60be02..88fe045 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -484,15 +484,17 @@ cont:
compress_type, pages);
}
if (ret <= 0) {
+   unsigned long clear_flags = EXTENT_DELALLOC |
+   EXTENT_DEFRAG;
+   clear_flags |= (ret < 0) ? EXTENT_DO_ACCOUNTING : 0;
+
/*
 * inline extent creation worked or returned error,
 * we don't need to create any more async work items.
 * Unlock and free up our temp pages.
 */
extent_clear_unlock_delalloc(inode, start, end, NULL,
-EXTENT_DIRTY |
-EXTENT_DELALLOC,
-PAGE_UNLOCK |
+clear_flags, PAGE_UNLOCK |
 PAGE_CLEAR_DIRTY |
 PAGE_SET_WRITEBACK |
 PAGE_END_WRITEBACK);
@@ -593,9 +595,10 @@ free_pages_out:
 
 cleanup_and_out:
extent_clear_unlock_delalloc(inode, start, end, NULL,
-EXTENT_DIRTY | EXTENT_DELALLOC,
-PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
-PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
+EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING |
+EXTENT_DEFRAG, PAGE_UNLOCK |
+PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
+PAGE_END_WRITEBACK);
if (!trans || IS_ERR(trans))
btrfs_error(root->fs_info, ret, "Failed to join transaction");
else
@@ -770,8 +773,8 @@ retry:
extent_clear_unlock_delalloc(inode, async_extent->start,
async_extent->start +
async_extent->ram_size - 1,
-   NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
-   EXTENT_DIRTY, PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
+   NULL, EXTENT_LOCKED | EXTENT_DELALLOC,
+   PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
PAGE_SET_WRITEBACK);
ret = btrfs_submit_compressed_write(inode,
async_extent->start,
@@ -795,9 +798,9 @@ out_free:
 async_extent->start +
 async_extent->ram_size - 1,
 NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
-EXTENT_DIRTY, PAGE_UNLOCK |
-PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
-PAGE_END_WRITEBACK);
+EXTENT_DEFRAG | EXTENT_DO_ACCOUNTING,
+PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
+PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
kfree(async_extent);
goto again;
 }
@@ -884,7 +887,7 @@ static noinline int __cow_file_range(struct 
btrfs_trans_handle *trans,
if (ret == 0) {
extent_clear_unlock_delalloc(inode, start, end, NULL,
 EXTENT_LOCKED | EXTENT_DELALLOC |
-EXTENT_DIRTY, PAGE_UNLOCK |
+EXTENT_DEFRAG, PAGE_UNLOCK |
 PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
 PAGE_END_WRITEBACK);
 
@@ -995,10 +998,10 @@ out_reserve:
btrfs_free_reserved_extent(root, ins.objectid, ins.offset);
 out_unlock:
extent_clear_unlock_delalloc(inode, start, end, locked_page,
-EXTENT_LOCKED | EXTENT_DIRTY |
- 

Re: Recovering from btrfs error Couldn't read chunk root.

2013-07-29 Thread Kyriakos
Unrecoverable?
I know i cant mount and have access but my data are still there intact
( as i was using them till the reboot) i shouldn't be able to
extract/recover them to another disc? With any magic command without
mounting?
Any other solutions?

http://bpaste.net/show/118112/

On 29 July 2013 15:06, Wang Shilong  wrote:
>
> 在 2013-7-29,上午2:12,Kyriakos  写道:
>
>> Just tried it as you said with the -v option enabled
>> This is my output:
>>
>> http://bpaste.net/show/118112/
>>
>>
> This is a *long* email, and seems that btrfs list refuse it.
>
>>  Device extent: devid = 1, start = 1667558801408, len = 1073741824,
>> chunk offset = 1663255445504
>> Couldn't map the block 626309926912
>> btrfs: volumes.c:1020: btrfs_num_copies: Assertion `!(ce->start >
>> logical || ce->start + ce->size < logical)' failed.
>> Aborted (core dumped)
>
> Strange enough, we don't find any chunks during scanning process.
>
> And seems this is unrecoverable ~_~
>
>
> Wang,
>>
>>
>> Any thoughts?
>>
>> On 28 July 2013 08:17, Wang Shilong  wrote:
>>> Hello,
>>>
>>> It seems Btrfs Chunk Tree is damaged, so you can not mount Btrfs filesystem 
>>> any more.
>>>
>>> However, you can try the latest Btrfs-progs, Miao Xie implements chunk tree 
>>> recover function.
>>>
>>> The url is:
>>>
>>>git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git
>>>
>>>
>>> you can try it:
>>>btrfs chunk-recover  -v 
>>>
>>> This is Time-consuming, because it will scan the whole disk. And also,
>>> please catch output of processing(this is helpful to us if the recovery 
>>> fails, -v option
>>> enable this).
>>>
>>> Thanks,
>>> Wang
>>>
>>>
>>>
>>>
>>>
>>>
>
--
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 v2] Btrfs-progs: add missing path alloc return value check

2013-07-29 Thread Filipe David Borba Manana
Also remove unused path in extent-tree.c:finish_current_insert().

Signed-off-by: Filipe David Borba Manana 
---

V2: added 1 more path alloc check and removed unnecessary path
allocation in extent-tree.c:finish_current_insert().

 extent-tree.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/extent-tree.c b/extent-tree.c
index f597e16..e4adaa3 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1487,6 +1487,8 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle 
*trans,
}
 
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
path->reada = 1;
 
key.objectid = bytenr;
@@ -1577,6 +1579,8 @@ int btrfs_set_block_flags(struct btrfs_trans_handle 
*trans,
  BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
 
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
path->reada = 1;
 
key.objectid = bytenr;
@@ -2078,7 +2082,6 @@ static int finish_current_insert(struct 
btrfs_trans_handle *trans,
u64 end;
u64 priv;
struct btrfs_fs_info *info = extent_root->fs_info;
-   struct btrfs_path *path;
struct pending_extent_op *extent_op;
struct btrfs_key key;
int ret;
@@ -2086,8 +2089,6 @@ static int finish_current_insert(struct 
btrfs_trans_handle *trans,
btrfs_fs_incompat(extent_root->fs_info,
  BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
 
-   path = btrfs_alloc_path();
-
while(1) {
ret = find_first_extent_bit(&info->extent_ins, 0, &start,
&end, EXTENT_LOCKED);
@@ -2121,7 +2122,6 @@ static int finish_current_insert(struct 
btrfs_trans_handle *trans,
  GFP_NOFS);
kfree(extent_op);
}
-   btrfs_free_path(path);
return 0;
 }
 
-- 
1.7.9.5

--
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 v3] Btrfs-progs: add missing path alloc return value check

2013-07-29 Thread Filipe David Borba Manana
Also remove unused path in extent-tree.c:finish_current_insert().

Signed-off-by: Filipe David Borba Manana 
---

V2: added 1 more path alloc check and removed unnecessary path
allocation in extent-tree.c:finish_current_insert().
V3: added missing path alloc checks to dir-item.c, file-item.c
and btrfs-corrupt-block.c too.

 btrfs-corrupt-block.c |2 ++
 dir-item.c|2 ++
 extent-tree.c |8 
 file-item.c   |2 ++
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 8176fad..22facd4 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -159,6 +159,8 @@ static int corrupt_extent(struct btrfs_trans_handle *trans,
int should_del = rand() % 3;
 
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
 
key.objectid = bytenr;
key.type = (u8)-1;
diff --git a/dir-item.c b/dir-item.c
index f00485a..0ab3c5e 100644
--- a/dir-item.c
+++ b/dir-item.c
@@ -123,6 +123,8 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, 
struct btrfs_root
btrfs_set_key_type(&key, BTRFS_DIR_ITEM_KEY);
key.offset = btrfs_name_hash(name, name_len);
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
data_size = sizeof(*dir_item) + name_len;
dir_item = insert_with_overflow(trans, root, path, &key, data_size,
name, name_len);
diff --git a/extent-tree.c b/extent-tree.c
index f597e16..e4adaa3 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1487,6 +1487,8 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle 
*trans,
}
 
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
path->reada = 1;
 
key.objectid = bytenr;
@@ -1577,6 +1579,8 @@ int btrfs_set_block_flags(struct btrfs_trans_handle 
*trans,
  BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
 
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
path->reada = 1;
 
key.objectid = bytenr;
@@ -2078,7 +2082,6 @@ static int finish_current_insert(struct 
btrfs_trans_handle *trans,
u64 end;
u64 priv;
struct btrfs_fs_info *info = extent_root->fs_info;
-   struct btrfs_path *path;
struct pending_extent_op *extent_op;
struct btrfs_key key;
int ret;
@@ -2086,8 +2089,6 @@ static int finish_current_insert(struct 
btrfs_trans_handle *trans,
btrfs_fs_incompat(extent_root->fs_info,
  BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
 
-   path = btrfs_alloc_path();
-
while(1) {
ret = find_first_extent_bit(&info->extent_ins, 0, &start,
&end, EXTENT_LOCKED);
@@ -2121,7 +2122,6 @@ static int finish_current_insert(struct 
btrfs_trans_handle *trans,
  GFP_NOFS);
kfree(extent_op);
}
-   btrfs_free_path(path);
return 0;
 }
 
diff --git a/file-item.c b/file-item.c
index 9c787f0..82bf99e 100644
--- a/file-item.c
+++ b/file-item.c
@@ -417,6 +417,8 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
root = root->fs_info->csum_root;
 
path = btrfs_alloc_path();
+   if (!path)
+   return -ENOMEM;
 
while (1) {
key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
-- 
1.7.9.5

--
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: Online data deduplication

2013-07-29 Thread Liu Bo
On Mon, Jul 29, 2013 at 09:05:42PM +0530, Hemanth Kumar wrote:
> Hello,
> I am willing to perform QA on online data deduplication. From where
> can i download the patches?

Hi Hemanth Kumar H C,

I really appreciate this :)

Right now I'm planning v5 version patch set, which will come out probably in
this week(Only one bug is left on my hands)

v5 version will have a big change on disk format, performance, btw.

For v4 you can find it in the following url:
https://patchwork.kernel.org/patch/2565601/
https://patchwork.kernel.org/patch/2565591/

thanks,
-liubo
--
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 v2] Btrfs: optimize function btrfs_read_chunk_tree

2013-07-29 Thread Miao Xie
On mon, 29 Jul 2013 19:22:34 +0100, Filipe David Borba Manana wrote:
> After reading all device items from the chunk tree, don't
> exit the loop and then navigate down the tree again to find
> the chunk items. Instead just read all device items and
> chunk items with a single tree search. This is possible
> because all device items are found before any chunk item in
> the chunks tree.
> 
> Signed-off-by: Filipe David Borba Manana 
> ---
> 
> V2: Simplified logic inside the loop
> (suggested by Josef Bacik on irc).
> 
>  fs/btrfs/volumes.c |   30 ++
>  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 090f57c..45c5ec3 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -5676,14 +5676,13 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>   mutex_lock(&uuid_mutex);
>   lock_chunks(root);
>  
> - /* first we search for all of the device items, and then we
> -  * read in all of the chunk items.  This way we can create chunk
> -  * mappings that reference all of the devices that are afound
> -  */
> + /* Read all device items, and then all the chunk items. All
> +device items are found before any chunk item (their object id
> +is smaller than the lowest possible object id for a chunk
> +item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). */

According to the coding style of the kernel(Documentation/CodingStyle),
the preferred style for long (multi-line) comments is:

/*
 * This is the preferred style for multi-line
 * comments in the Linux kernel source code.
 *
 * Description:  A column of asterisks on the left side,
 * with beginning and ending almost-blank lines.
 */

The other code is OK.

Reviewed-by: Miao Xie 

>   key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
>   key.offset = 0;
>   key.type = 0;
> -again:
>   ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>   if (ret < 0)
>   goto error;
> @@ -5699,17 +5698,13 @@ again:
>   break;
>   }
>   btrfs_item_key_to_cpu(leaf, &found_key, slot);
> - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
> - if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID)
> - break;
> - if (found_key.type == BTRFS_DEV_ITEM_KEY) {
> - struct btrfs_dev_item *dev_item;
> - dev_item = btrfs_item_ptr(leaf, slot,
> + if (found_key.type == BTRFS_DEV_ITEM_KEY) {
> + struct btrfs_dev_item *dev_item;
> + dev_item = btrfs_item_ptr(leaf, slot,
> struct btrfs_dev_item);
> - ret = read_one_dev(root, leaf, dev_item);
> - if (ret)
> - goto error;
> - }
> + ret = read_one_dev(root, leaf, dev_item);
> + if (ret)
> + goto error;
>   } else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) {
>   struct btrfs_chunk *chunk;
>   chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
> @@ -5719,11 +5714,6 @@ again:
>   }
>   path->slots[0]++;
>   }
> - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
> - key.objectid = 0;
> - btrfs_release_path(path);
> - goto again;
> - }
>   ret = 0;
>  error:
>   unlock_chunks(root);
> 

--
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 v3] Btrfs-progs: add missing path alloc return value check

2013-07-29 Thread Miao Xie
On  mon, 29 Jul 2013 23:37:19 +0100, Filipe David Borba Manana wrote:
> Also remove unused path in extent-tree.c:finish_current_insert().
> 
> Signed-off-by: Filipe David Borba Manana 
> ---
> 
> V2: added 1 more path alloc check and removed unnecessary path
> allocation in extent-tree.c:finish_current_insert().
> V3: added missing path alloc checks to dir-item.c, file-item.c
> and btrfs-corrupt-block.c too.

fixup_extent_refs() in cmds-check.c and btrfs_add_root_ref() in root-tree.c also
miss the path allocation check.

Thanks
Miao

> 
>  btrfs-corrupt-block.c |2 ++
>  dir-item.c|2 ++
>  extent-tree.c |8 
>  file-item.c   |2 ++
>  4 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
> index 8176fad..22facd4 100644
> --- a/btrfs-corrupt-block.c
> +++ b/btrfs-corrupt-block.c
> @@ -159,6 +159,8 @@ static int corrupt_extent(struct btrfs_trans_handle 
> *trans,
>   int should_del = rand() % 3;
>  
>   path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
>  
>   key.objectid = bytenr;
>   key.type = (u8)-1;
> diff --git a/dir-item.c b/dir-item.c
> index f00485a..0ab3c5e 100644
> --- a/dir-item.c
> +++ b/dir-item.c
> @@ -123,6 +123,8 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle 
> *trans, struct btrfs_root
>   btrfs_set_key_type(&key, BTRFS_DIR_ITEM_KEY);
>   key.offset = btrfs_name_hash(name, name_len);
>   path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
>   data_size = sizeof(*dir_item) + name_len;
>   dir_item = insert_with_overflow(trans, root, path, &key, data_size,
>   name, name_len);
> diff --git a/extent-tree.c b/extent-tree.c
> index f597e16..e4adaa3 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -1487,6 +1487,8 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle 
> *trans,
>   }
>  
>   path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
>   path->reada = 1;
>  
>   key.objectid = bytenr;
> @@ -1577,6 +1579,8 @@ int btrfs_set_block_flags(struct btrfs_trans_handle 
> *trans,
> BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
>  
>   path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
>   path->reada = 1;
>  
>   key.objectid = bytenr;
> @@ -2078,7 +2082,6 @@ static int finish_current_insert(struct 
> btrfs_trans_handle *trans,
>   u64 end;
>   u64 priv;
>   struct btrfs_fs_info *info = extent_root->fs_info;
> - struct btrfs_path *path;
>   struct pending_extent_op *extent_op;
>   struct btrfs_key key;
>   int ret;
> @@ -2086,8 +2089,6 @@ static int finish_current_insert(struct 
> btrfs_trans_handle *trans,
>   btrfs_fs_incompat(extent_root->fs_info,
> BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA);
>  
> - path = btrfs_alloc_path();
> -
>   while(1) {
>   ret = find_first_extent_bit(&info->extent_ins, 0, &start,
>   &end, EXTENT_LOCKED);
> @@ -2121,7 +2122,6 @@ static int finish_current_insert(struct 
> btrfs_trans_handle *trans,
> GFP_NOFS);
>   kfree(extent_op);
>   }
> - btrfs_free_path(path);
>   return 0;
>  }
>  
> diff --git a/file-item.c b/file-item.c
> index 9c787f0..82bf99e 100644
> --- a/file-item.c
> +++ b/file-item.c
> @@ -417,6 +417,8 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
>   root = root->fs_info->csum_root;
>  
>   path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
>  
>   while (1) {
>   key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
> 

--
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 v2] Btrfs-progs: optimize function btrfs_read_chunk_tree

2013-07-29 Thread Miao Xie
On mon, 29 Jul 2013 19:35:49 +0100, Filipe David Borba Manana wrote:
> After reading all device items from the chunk tree, don't
> exit the loop and then navigate down the tree again to find
> the chunk items. Instead just read all device items and
> chunk items with a single tree search. This is possible
> because all device items are found before any chunk item in
> the chunks tree.
> 
> This is a port of the corresponding kernel patch to keep both
> kernel and btrfs-progs identical:
> 
> https://patchwork.kernel.org/patch/2835105/
> 
> Signed-off-by: Filipe David Borba Manana 
> ---
> 
> V2: Simplified logic inside the loop
> (suggested by Josef Bacik on irc).

The comment is the same to the kernel side(Coding style problem of the comment).

Reviewed-by: Miao Xie 

> 
>  volumes.c |   28 +---
>  1 file changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/volumes.c b/volumes.c
> index 0ff2283..2c69f28 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -1718,14 +1718,13 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>   if (!path)
>   return -ENOMEM;
>  
> - /* first we search for all of the device items, and then we
> -  * read in all of the chunk items.  This way we can create chunk
> -  * mappings that reference all of the devices that are afound
> -  */
> + /* Read all device items, and then all the chunk items. All
> +device items are found before any chunk item (their object id
> +is smaller than the lowest possible object id for a chunk
> +item - BTRFS_FIRST_CHUNK_TREE_OBJECTID). */
>   key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
>   key.offset = 0;
>   key.type = 0;
> -again:
>   ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>   while(1) {
>   leaf = path->nodes[0];
> @@ -1739,16 +1738,12 @@ again:
>   break;
>   }
>   btrfs_item_key_to_cpu(leaf, &found_key, slot);
> - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
> - if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID)
> - break;
> - if (found_key.type == BTRFS_DEV_ITEM_KEY) {
> - struct btrfs_dev_item *dev_item;
> - dev_item = btrfs_item_ptr(leaf, slot,
> + if (found_key.type == BTRFS_DEV_ITEM_KEY) {
> + struct btrfs_dev_item *dev_item;
> + dev_item = btrfs_item_ptr(leaf, slot,
> struct btrfs_dev_item);
> - ret = read_one_dev(root, leaf, dev_item);
> - BUG_ON(ret);
> - }
> + ret = read_one_dev(root, leaf, dev_item);
> + BUG_ON(ret);
>   } else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) {
>   struct btrfs_chunk *chunk;
>   chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
> @@ -1757,11 +1752,6 @@ again:
>   }
>   path->slots[0]++;
>   }
> - if (key.objectid == BTRFS_DEV_ITEMS_OBJECTID) {
> - key.objectid = 0;
> - btrfs_release_path(root, path);
> - goto again;
> - }
>  
>   ret = 0;
>  error:
> 

--
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: use list_for_each_entry_safe() when delete items

2013-07-29 Thread Miao Xie
On mon, 29 Jul 2013 11:48:32 +0400, Azat Khuzhin wrote:
> On Sat, Jul 27, 2013 at 2:12 PM, Azat Khuzhin  wrote:
>> Replace list_for_each_entry() by list_for_each_entry_safe() in
>> __btrfs_close_devices()
>>
>> There is another place that delete items lock_stripe_add(), but there we
>> don't need safe version, because after deleting we exit from loop.
>>
>> Signed-off-by: Azat Khuzhin 
>> ---
>>  fs/btrfs/volumes.c |4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 78b8717..1d1b595 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -616,13 +616,13 @@ static void free_device(struct rcu_head *head)
>>
>>  static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>>  {
>> -   struct btrfs_device *device;
>> +   struct btrfs_device *device, *next;
>>
>> if (--fs_devices->opened > 0)
>> return 0;
>>
>> mutex_lock(&fs_devices->device_list_mutex);
>> -   list_for_each_entry(device, &fs_devices->devices, dev_list) {
>> +   list_for_each_entry_safe(device, next, &fs_devices->devices, 
>> dev_list) {
>> struct btrfs_device *new_device;
>> struct rcu_string *name;
> 
> There is "kfree(device);" at the end of loop, maybe there must "goto
> again;" after it?
> (instead of this patch)

Your fix is right, we needn't search from the head once again.

The other fix way is:
call_rcu(&device->rcu, free_device);
+   device = new_device;
 }
but from the viewpoint of the readability, this way is not so good.

Reviewed-by: Miao Xie 

> 
>>
>> --
>> 1.7.10.4
>>
> 
> 
> 

--
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] xfstests: generic/315: allow a little tolerance for our used check

2013-07-29 Thread Jeff Liu
On 07/30/2013 02:55 AM, Josef Bacik wrote:

> So df in btrfs is tricky at best, and relying on it for accurate information 
> is
> not great, but it's the best way to verify this test.  To get around btrfs 
> being
> inconsistent sometimes just use _within_tolerance to check our new df value to
> make sure that our truncate did something.  With this patch I no longer see
> transient failures of this test.  Thanks,
> 
> Signed-off-by: Josef Bacik 
> ---
>  tests/generic/315 |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/generic/315 b/tests/generic/315
> index 7cfc40d..9c01b5e 100644
> --- a/tests/generic/315
> +++ b/tests/generic/315
> @@ -73,7 +73,8 @@ sync
>  
>  # Preallocated disk space should be released
>  avail_done=`df -P $TEST_DIR | awk 'END {print $4}'`
> -[ "$avail_done" -eq "$avail_begin" ] || _fail "Available disk space 
> ($avail_done KiB)"
> +_within_tolerance "df" $avail_done $avail_begin 1%
> +[ $? -eq 0 ] || _fail "Available disk space ($avail_done KiB) wanted 
> ($avail_begin KiB)"

Looks good to me.
Reviewed-by: Jie Liu 

Thanks,
-Jeff
--
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: cleanup arguments to extent_clear_unlock_delalloc

2013-07-29 Thread Miao Xie
On mon, 29 Jul 2013 11:23:00 -0400, Josef Bacik wrote:
> This patch removes the io_tree argument for extent_clear_unlock_delalloc since
> we always use &BTRFS_I(inode)->io_tree, and it separates out the extent tree
> operations from the page operations.  This way we just pass in the extent bits
> we want to clear and then pass in the operations we want done to the pages.
> This is because I'm going to fix what extent bits we clear in some cases and
> rather than add a bunch of new flags we'll just use the actual extent bits we
> want to clear.  Thanks,
> 
> Signed-off-by: Josef Bacik 

Reviewed-by: Miao Xie 

> ---
>  fs/btrfs/extent_io.c |   32 ---
>  fs/btrfs/extent_io.h |   21 +++
>  fs/btrfs/inode.c |  162 
> --
>  3 files changed, 85 insertions(+), 130 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index cc685dd..deaea9c 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1678,31 +1678,21 @@ out_failed:
>   return found;
>  }
>  
> -int extent_clear_unlock_delalloc(struct inode *inode,
> - struct extent_io_tree *tree,
> - u64 start, u64 end, struct page *locked_page,
> - unsigned long op)
> +int extent_clear_unlock_delalloc(struct inode *inode, u64 start, u64 end,
> +  struct page *locked_page,
> +  unsigned long clear_bits,
> +  unsigned long page_ops)
>  {
> + struct extent_io_tree *tree = &BTRFS_I(inode)->io_tree;
>   int ret;
>   struct page *pages[16];
>   unsigned long index = start >> PAGE_CACHE_SHIFT;
>   unsigned long end_index = end >> PAGE_CACHE_SHIFT;
>   unsigned long nr_pages = end_index - index + 1;
>   int i;
> - unsigned long clear_bits = 0;
> -
> - if (op & EXTENT_CLEAR_UNLOCK)
> - clear_bits |= EXTENT_LOCKED;
> - if (op & EXTENT_CLEAR_DIRTY)
> - clear_bits |= EXTENT_DIRTY;
> -
> - if (op & EXTENT_CLEAR_DELALLOC)
> - clear_bits |= EXTENT_DELALLOC;
>  
>   clear_extent_bit(tree, start, end, clear_bits, 1, 0, NULL, GFP_NOFS);
> - if (!(op & (EXTENT_CLEAR_UNLOCK_PAGE | EXTENT_CLEAR_DIRTY |
> - EXTENT_SET_WRITEBACK | EXTENT_END_WRITEBACK |
> - EXTENT_SET_PRIVATE2)))
> + if (page_ops == 0)
>   return 0;
>  
>   while (nr_pages > 0) {
> @@ -1711,20 +1701,20 @@ int extent_clear_unlock_delalloc(struct inode *inode,
>nr_pages, ARRAY_SIZE(pages)), pages);
>   for (i = 0; i < ret; i++) {
>  
> - if (op & EXTENT_SET_PRIVATE2)
> + if (page_ops & PAGE_SET_PRIVATE2)
>   SetPagePrivate2(pages[i]);
>  
>   if (pages[i] == locked_page) {
>   page_cache_release(pages[i]);
>   continue;
>   }
> - if (op & EXTENT_CLEAR_DIRTY)
> + if (page_ops & PAGE_CLEAR_DIRTY)
>   clear_page_dirty_for_io(pages[i]);
> - if (op & EXTENT_SET_WRITEBACK)
> + if (page_ops & PAGE_SET_WRITEBACK)
>   set_page_writeback(pages[i]);
> - if (op & EXTENT_END_WRITEBACK)
> + if (page_ops & PAGE_END_WRITEBACK)
>   end_page_writeback(pages[i]);
> - if (op & EXTENT_CLEAR_UNLOCK_PAGE)
> + if (page_ops & PAGE_UNLOCK)
>   unlock_page(pages[i]);
>   page_cache_release(pages[i]);
>   }
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index f7544af..c450620 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -44,14 +44,11 @@
>  #define EXTENT_BUFFER_DUMMY 9
>  
>  /* these are flags for extent_clear_unlock_delalloc */
> -#define EXTENT_CLEAR_UNLOCK_PAGE 0x1
> -#define EXTENT_CLEAR_UNLOCK   0x2
> -#define EXTENT_CLEAR_DELALLOC 0x4
> -#define EXTENT_CLEAR_DIRTY0x8
> -#define EXTENT_SET_WRITEBACK  0x10
> -#define EXTENT_END_WRITEBACK  0x20
> -#define EXTENT_SET_PRIVATE2   0x40
> -#define EXTENT_CLEAR_ACCOUNTING  0x80
> +#define PAGE_UNLOCK  (1 << 0)
> +#define PAGE_CLEAR_DIRTY (1 << 1)
> +#define PAGE_SET_WRITEBACK   (1 << 2)
> +#define PAGE_END_WRITEBACK   (1 << 3)
> +#define PAGE_SET_PRIVATE2(1 << 4)
>  
>  /*
>   * page->private values.  Every page that is controlled by the extent
> @@ -328,10 +325,10 @@ int map_private_extent_buffer(struct extent_buffer *eb, 
> unsigned long offset,
> unsigned long *map_len);
>  int extent_range_clear_dirty_for_io(struct inode *inode, u64 start, u64 end);
>  int extent_range_redirty_for_io(struct inode *inode,

Re: [PATCH] Btrfs: fix what bits we clear when erroring out from delalloc

2013-07-29 Thread Miao Xie
On mon, 29 Jul 2013 13:24:22 -0400, Josef Bacik wrote:
> First of all we no longer set EXTENT_DIRTY when we dirty an extent so this 
> patch
> removes the clearing of EXTENT_DIRTY since it confuses me.  This patch also 
> adds
> clearing EXTENT_DEFRAG and also doing EXTENT_DO_ACCOUNTING when we have 
> errors.
> This is because if we are clearing delalloc without adding an ordered extent
> then we need to make sure the enospc handling stuff is accounted for.  Also if
> this range was DEFRAG we need to make sure that bit is cleared so we dont leak
> it.  Thanks,
> 
> Signed-off-by: Josef Bacik 
> ---
>  fs/btrfs/inode.c |   46 ++
>  1 files changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a60be02..686ba5c 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -490,8 +490,9 @@ cont:
>* Unlock and free up our temp pages.
>*/
>   extent_clear_unlock_delalloc(inode, start, end, NULL,
> -  EXTENT_DIRTY |
> -  EXTENT_DELALLOC,
> +  EXTENT_DELALLOC |
> +  EXTENT_DO_ACCOUNTING |
> +  EXTENT_DEFRAG,
>PAGE_UNLOCK |
>PAGE_CLEAR_DIRTY |
>PAGE_SET_WRITEBACK |

I found we released the reserved space in cow_file_range_inline(), but at that 
time,
we didn't drop the outstanding_extents counter by the number of the delalloc 
extents,
so it might leave some reserved space which was not released. So I think we 
should remove
the release function in cow_file_range_inline().

(This bug is not introduced by your patch. but if we don't fix the above 
problem before
 applying your patch, the double release would happen because we have released 
the space
 in cow_file_range_inline())

> @@ -593,9 +594,10 @@ free_pages_out:
>  
>  cleanup_and_out:
>   extent_clear_unlock_delalloc(inode, start, end, NULL,
> -  EXTENT_DIRTY | EXTENT_DELALLOC,
> -  PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> -  PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
> +  EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING |
> +  EXTENT_DEFRAG, PAGE_UNLOCK |
> +  PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
> +  PAGE_END_WRITEBACK);
>   if (!trans || IS_ERR(trans))
>   btrfs_error(root->fs_info, ret, "Failed to join transaction");
>   else
> @@ -770,8 +772,8 @@ retry:
>   extent_clear_unlock_delalloc(inode, async_extent->start,
>   async_extent->start +
>   async_extent->ram_size - 1,
> - NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
> - EXTENT_DIRTY, PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> + NULL, EXTENT_LOCKED | EXTENT_DELALLOC,
> + PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
>   PAGE_SET_WRITEBACK);
>   ret = btrfs_submit_compressed_write(inode,
>   async_extent->start,
> @@ -795,9 +797,9 @@ out_free:
>async_extent->start +
>async_extent->ram_size - 1,
>NULL, EXTENT_LOCKED | EXTENT_DELALLOC |
> -  EXTENT_DIRTY, PAGE_UNLOCK |
> -  PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
> -  PAGE_END_WRITEBACK);
> +  EXTENT_DEFRAG | EXTENT_DO_ACCOUNTING,
> +  PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> +  PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK);
>   kfree(async_extent);
>   goto again;
>  }
> @@ -884,7 +886,7 @@ static noinline int __cow_file_range(struct 
> btrfs_trans_handle *trans,
>   if (ret == 0) {
>   extent_clear_unlock_delalloc(inode, start, end, NULL,
>EXTENT_LOCKED | EXTENT_DELALLOC |
> -  EXTENT_DIRTY, PAGE_UNLOCK |
> +  EXTENT_DEFRAG, PAGE_UNLOCK |
>PAGE_CLEAR_DIRTY | PAGE_SET_WRITEBACK |
>PAGE_END_WRITEBACK);

If we remove the reserved space release function in cow_file_range_inline() as 
I said above,
we should add EXTENT_DO_ACCOUNTING here.

I will send a patch to fix t