From: Nikolay Borisov <nborisov@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: Nikolay Borisov <nborisov@suse.com>
Subject: [PATCH] btrfs: Remove unnecessary check from join_running_log_trans
Date: Thu, 23 May 2019 14:51:26 +0300 [thread overview]
Message-ID: <20190523115126.10532-1-nborisov@suse.com> (raw)
join_running_log_trans checks btrfs_root::log_root outside of
btrfs_root::log_mutex to avoid contention on the mutex. Turns out this
check is not necessary because the two callers of join_running_log_trans
(both of which deal with removing entries from the tree-log during
unlink) explicitly check whether the respective inode has been logged in
the current transaction. If it hasn't then it won't have any items in
the tree-log and call path will return before calling
join_running_log_trans. If the check passes, however, then it's
guaranteed that btrfs_root::log_root is set because the inode is logged.
Those guarantees allows us to remove the speculative as well as the
implicity and tricky memory barrier. No functionl changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
I have evaluated if there is any performance impact (there isn't one). Here's
the script used:
for i in {1..10}; do
echo "Testun run : $i"
time ./ltp/fsstress -d /media/scratch/ -p5 -n 100000 -z -fcreat=100 -f write=100 -f fsync=70 -f unlink=80
rm -rf /media/scratch/*
echo "Executions of join_running_trans : $(trace-cmd show | wc -l)"
trace-cmd clear
done
And the result :
Unpatched (Sys) Unpatched (Real) Unpatched (JRT exec) Patched (Sys) Patched(Real) Patched (JRT exec)
161 387 153215 183 393 149153
165 392 158490 159 404 158118
140 381 147707 145 373 145676
143 394 147129 148 383 131029
206 410 157987 152 383 136134
152 376 157771 143 387 131048
140 371 153929 146 376 149885
149 376 152723 207 407 147477
164 396 157385 160 393 155272
146 373 147937 148 384 152828
stddev 19.75 12.44 4525 20.5 11.06 9722
mean 156.6 385.6 153427.3 159.1 388.3 145662
median 150.5 384 153572 150 385.5 148315
JRT exec means executions of join_running_transaction during that iteration of
the test case.
fs/btrfs/tree-log.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 6c47f6ed3e94..6c8aff105b0c 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -188,10 +188,6 @@ static int join_running_log_trans(struct btrfs_root *root)
{
int ret = -ENOENT;
- smp_mb();
- if (!root->log_root)
- return -ENOENT;
-
mutex_lock(&root->log_mutex);
if (root->log_root) {
ret = 0;
--
2.17.1
next reply other threads:[~2019-05-23 11:51 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-23 11:51 Nikolay Borisov [this message]
2019-07-12 11:24 ` [PATCH] btrfs: Remove unnecessary check from join_running_log_trans Filipe Manana
2019-08-01 13:03 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190523115126.10532-1-nborisov@suse.com \
--to=nborisov@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).