On 05/24/2011 11:56 PM, liubo wrote:
>> > The problems I hit:
>> >
>> > When an inode is dropped from cache (just via iput) and then read in
>> > again, the BTRFS_I(inode)->logged_trans goes back to zero. When this
>> > happens the logging code assumes the inode isn't in the log and hits
>> > -EEXIST if it finds inode items.
>> >
>
> ok, I just find where the problem addresses. This is because I've put
> a check between logged_trans and transaction_id, which is inclined to
> filter those that are first logged, and I'm sorry for not taking the
> 'iput' stuff into consideration. And it's easy to fix this, as we
> can just kick this check off and put another check while searching
> 'BTRFS_INODE_ITEM_KEY', since if we cannot find a inode item in a tree,
> it proves that this inode is definitely not in the tree.
>
> So I'd like to make some changes like this patch(_UNTEST_):
I've thought of this problem more and came up with a _better and more efficient_ patch.
It will always get BTRFS_I(inode)->logged_trans correct value.
But I'm still trying to test it somehow... :P
Here it is:
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 40f6f8f..d22b3bf 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1769,12 +1769,9 @@ static int btrfs_finish_ordered_io(struct inode *inode, u64 start, u64 end)
add_pending_csums(trans, inode, ordered_extent->file_offset,
&ordered_extent->list);
- ret = btrfs_ordered_update_i_size(inode, 0, ordered_extent);
- if (!ret) {
- ret = btrfs_update_inode(trans, root, inode);
- BUG_ON(ret);
- } else
- btrfs_set_inode_last_trans(trans, inode);
+ btrfs_ordered_update_i_size(inode, 0, ordered_extent);
+ ret = btrfs_update_inode(trans, root, inode);
+ BUG_ON(ret);
ret = 0;
out:
if (nolock) {
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 912397c..92fe5dd 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3032,6 +3032,37 @@ out:
return ret;
}
+static int check_logged_trans(struct btrfs_trans_handle *trans,
+ struct btrfs_root *root, struct inode *inode)
+{
+ struct btrfs_inode_item *inode_item;
+ struct btrfs_path *path;
+ int ret;
+
+ path = btrfs_alloc_path();
+ if (!path)
+ return -ENOMEM;
+
+ ret = btrfs_search_slot(trans, root,
+ &BTRFS_I(inode)->location, path, 0, 0);
+ if (ret) {
+ if (ret > 0)
+ ret = 0;
+ goto out;
+ }
+
+ btrfs_unlock_up_safe(path, 1);
+ inode_item = btrfs_item_ptr(path->nodes[0], path->slots[0],
+ struct btrfs_inode_item);
+
+ BTRFS_I(inode)->logged_trans = btrfs_inode_transid(path->nodes[0],
+ inode_item);
+out:
+ btrfs_free_path(path);
+ return ret;
+}
+
+
static int inode_in_log(struct btrfs_trans_handle *trans,
struct inode *inode)
{
@@ -3084,6 +3115,18 @@ int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
if (ret)
goto end_no_trans;
+ /*
+ * After we iput a inode and reread it from disk, logged_trans is 0.
+ * However, this inode _may_ still remain in log tree and not be
+ * committed yet.
+ * So we need to check the log tree to get logged_trans a right value.
+ */
+ if (!BTRFS_I(inode)->logged_trans && root->log_root) {
+ ret = check_logged_trans(trans, root->log_root, inode);
+ if (ret)
+ goto end_no_trans;
+ }
+
if (inode_in_log(trans, inode)) {
ret = BTRFS_NO_LOG_SYNC;
goto end_no_trans;
thanks,
liubo
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
