* [PATCH] btrfs: make inode ref log recovery faster
@ 2011-02-22 11:42 liubo
2011-02-22 14:32 ` David Sterba
0 siblings, 1 reply; 7+ messages in thread
From: liubo @ 2011-02-22 11:42 UTC (permalink / raw)
To: Linux Btrfs; +Cc: Chris Mason
When we recover from crash via write-ahead log tree and process
the inode refs, for each btrfs_inode_ref item, we will
1) check if we already have a perfect match in fs/file tree, if
we have, then we're done.
2) search the corresponding back reference in fs/file tree, and
check all the names in this back reference to see if they are
also in the log to avoid conflict corners.
3) recover the logged inode refs to fs/file tree.
In current btrfs, however,
- for 2)'s check, once is enough, since the checked back references
will remain unchanged after processing all the inode refs belonged
to the key.
- it has no need to do another 1) between 2) and 3).
This patch focus on the above problems and
I've made a small test to show how it improves,
$dd if=/dev/zero of=foobar bs=4K count=1
$sync
$make 100 hard links continuously, like ln foobar link_i
$fsync foobar
$echo b > /proc/sysrq-trigger
after reboot
$time mount DEV PATH
without patch:
real 0m0.285s
user 0m0.001s
sys 0m0.009s
with patch:
real 0m0.123s
user 0m0.000s
sys 0m0.010s
Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
fs/btrfs/tree-log.c | 33 +++++++++++----------------------
1 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index a4bbb85..8f2a9f3 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -799,12 +799,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
struct inode *dir;
int ret;
struct btrfs_inode_ref *ref;
- struct btrfs_dir_item *di;
struct inode *inode;
char *name;
int namelen;
unsigned long ref_ptr;
unsigned long ref_end;
+ int search_done = 0;
/*
* it is possible that we didn't log all the parent directories
@@ -845,7 +845,10 @@ again:
* existing back reference, and we don't want to create
* dangling pointers in the directory.
*/
-conflict_again:
+
+ if (search_done)
+ goto insert;
+
ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
if (ret == 0) {
char *victim_name;
@@ -888,35 +891,21 @@ conflict_again:
victim_name_len);
kfree(victim_name);
btrfs_release_path(root, path);
- goto conflict_again;
}
kfree(victim_name);
ptr = (unsigned long)(victim_ref + 1) + victim_name_len;
}
BUG_ON(ret);
- }
- btrfs_release_path(root, path);
- /* look for a conflicting sequence number */
- di = btrfs_lookup_dir_index_item(trans, root, path, dir->i_ino,
- btrfs_inode_ref_index(eb, ref),
- name, namelen, 0);
- if (di && !IS_ERR(di)) {
- ret = drop_one_dir_item(trans, root, path, dir, di);
- BUG_ON(ret);
- }
- btrfs_release_path(root, path);
-
-
- /* look for a conflicting name */
- di = btrfs_lookup_dir_item(trans, root, path, dir->i_ino,
- name, namelen, 0);
- if (di && !IS_ERR(di)) {
- ret = drop_one_dir_item(trans, root, path, dir, di);
- BUG_ON(ret);
+ /*
+ * NOTE: we have searched root tree and checked the
+ * coresponding ref, it does not need to check again.
+ */
+ search_done = 1;
}
btrfs_release_path(root, path);
+insert:
/* insert our name */
ret = btrfs_add_link(trans, dir, inode, name, namelen, 0,
btrfs_inode_ref_index(eb, ref));
--
1.6.5.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: make inode ref log recovery faster
2011-02-22 11:42 [PATCH] btrfs: make inode ref log recovery faster liubo
@ 2011-02-22 14:32 ` David Sterba
2011-02-23 1:12 ` liubo
0 siblings, 1 reply; 7+ messages in thread
From: David Sterba @ 2011-02-22 14:32 UTC (permalink / raw)
To: Linux Btrfs
Hi,
no deeper analysis done, but the double free error was obvious :)
On Tue, Feb 22, 2011 at 07:42:25PM +0800, liubo wrote:
>
> When we recover from crash via write-ahead log tree and process
> the inode refs, for each btrfs_inode_ref item, we will
> 1) check if we already have a perfect match in fs/file tree, if
> we have, then we're done.
> 2) search the corresponding back reference in fs/file tree, and
> check all the names in this back reference to see if they are
> also in the log to avoid conflict corners.
> 3) recover the logged inode refs to fs/file tree.
>
> In current btrfs, however,
> - for 2)'s check, once is enough, since the checked back references
> will remain unchanged after processing all the inode refs belonged
> to the key.
> - it has no need to do another 1) between 2) and 3).
>
> This patch focus on the above problems and
> I've made a small test to show how it improves,
>
> $dd if=/dev/zero of=foobar bs=4K count=1
> $sync
> $make 100 hard links continuously, like ln foobar link_i
> $fsync foobar
> $echo b > /proc/sysrq-trigger
> after reboot
> $time mount DEV PATH
>
> without patch:
> real 0m0.285s
> user 0m0.001s
> sys 0m0.009s
>
> with patch:
> real 0m0.123s
> user 0m0.000s
> sys 0m0.010s
>
> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
> ---
> fs/btrfs/tree-log.c | 33 +++++++++++----------------------
> 1 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index a4bbb85..8f2a9f3 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -799,12 +799,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
> struct inode *dir;
> int ret;
> struct btrfs_inode_ref *ref;
> - struct btrfs_dir_item *di;
> struct inode *inode;
> char *name;
> int namelen;
> unsigned long ref_ptr;
> unsigned long ref_end;
> + int search_done = 0;
>
> /*
> * it is possible that we didn't log all the parent directories
> @@ -845,7 +845,10 @@ again:
> * existing back reference, and we don't want to create
> * dangling pointers in the directory.
> */
> -conflict_again:
> +
> + if (search_done)
> + goto insert;
> +
> ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
> if (ret == 0) {
> char *victim_name;
> @@ -888,35 +891,21 @@ conflict_again:
> victim_name_len);
> kfree(victim_name);
^^^^^^^^^^^^^^^^^^^
> btrfs_release_path(root, path);
> - goto conflict_again;
> }
> kfree(victim_name);
^^^^^^^^^^^^^^^^^^^
double free
> ptr = (unsigned long)(victim_ref + 1) + victim_name_len;
> }
> BUG_ON(ret);
> - }
> - btrfs_release_path(root, path);
>
> - /* look for a conflicting sequence number */
> - di = btrfs_lookup_dir_index_item(trans, root, path, dir->i_ino,
> - btrfs_inode_ref_index(eb, ref),
> - name, namelen, 0);
> - if (di && !IS_ERR(di)) {
> - ret = drop_one_dir_item(trans, root, path, dir, di);
> - BUG_ON(ret);
> - }
> - btrfs_release_path(root, path);
> -
> -
> - /* look for a conflicting name */
> - di = btrfs_lookup_dir_item(trans, root, path, dir->i_ino,
> - name, namelen, 0);
> - if (di && !IS_ERR(di)) {
> - ret = drop_one_dir_item(trans, root, path, dir, di);
> - BUG_ON(ret);
> + /*
> + * NOTE: we have searched root tree and checked the
> + * coresponding ref, it does not need to check again.
> + */
> + search_done = 1;
> }
> btrfs_release_path(root, path);
>
> +insert:
> /* insert our name */
> ret = btrfs_add_link(trans, dir, inode, name, namelen, 0,
> btrfs_inode_ref_index(eb, ref));
> --
d/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: make inode ref log recovery faster
2011-02-22 14:32 ` David Sterba
@ 2011-02-23 1:12 ` liubo
2011-02-23 1:30 ` Josef Bacik
0 siblings, 1 reply; 7+ messages in thread
From: liubo @ 2011-02-23 1:12 UTC (permalink / raw)
To: Linux Btrfs
On 02/22/2011 10:32 PM, David Sterba wrote:
> Hi,
>
> no deeper analysis done, but the double free error was obvious :)
>
> On Tue, Feb 22, 2011 at 07:42:25PM +0800, liubo wrote:
>> When we recover from crash via write-ahead log tree and process
>> the inode refs, for each btrfs_inode_ref item, we will
>> 1) check if we already have a perfect match in fs/file tree, if
>> we have, then we're done.
>> 2) search the corresponding back reference in fs/file tree, and
>> check all the names in this back reference to see if they are
>> also in the log to avoid conflict corners.
>> 3) recover the logged inode refs to fs/file tree.
>>
>> In current btrfs, however,
>> - for 2)'s check, once is enough, since the checked back references
>> will remain unchanged after processing all the inode refs belonged
>> to the key.
>> - it has no need to do another 1) between 2) and 3).
>>
>> This patch focus on the above problems and
>> I've made a small test to show how it improves,
>>
>> $dd if=/dev/zero of=foobar bs=4K count=1
>> $sync
>> $make 100 hard links continuously, like ln foobar link_i
>> $fsync foobar
>> $echo b > /proc/sysrq-trigger
>> after reboot
>> $time mount DEV PATH
>>
>> without patch:
>> real 0m0.285s
>> user 0m0.001s
>> sys 0m0.009s
>>
>> with patch:
>> real 0m0.123s
>> user 0m0.000s
>> sys 0m0.010s
>>
>> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
>> ---
>> fs/btrfs/tree-log.c | 33 +++++++++++----------------------
>> 1 files changed, 11 insertions(+), 22 deletions(-)
>>
>> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
>> index a4bbb85..8f2a9f3 100644
>> --- a/fs/btrfs/tree-log.c
>> +++ b/fs/btrfs/tree-log.c
>> @@ -799,12 +799,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
>> struct inode *dir;
>> int ret;
>> struct btrfs_inode_ref *ref;
>> - struct btrfs_dir_item *di;
>> struct inode *inode;
>> char *name;
>> int namelen;
>> unsigned long ref_ptr;
>> unsigned long ref_end;
>> + int search_done = 0;
>>
>> /*
>> * it is possible that we didn't log all the parent directories
>> @@ -845,7 +845,10 @@ again:
>> * existing back reference, and we don't want to create
>> * dangling pointers in the directory.
>> */
>> -conflict_again:
>> +
>> + if (search_done)
>> + goto insert;
>> +
>> ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
>> if (ret == 0) {
>> char *victim_name;
>> @@ -888,35 +891,21 @@ conflict_again:
>> victim_name_len);
>> kfree(victim_name);
> ^^^^^^^^^^^^^^^^^^^
>> btrfs_release_path(root, path);
>> - goto conflict_again;
>> }
>> kfree(victim_name);
> ^^^^^^^^^^^^^^^^^^^
> double free
thanks for reviewing, but the first one is followed by a goto phrase, so IMO it is ok.
>
>> ptr = (unsigned long)(victim_ref + 1) + victim_name_len;
>> }
>> BUG_ON(ret);
>> - }
>> - btrfs_release_path(root, path);
>>
>> - /* look for a conflicting sequence number */
>> - di = btrfs_lookup_dir_index_item(trans, root, path, dir->i_ino,
>> - btrfs_inode_ref_index(eb, ref),
>> - name, namelen, 0);
>> - if (di && !IS_ERR(di)) {
>> - ret = drop_one_dir_item(trans, root, path, dir, di);
>> - BUG_ON(ret);
>> - }
>> - btrfs_release_path(root, path);
>> -
>> -
>> - /* look for a conflicting name */
>> - di = btrfs_lookup_dir_item(trans, root, path, dir->i_ino,
>> - name, namelen, 0);
>> - if (di && !IS_ERR(di)) {
>> - ret = drop_one_dir_item(trans, root, path, dir, di);
>> - BUG_ON(ret);
>> + /*
>> + * NOTE: we have searched root tree and checked the
>> + * coresponding ref, it does not need to check again.
>> + */
>> + search_done = 1;
>> }
>> btrfs_release_path(root, path);
>>
>> +insert:
>> /* insert our name */
>> ret = btrfs_add_link(trans, dir, inode, name, namelen, 0,
>> btrfs_inode_ref_index(eb, ref));
>> --
>
> d/
> --
> 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] 7+ messages in thread
* Re: [PATCH] btrfs: make inode ref log recovery faster
2011-02-23 1:12 ` liubo
@ 2011-02-23 1:30 ` Josef Bacik
2011-02-23 1:42 ` liubo
0 siblings, 1 reply; 7+ messages in thread
From: Josef Bacik @ 2011-02-23 1:30 UTC (permalink / raw)
To: liubo; +Cc: Linux Btrfs
On Wed, Feb 23, 2011 at 09:12:36AM +0800, liubo wrote:
> On 02/22/2011 10:32 PM, David Sterba wrote:
> > Hi,
> >
> > no deeper analysis done, but the double free error was obvious :)
> >
> > On Tue, Feb 22, 2011 at 07:42:25PM +0800, liubo wrote:
> >> When we recover from crash via write-ahead log tree and process
> >> the inode refs, for each btrfs_inode_ref item, we will
> >> 1) check if we already have a perfect match in fs/file tree, if
> >> we have, then we're done.
> >> 2) search the corresponding back reference in fs/file tree, and
> >> check all the names in this back reference to see if they are
> >> also in the log to avoid conflict corners.
> >> 3) recover the logged inode refs to fs/file tree.
> >>
> >> In current btrfs, however,
> >> - for 2)'s check, once is enough, since the checked back references
> >> will remain unchanged after processing all the inode refs belonged
> >> to the key.
> >> - it has no need to do another 1) between 2) and 3).
> >>
> >> This patch focus on the above problems and
> >> I've made a small test to show how it improves,
> >>
> >> $dd if=/dev/zero of=foobar bs=4K count=1
> >> $sync
> >> $make 100 hard links continuously, like ln foobar link_i
> >> $fsync foobar
> >> $echo b > /proc/sysrq-trigger
> >> after reboot
> >> $time mount DEV PATH
> >>
> >> without patch:
> >> real 0m0.285s
> >> user 0m0.001s
> >> sys 0m0.009s
> >>
> >> with patch:
> >> real 0m0.123s
> >> user 0m0.000s
> >> sys 0m0.010s
> >>
> >> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
> >> ---
> >> fs/btrfs/tree-log.c | 33 +++++++++++----------------------
> >> 1 files changed, 11 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> >> index a4bbb85..8f2a9f3 100644
> >> --- a/fs/btrfs/tree-log.c
> >> +++ b/fs/btrfs/tree-log.c
> >> @@ -799,12 +799,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
> >> struct inode *dir;
> >> int ret;
> >> struct btrfs_inode_ref *ref;
> >> - struct btrfs_dir_item *di;
> >> struct inode *inode;
> >> char *name;
> >> int namelen;
> >> unsigned long ref_ptr;
> >> unsigned long ref_end;
> >> + int search_done = 0;
> >>
> >> /*
> >> * it is possible that we didn't log all the parent directories
> >> @@ -845,7 +845,10 @@ again:
> >> * existing back reference, and we don't want to create
> >> * dangling pointers in the directory.
> >> */
> >> -conflict_again:
> >> +
> >> + if (search_done)
> >> + goto insert;
> >> +
> >> ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
> >> if (ret == 0) {
> >> char *victim_name;
> >> @@ -888,35 +891,21 @@ conflict_again:
> >> victim_name_len);
> >> kfree(victim_name);
> > ^^^^^^^^^^^^^^^^^^^
> >> btrfs_release_path(root, path);
> >> - goto conflict_again;
> >> }
> >> kfree(victim_name);
> > ^^^^^^^^^^^^^^^^^^^
> > double free
>
> thanks for reviewing, but the first one is followed by a goto phrase, so IMO it is ok.
>
Your patch removes that goto, so it's not ok. Thanks,
Josef
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: make inode ref log recovery faster
2011-02-23 1:30 ` Josef Bacik
@ 2011-02-23 1:42 ` liubo
2011-02-23 7:09 ` [PATCH v2] " liubo
0 siblings, 1 reply; 7+ messages in thread
From: liubo @ 2011-02-23 1:42 UTC (permalink / raw)
To: Josef Bacik; +Cc: Linux Btrfs, David Sterba
On 02/23/2011 09:30 AM, Josef Bacik wrote:
> On Wed, Feb 23, 2011 at 09:12:36AM +0800, liubo wrote:
>> On 02/22/2011 10:32 PM, David Sterba wrote:
>>> Hi,
>>>
>>> no deeper analysis done, but the double free error was obvious :)
>>>
>>> On Tue, Feb 22, 2011 at 07:42:25PM +0800, liubo wrote:
>>>> When we recover from crash via write-ahead log tree and process
>>>> the inode refs, for each btrfs_inode_ref item, we will
>>>> 1) check if we already have a perfect match in fs/file tree, if
>>>> we have, then we're done.
>>>> 2) search the corresponding back reference in fs/file tree, and
>>>> check all the names in this back reference to see if they are
>>>> also in the log to avoid conflict corners.
>>>> 3) recover the logged inode refs to fs/file tree.
>>>>
>>>> In current btrfs, however,
>>>> - for 2)'s check, once is enough, since the checked back references
>>>> will remain unchanged after processing all the inode refs belonged
>>>> to the key.
>>>> - it has no need to do another 1) between 2) and 3).
>>>>
>>>> This patch focus on the above problems and
>>>> I've made a small test to show how it improves,
>>>>
>>>> $dd if=/dev/zero of=foobar bs=4K count=1
>>>> $sync
>>>> $make 100 hard links continuously, like ln foobar link_i
>>>> $fsync foobar
>>>> $echo b > /proc/sysrq-trigger
>>>> after reboot
>>>> $time mount DEV PATH
>>>>
>>>> without patch:
>>>> real 0m0.285s
>>>> user 0m0.001s
>>>> sys 0m0.009s
>>>>
>>>> with patch:
>>>> real 0m0.123s
>>>> user 0m0.000s
>>>> sys 0m0.010s
>>>>
>>>> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
>>>> ---
>>>> fs/btrfs/tree-log.c | 33 +++++++++++----------------------
>>>> 1 files changed, 11 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
>>>> index a4bbb85..8f2a9f3 100644
>>>> --- a/fs/btrfs/tree-log.c
>>>> +++ b/fs/btrfs/tree-log.c
>>>> @@ -799,12 +799,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
>>>> struct inode *dir;
>>>> int ret;
>>>> struct btrfs_inode_ref *ref;
>>>> - struct btrfs_dir_item *di;
>>>> struct inode *inode;
>>>> char *name;
>>>> int namelen;
>>>> unsigned long ref_ptr;
>>>> unsigned long ref_end;
>>>> + int search_done = 0;
>>>>
>>>> /*
>>>> * it is possible that we didn't log all the parent directories
>>>> @@ -845,7 +845,10 @@ again:
>>>> * existing back reference, and we don't want to create
>>>> * dangling pointers in the directory.
>>>> */
>>>> -conflict_again:
>>>> +
>>>> + if (search_done)
>>>> + goto insert;
>>>> +
>>>> ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
>>>> if (ret == 0) {
>>>> char *victim_name;
>>>> @@ -888,35 +891,21 @@ conflict_again:
>>>> victim_name_len);
>>>> kfree(victim_name);
>>> ^^^^^^^^^^^^^^^^^^^
>>>> btrfs_release_path(root, path);
>>>> - goto conflict_again;
>>>> }
>>>> kfree(victim_name);
>>> ^^^^^^^^^^^^^^^^^^^
>>> double free
>> thanks for reviewing, but the first one is followed by a goto phrase, so IMO it is ok.
>>
>
> Your patch removes that goto, so it's not ok. Thanks,
ahh, my fault.
I'll fix it, thanks a lot, :)
liubo
>
> 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] 7+ messages in thread
* [PATCH v2] btrfs: make inode ref log recovery faster
2011-02-23 1:42 ` liubo
@ 2011-02-23 7:09 ` liubo
2011-02-23 7:20 ` Li Zefan
0 siblings, 1 reply; 7+ messages in thread
From: liubo @ 2011-02-23 7:09 UTC (permalink / raw)
To: Linux Btrfs; +Cc: Josef Bacik, David Sterba
When we recover from crash via write-ahead log tree and process
the inode refs, for each btrfs_inode_ref item, we will
1) check if we already have a perfect match in fs/file tree, if
we have, then we're done.
2) search the corresponding back reference in fs/file tree, and
check all the names in this back reference to see if they are
also in the log to avoid conflict corners.
3) recover the logged inode refs to fs/file tree.
In current btrfs, however,
- for 2)'s check, once is enough, since the checked back reference
will remain unchanged after processing all the inode refs belonged
to the key.
- it has no need to do another 1) between 2) and 3).
I've made a small test to show how it improves,
$dd if=/dev/zero of=foobar bs=4K count=1
$sync
$make 100 hard links continuously, like ln foobar link_i
$fsync foobar
$echo b > /proc/sysrq-trigger
after reboot
$time mount DEV PATH
without patch:
real 0m0.285s
user 0m0.001s
sys 0m0.009s
with patch:
real 0m0.123s
user 0m0.000s
sys 0m0.010s
Changelog v1->v2:
- fix double free - pointed by David Sterba
Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
---
fs/btrfs/tree-log.c | 37 +++++++++++++------------------------
1 files changed, 13 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index a4bbb85..bbdc63e 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -799,12 +799,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
struct inode *dir;
int ret;
struct btrfs_inode_ref *ref;
- struct btrfs_dir_item *di;
struct inode *inode;
char *name;
int namelen;
unsigned long ref_ptr;
unsigned long ref_end;
+ int search_done = 0;
/*
* it is possible that we didn't log all the parent directories
@@ -845,7 +845,10 @@ again:
* existing back reference, and we don't want to create
* dangling pointers in the directory.
*/
-conflict_again:
+
+ if (search_done)
+ goto insert;
+
ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
if (ret == 0) {
char *victim_name;
@@ -887,36 +890,22 @@ conflict_again:
inode, victim_name,
victim_name_len);
kfree(victim_name);
- btrfs_release_path(root, path);
- goto conflict_again;
}
- kfree(victim_name);
+ if (victim_name)
+ kfree(victim_name);
ptr = (unsigned long)(victim_ref + 1) + victim_name_len;
}
BUG_ON(ret);
- }
- btrfs_release_path(root, path);
- /* look for a conflicting sequence number */
- di = btrfs_lookup_dir_index_item(trans, root, path, dir->i_ino,
- btrfs_inode_ref_index(eb, ref),
- name, namelen, 0);
- if (di && !IS_ERR(di)) {
- ret = drop_one_dir_item(trans, root, path, dir, di);
- BUG_ON(ret);
- }
- btrfs_release_path(root, path);
-
-
- /* look for a conflicting name */
- di = btrfs_lookup_dir_item(trans, root, path, dir->i_ino,
- name, namelen, 0);
- if (di && !IS_ERR(di)) {
- ret = drop_one_dir_item(trans, root, path, dir, di);
- BUG_ON(ret);
+ /*
+ * NOTE: we have searched root tree and checked the
+ * coresponding ref, it does not need to check again.
+ */
+ search_done = 1;
}
btrfs_release_path(root, path);
+insert:
/* insert our name */
ret = btrfs_add_link(trans, dir, inode, name, namelen, 0,
btrfs_inode_ref_index(eb, ref));
--
1.6.5.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] btrfs: make inode ref log recovery faster
2011-02-23 7:09 ` [PATCH v2] " liubo
@ 2011-02-23 7:20 ` Li Zefan
0 siblings, 0 replies; 7+ messages in thread
From: Li Zefan @ 2011-02-23 7:20 UTC (permalink / raw)
To: liubo; +Cc: Linux Btrfs, Josef Bacik, David Sterba
> -conflict_again:
> +
> + if (search_done)
> + goto insert;
> +
> ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
> if (ret == 0) {
> char *victim_name;
> @@ -887,36 +890,22 @@ conflict_again:
> inode, victim_name,
> victim_name_len);
> kfree(victim_name);
> - btrfs_release_path(root, path);
> - goto conflict_again;
> }
> - kfree(victim_name);
> + if (victim_name)
> + kfree(victim_name);
This is still wrong..
Just remove the first kfree() and remain the second one.
> ptr = (unsigned long)(victim_ref + 1) + victim_name_len;
> }
> BUG_ON(ret);
> - }
> - btrfs_release_path(root, path);
>
> - /* look for a conflicting sequence number */
> - di = btrfs_lookup_dir_index_item(trans, root, path, dir->i_ino,
> - btrfs_inode_ref_index(eb, ref),
> - name, namelen, 0);
> - if (di && !IS_ERR(di)) {
> - ret = drop_one_dir_item(trans, root, path, dir, di);
> - BUG_ON(ret);
> - }
> - btrfs_release_path(root, path);
> -
> -
> - /* look for a conflicting name */
> - di = btrfs_lookup_dir_item(trans, root, path, dir->i_ino,
> - name, namelen, 0);
> - if (di && !IS_ERR(di)) {
> - ret = drop_one_dir_item(trans, root, path, dir, di);
> - BUG_ON(ret);
> + /*
> + * NOTE: we have searched root tree and checked the
> + * coresponding ref, it does not need to check again.
> + */
> + search_done = 1;
> }
> btrfs_release_path(root, path);
>
> +insert:
> /* insert our name */
> ret = btrfs_add_link(trans, dir, inode, name, namelen, 0,
> btrfs_inode_ref_index(eb, ref));
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-02-23 7:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-22 11:42 [PATCH] btrfs: make inode ref log recovery faster liubo
2011-02-22 14:32 ` David Sterba
2011-02-23 1:12 ` liubo
2011-02-23 1:30 ` Josef Bacik
2011-02-23 1:42 ` liubo
2011-02-23 7:09 ` [PATCH v2] " liubo
2011-02-23 7:20 ` Li Zefan
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).