[PATCH] Btrfs: remove warning check in io_ctl_map_page

[PATCH] Btrfs: remove warning check in io_ctl_map_page

[shhuiw]

From: Wang Sheng-Hui <shhuiw@gmail.com>

io_ctl_map_page is called by many functions in free-space-cache.
In some scenarios, the ->cur is not null, e.g. io_ctl_add_entry.
Remove the check here.

Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
 fs/btrfs/free-space-cache.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index c3318cb..4ea66d4 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -307,7 +307,6 @@ static void io_ctl_unmap_page(struct io_ctl *io_ctl)
 
 static void io_ctl_map_page(struct io_ctl *io_ctl, int clear)
 {
-	WARN_ON(io_ctl->cur);
 	BUG_ON(io_ctl->index >= io_ctl->num_pages);
 	io_ctl->page = io_ctl->pages[io_ctl->index++];
 	io_ctl->cur = kmap(io_ctl->page);
-- 
1.6.0.2

[PATCH] Btrfs: check crc area early in io_ctl_init

[shhuiw]

From: Wang Sheng-Hui <shhuiw@gmail.com>

When use crc area, we should check if it can host the desired num of
crcs. Add the check in init stage.

And the check should be more strict: the first page has sizeof(u64)*2
cannot used for crc.

Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
 fs/btrfs/free-space-cache.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 4ea66d4..058fc9b 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -286,8 +286,14 @@ static int io_ctl_init(struct io_ctl *io_ctl, struct inode *inode,
 	if (!io_ctl->pages)
 		return -ENOMEM;
 	io_ctl->root = root;
-	if (btrfs_ino(inode) != BTRFS_FREE_INO_OBJECTID)
+	if (btrfs_ino(inode) != BTRFS_FREE_INO_OBJECTID) {
 		io_ctl->check_crcs = 1;
+		if ((io_ctl.num_pages * sizeof(u32)) >
+		    (PAGE_CACHE_SIZE - sizeof(u64) * 2)) {
+			WARN_ON(1);
+			return -1;
+		}
+	}
 	return 0;
 }
 
@@ -917,7 +923,8 @@ int __btrfs_write_out_cache(struct btrfs_root *root, struct inode *inode,
 
 	/* Make sure we can fit our crcs into the first page */
 	if (io_ctl.check_crcs &&
-	    (io_ctl.num_pages * sizeof(u32)) >= PAGE_CACHE_SIZE) {
+	    (io_ctl.num_pages * sizeof(u32)) >
+	    (PAGE_CACHE_SIZE - sizeof(u64) * 2)) {
 		WARN_ON(1);
 		goto out_nospc;
 	}
-- 
1.6.0.2

[PATCH] Btrfs: cleanup: assign path->nodes[0]to leaf after ret check in __btrfs_write_out_cache

[shhuiw]

From: Wang Sheng-Hui <shhuiw@gmail.com>

If the ret check fails, the assignment is useless.
Move the assignment after the check.

Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
 fs/btrfs/free-space-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 058fc9b..d7b3286 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -1033,7 +1033,6 @@ int __btrfs_write_out_cache(struct btrfs_root *root, struct inode *inode,
 				 GFP_NOFS);
 		goto out;
 	}
-	leaf = path->nodes[0];
 	if (ret > 0) {
 		struct btrfs_key found_key;
 		BUG_ON(!path->slots[0]);
@@ -1049,6 +1048,7 @@ int __btrfs_write_out_cache(struct btrfs_root *root, struct inode *inode,
 			goto out;
 		}
 	}
+	leaf = path->nodes[0];
 
 	BTRFS_I(inode)->generation = trans->transid;
 	header = btrfs_item_ptr(leaf, path->slots[0],
-- 
1.6.0.2

Re: [PATCH] Btrfs: remove warning check in io_ctl_map_page

[Liu Bo]

On Fri, Nov 30, 2012 at 02:02:26PM +0800, shhuiw@gmail.com wrote:
> From: Wang Sheng-Hui <shhuiw@gmail.com>
> 
> io_ctl_map_page is called by many functions in free-space-cache.
> In some scenarios, the ->cur is not null, e.g. io_ctl_add_entry.
> Remove the check here.

Hi Wang,

Seems to be impossible according to the code, even in io_ctl_add_entry().

So have you seen such a warning in your flight running?  If you do,
maybe you can post it here and we can figure out the whys.

thanks,
liubo

> 
> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
> ---
>  fs/btrfs/free-space-cache.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index c3318cb..4ea66d4 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -307,7 +307,6 @@ static void io_ctl_unmap_page(struct io_ctl *io_ctl)
>  
>  static void io_ctl_map_page(struct io_ctl *io_ctl, int clear)
>  {
> -	WARN_ON(io_ctl->cur);
>  	BUG_ON(io_ctl->index >= io_ctl->num_pages);
>  	io_ctl->page = io_ctl->pages[io_ctl->index++];
>  	io_ctl->cur = kmap(io_ctl->page);
> -- 
> 1.6.0.2
> 
> --
> 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

Re: [PATCH] Btrfs: remove warning check in io_ctl_map_page

[Wang Sheng-Hui]

On 2012年11月30日 17:02, Liu Bo wrote:

> On Fri, Nov 30, 2012 at 02:02:26PM +0800, shhuiw@gmail.com wrote:
>> From: Wang Sheng-Hui <shhuiw@gmail.com>
>>
>> io_ctl_map_page is called by many functions in free-space-cache.
>> In some scenarios, the ->cur is not null, e.g. io_ctl_add_entry.
>> Remove the check here.
> 
> Hi Wang,
> 
> Seems to be impossible according to the code, even in io_ctl_add_entry().
> 
> So have you seen such a warning in your flight running?  If you do,
> maybe you can post it here and we can figure out the whys.
> 


Hi Liu Bo,

I just walked through the code. reread io_ctl_add_entry, and just noticed
that io_ctl_set_crc is called before io_ctl_map_page, which can guarantee
the ->cur is set to null before map.

Thanks for helping me understand the code.

Regards,
Sheng-Hui


> thanks,
> liubo
> 
>>
>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
>> ---
>>  fs/btrfs/free-space-cache.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
>> index c3318cb..4ea66d4 100644
>> --- a/fs/btrfs/free-space-cache.c
>> +++ b/fs/btrfs/free-space-cache.c
>> @@ -307,7 +307,6 @@ static void io_ctl_unmap_page(struct io_ctl *io_ctl)
>>  
>>  static void io_ctl_map_page(struct io_ctl *io_ctl, int clear)
>>  {
>> -	WARN_ON(io_ctl->cur);
>>  	BUG_ON(io_ctl->index >= io_ctl->num_pages);
>>  	io_ctl->page = io_ctl->pages[io_ctl->index++];
>>  	io_ctl->cur = kmap(io_ctl->page);
>> -- 
>> 1.6.0.2
>>
>> --
>> 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