2010-07-29 09:05:09

by Miao Xie

Subject: [PATCH 7/7] btrfs: fix a wrong error check in add_ra_bio_pages()

From: Liu Bo <[email protected]>

Only when a page is not found by page_index, we'll go to the error check.

Signed-off-by: Liu Bo <[email protected]>
Signed-off-by: Miao Xie <[email protected]>
---
fs/btrfs/compression.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index cb3877c..8458840 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -467,7 +467,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
rcu_read_lock();
page = radix_tree_lookup(&mapping->page_tree, page_index);
rcu_read_unlock();
- if (page) {
+ if (!page) {
check_misses:
misses++;
if (misses > 4)
--
1.7.0.1

2010-07-29 10:38:42

by Yan, Zheng

Subject: Re: [PATCH 7/7] btrfs: fix a wrong error check in add_ra_bio_pages()

2010/7/29 Miao Xie <[email protected]>:
> From: Liu Bo <[email protected]>
>
> Only when a page is not found by page_index, we'll go to the error check.
>
> Signed-off-by: Liu Bo <[email protected]>
> Signed-off-by: Miao Xie <[email protected]>
> ---
> ?fs/btrfs/compression.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index cb3877c..8458840 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -467,7 +467,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
> ? ? ? ? ? ? ? ?rcu_read_lock();
> ? ? ? ? ? ? ? ?page = radix_tree_lookup(&mapping->page_tree, page_index);
> ? ? ? ? ? ? ? ?rcu_read_unlock();
> - ? ? ? ? ? ? ? if (page) {
> + ? ? ? ? ? ? ? if (!page) {
> ?check_misses:
> ? ? ? ? ? ? ? ? ? ? ? ?misses++;
> ? ? ? ? ? ? ? ? ? ? ? ?if (misses > 4)

This patch is wrong. The word "miss" here means "miss for read-ahead because
the page is already in the page cache"

Yan, Zheng

2010-07-29 10:47:09

by Miao Xie

Subject: Re: [PATCH 7/7] btrfs: fix a wrong error check in add_ra_bio_pages()

On Thu, 29 Jul 2010 18:38:39 +0800, Yan, Zheng wrote:
> 2010/7/29 Miao Xie<[email protected]>:
>> From: Liu Bo<[email protected]>
>>
>> Only when a page is not found by page_index, we'll go to the error check.
>>
>> Signed-off-by: Liu Bo<[email protected]>
>> Signed-off-by: Miao Xie<[email protected]>
>> ---
>> fs/btrfs/compression.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
>> index cb3877c..8458840 100644
>> --- a/fs/btrfs/compression.c
>> +++ b/fs/btrfs/compression.c
>> @@ -467,7 +467,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>> rcu_read_lock();
>> page = radix_tree_lookup(&mapping->page_tree, page_index);
>> rcu_read_unlock();
>> - if (page) {
>> + if (!page) {
>> check_misses:
>> misses++;
>> if (misses> 4)
>
> This patch is wrong. The word "miss" here means "miss for read-ahead because
> the page is already in the page cache"

Oh, my mistake. Sorry!
Please ignore this patch.

Thanks for your review! How about the others?
Miao

>
> Yan, Zheng
>
>