btrfs: avoid unnecessary log mutex contention when syncing log

When syncing the log we acquire the root's log mutex just to update the
root's last_log_commit. This is unnecessary because:

1) At this point there can only be one task updating this value, which is
   the task committing the current log transaction. Any task that enters
   btrfs_sync_log() has to wait for the previous log transaction to commit
   and wait for the current log transaction to commit if someone else
   already started it (in this case it never reaches to the point of
   updating last_log_commit, as that is done by the committing task);

2) All readers of the root's last_log_commit don't acquire the root's
   log mutex. This is to avoid blocking the readers, potentially for too
   long and because getting a stale value of last_log_commit does not
   cause any functional problem, in the worst case getting a stale value
   results in logging an inode unnecessarily. Plus it's actually very
   rare to get a stale value that results in unnecessarily logging the
   inode.

So in order to avoid unnecessary contention on the root's log mutex,
which is used for several different purposes, like starting/joining a
log transaction and starting writeback of a log transaction, stop
acquiring the log mutex for updating the root's last_log_commit.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index e6430ac9bbe8511059168f790aa8af1ca4ca1304..a7ce23b9d2ee9032b8a2418705e714f20056f02e 100644 (file)
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3328,10 +3328,16 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
                goto out_wake_log_root;
        }
 
-       mutex_lock(&root->log_mutex);
-       if (root->last_log_commit < log_transid)
-               root->last_log_commit = log_transid;
-       mutex_unlock(&root->log_mutex);
+       /*
+        * We know there can only be one task here, since we have not yet set
+        * root->log_commit[index1] to 0 and any task attempting to sync the
+        * log must wait for the previous log transaction to commit if it's
+        * still in progress or wait for the current log transaction commit if
+        * someone else already started it. We use <= and not < because the
+        * first log transaction has an ID of 0.
+        */
+       ASSERT(root->last_log_commit <= log_transid);
+       root->last_log_commit = log_transid;
 
 out_wake_log_root:
        mutex_lock(&log_root_tree->log_mutex);