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 <[email protected]> > --- > 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/ -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
