On Fri, Sep 14, 2012 at 08:45:16AM -0400, Josef Bacik wrote:

On Fri, Sep 14, 2012 at 02:58:07AM -0600, Liu Bo wrote:

quoted

nocow_only is now an obsolete argument.

Why is it obsolete, it looks like it's being used to me?  Thanks,

It's always 0 so the removed code never executes.

I'm tracking where it started, pretty long ago:

commit 7ea394f1192bee1af67ea4762c88ef4b7b0487a8
Author: Yan Zheng [off-list ref]
Date:   Tue Aug 5 13:05:02 2008 -0400

without any explanations what it fixes, so I'd be rather cautious here.

I found this hunk from

commit 24bbcf0442ee04660a5a030efdbb6d03f1c275cb
Author: Yan, Zheng [off-list ref]
Date:   Thu Nov 12 09:36:34 2009 +0000
Subject: Btrfs: Add delayed iput

does something suspicious:

@@ -991,11 +994,11 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
                mutex_unlock(&root->fs_info->trans_mutex);

                if (flush_on_commit) {
-                       btrfs_start_delalloc_inodes(root);
-                       ret = btrfs_wait_ordered_extents(root, 0);
+                       btrfs_start_delalloc_inodes(root, 1);
+                       ret = btrfs_wait_ordered_extents(root, 0, 1);
                        BUG_ON(ret);
                } else if (snap_pending) {
-                       ret = btrfs_wait_ordered_extents(root, 1);
+                       ret = btrfs_wait_ordered_extents(root, 0, 1);
                        BUG_ON(ret);
                }
---

The purpose was to add the dealy_iput argument to the end of the list,
but why does it change the nocow from 1 -> 0 in the if(snap_pending) branch?

The code was last touched by Sage in commit

commit 0bdb1db297ab36865a63ee722d35ff0a1f0ae522
Author: Sage Weil [off-list ref]
Date:   Fri Feb 19 14:13:50 2010 -0800
Subject: Btrfs: flush data on snapshot creation

    Flush any delalloc extents when we create a snapshot, so that recently
    written file data is always included in the snapshot.

    A later commit will add the ability to snapshot without the flush, but
    most people expect flushing.

    Signed-off-by: Sage Weil [off-list ref]
    Signed-off-by: Chris Mason [off-list ref]

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 2a36e23..2d654c1 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c

@@ -997,13 +997,10 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,

                mutex_unlock(&root->fs_info->trans_mutex);

-               if (flush_on_commit) {
+               if (flush_on_commit || snap_pending) {
                        btrfs_start_delalloc_inodes(root, 1);
                        ret = btrfs_wait_ordered_extents(root, 0, 1);
                        BUG_ON(ret);
-               } else if (snap_pending) {
-                       ret = btrfs_wait_ordered_extents(root, 0, 1);
-                       BUG_ON(ret);
                }
---

the calls to btrfs_wait_ordered_extents were same and thus mergeable.