On thu, 30 Jun 2011 16:03:21 +0800, Miao Xie wrote: > Hi, Chris > > I think the snapshot should be the image of the fs tree before it was created, > so the metadata of the snapshot should not exist in the its tree. But now, we > found the directory item and directory name index is in both the snapshot tree > and the fs tree. > > Besides that, it also makes the users feel strange. For example: > # mkfs.btrfs /dev/sda1 > # mount /dev/sda1 /mnt > # mkdir /mnt/1 > # cd /mnt/1 > # btrfs subvolume snapshot /mnt snap0 > # ll /mnt/1 > total 0 > drwxr-xr-x 1 root root 10 Jun 30 15:01 1 > ^^^ > # ll /mnt/1/snap0/ > total 0 > drwxr-xr-x 1 root root 10 Jun 30 15:01 1 > ^^^ > It is also 10, but... > # ll /mnt/1/snap0/1 > total 0 > [None]
If we do # touch /mnt/1/snap0/1/snap0/a WARN_ON_ONCE(in d_set_d_op(), fs/dcache.c:1345) will be triggered. If we insert directory item and directory name index and update the parent inode as the last step, this warning will not happen. Thanks Miao > There is nothing in the directory 1 in snap0, but btrfs told the length of > this directory is 10, it is strange. > > So I think we should insert directory item and directory name index and update > the parent inode as the last step of snapshot creation. > > Thanks > Miao > > On Fri, 17 Jun 2011 17:12:28 -0400, Chris Mason wrote: >> commit e999376f094162aa425ae749aa1df95ab928d010 >> Author: Chris Mason <chris.ma...@oracle.com> >> Date: Fri Jun 17 16:14:09 2011 -0400 >> >> Btrfs: avoid delayed metadata items during commits >> >> Snapshot creation has two phases. One is the initial snapshot setup, >> and the second is done during commit, while nobody is allowed to modify >> the root we are snapshotting. >> >> The delayed metadata insertion code can break that rule, it does a >> delayed inode update on the inode of the parent of the snapshot, >> and delayed directory item insertion. >> >> This makes sure to run the pending delayed operations before we >> record the snapshot root, which avoids corruptions. >> >> Signed-off-by: Chris Mason <chris.ma...@oracle.com> >> >> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c >> index fc515b7..f1cbd02 100644 >> --- a/fs/btrfs/delayed-inode.c >> +++ b/fs/btrfs/delayed-inode.c >> @@ -1237,6 +1237,13 @@ again: >> return 0; >> } >> >> +void btrfs_assert_delayed_root_empty(struct btrfs_root *root) >> +{ >> + struct btrfs_delayed_root *delayed_root; >> + delayed_root = btrfs_get_delayed_root(root); >> + WARN_ON(btrfs_first_delayed_node(delayed_root)); >> +} >> + >> void btrfs_balance_delayed_items(struct btrfs_root *root) >> { >> struct btrfs_delayed_root *delayed_root; >> diff --git a/fs/btrfs/delayed-inode.h b/fs/btrfs/delayed-inode.h >> index cb79b67..d1a6a29 100644 >> --- a/fs/btrfs/delayed-inode.h >> +++ b/fs/btrfs/delayed-inode.h >> @@ -137,4 +137,8 @@ int btrfs_readdir_delayed_dir_index(struct file *filp, >> void *dirent, >> /* for init */ >> int __init btrfs_delayed_inode_init(void); >> void btrfs_delayed_inode_exit(void); >> + >> +/* for debugging */ >> +void btrfs_assert_delayed_root_empty(struct btrfs_root *root); >> + >> #endif >> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c >> index c073d85..51dcec8 100644 >> --- a/fs/btrfs/transaction.c >> +++ b/fs/btrfs/transaction.c >> @@ -957,6 +957,15 @@ static noinline int create_pending_snapshot(struct >> btrfs_trans_handle *trans, >> ret = btrfs_update_inode(trans, parent_root, parent_inode); >> BUG_ON(ret); >> >> + /* >> + * pull in the delayed directory update >> + * and the delayed inode item >> + * otherwise we corrupt the FS during >> + * snapshot >> + */ >> + ret = btrfs_run_delayed_items(trans, root); >> + BUG_ON(ret); >> + >> record_root_in_trans(trans, root); >> btrfs_set_root_last_snapshot(&root->root_item, trans->transid); >> memcpy(new_root_item, &root->root_item, sizeof(*new_root_item)); >> @@ -1018,14 +1027,6 @@ static noinline int create_pending_snapshots(struct >> btrfs_trans_handle *trans, >> int ret; >> >> list_for_each_entry(pending, head, list) { >> - /* >> - * We must deal with the delayed items before creating >> - * snapshots, or we will create a snapthot with inconsistent >> - * information. >> - */ >> - ret = btrfs_run_delayed_items(trans, fs_info->fs_root); >> - BUG_ON(ret); >> - >> ret = create_pending_snapshot(trans, fs_info, pending); >> BUG_ON(ret); >> } >> @@ -1319,15 +1320,21 @@ int btrfs_commit_transaction(struct >> btrfs_trans_handle *trans, >> */ >> mutex_lock(&root->fs_info->reloc_mutex); >> >> - ret = create_pending_snapshots(trans, root->fs_info); >> + ret = btrfs_run_delayed_items(trans, root); >> BUG_ON(ret); >> >> - ret = btrfs_run_delayed_items(trans, root); >> + ret = create_pending_snapshots(trans, root->fs_info); >> BUG_ON(ret); >> >> ret = btrfs_run_delayed_refs(trans, root, (unsigned long)-1); >> BUG_ON(ret); >> >> + /* >> + * make sure none of the code above managed to slip in a >> + * delayed item >> + */ >> + btrfs_assert_delayed_root_empty(root); >> + >> WARN_ON(cur_trans != trans->transaction); >> >> btrfs_scrub_pause(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 > -- 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