From: Nikolay Borisov <nborisov@suse.com>
To: David Sterba <dsterba@suse.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 1/3] btrfs: switch extent_buffer blocking_writers from atomic to int
Date: Fri, 31 May 2019 11:29:33 +0300 [thread overview]
Message-ID: <ed094516-d79b-0981-5f1e-52002db1d874@suse.com> (raw)
In-Reply-To: <ebba72a9ef564b27fb5f652e3edcfbca3e981a10.1559233731.git.dsterba@suse.com>
On 30.05.19 г. 19:31 ч., David Sterba wrote:
> The blocking_writers is either 0 or 1 and always updated under the lock,
> so we don't need the atomic_t semantics.>
> Signed-off-by: David Sterba <dsterba@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/extent_io.c | 2 +-
> fs/btrfs/extent_io.h | 2 +-
> fs/btrfs/locking.c | 46 +++++++++++++++++++------------------------
> fs/btrfs/print-tree.c | 2 +-
> 4 files changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 2f38c10d2bfb..57b6de9df7c4 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4823,7 +4823,7 @@ __alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start,
> eb->bflags = 0;
> rwlock_init(&eb->lock);
> atomic_set(&eb->blocking_readers, 0);
> - atomic_set(&eb->blocking_writers, 0);
> + eb->blocking_writers = 0;
> eb->lock_nested = false;
> init_waitqueue_head(&eb->write_lock_wq);
> init_waitqueue_head(&eb->read_lock_wq);
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index aa18a16a6ed7..201da61dfc21 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -167,7 +167,7 @@ struct extent_buffer {
> struct rcu_head rcu_head;
> pid_t lock_owner;
>
> - atomic_t blocking_writers;
> + int blocking_writers;
> atomic_t blocking_readers;
> bool lock_nested;
> /* >= 0 if eb belongs to a log tree, -1 otherwise */
> diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
> index 2f6c3c7851ed..5feb01147e19 100644
> --- a/fs/btrfs/locking.c
> +++ b/fs/btrfs/locking.c
> @@ -111,10 +111,10 @@ void btrfs_set_lock_blocking_write(struct extent_buffer *eb)
> */
> if (eb->lock_nested && current->pid == eb->lock_owner)
> return;
> - if (atomic_read(&eb->blocking_writers) == 0) {
> + if (eb->blocking_writers == 0) {
> btrfs_assert_spinning_writers_put(eb);
> btrfs_assert_tree_locked(eb);
> - atomic_inc(&eb->blocking_writers);
> + eb->blocking_writers++;
> write_unlock(&eb->lock);
> }
> }
> @@ -148,12 +148,11 @@ void btrfs_clear_lock_blocking_write(struct extent_buffer *eb)
> */
> if (eb->lock_nested && current->pid == eb->lock_owner)
> return;
> - BUG_ON(atomic_read(&eb->blocking_writers) != 1);
> write_lock(&eb->lock);
> + BUG_ON(eb->blocking_writers != 1);
> btrfs_assert_spinning_writers_get(eb);
> - /* atomic_dec_and_test implies a barrier */
> - if (atomic_dec_and_test(&eb->blocking_writers))
> - cond_wake_up_nomb(&eb->write_lock_wq);
> + if (--eb->blocking_writers == 0)
> + cond_wake_up(&eb->write_lock_wq);
> }
>
> /*
> @@ -167,12 +166,10 @@ void btrfs_tree_read_lock(struct extent_buffer *eb)
> if (trace_btrfs_tree_read_lock_enabled())
> start_ns = ktime_get_ns();
> again:
> - BUG_ON(!atomic_read(&eb->blocking_writers) &&
> - current->pid == eb->lock_owner);
> -
> read_lock(&eb->lock);
> - if (atomic_read(&eb->blocking_writers) &&
> - current->pid == eb->lock_owner) {
> + BUG_ON(eb->blocking_writers == 0 &&
> + current->pid == eb->lock_owner);
> + if (eb->blocking_writers && current->pid == eb->lock_owner) {
> /*
> * This extent is already write-locked by our thread. We allow
> * an additional read lock to be added because it's for the same
> @@ -185,10 +182,10 @@ void btrfs_tree_read_lock(struct extent_buffer *eb)
> trace_btrfs_tree_read_lock(eb, start_ns);
> return;
> }
> - if (atomic_read(&eb->blocking_writers)) {
> + if (eb->blocking_writers) {
> read_unlock(&eb->lock);
> wait_event(eb->write_lock_wq,
> - atomic_read(&eb->blocking_writers) == 0);
> + eb->blocking_writers == 0);
> goto again;
> }
> btrfs_assert_tree_read_locks_get(eb);
> @@ -203,11 +200,11 @@ void btrfs_tree_read_lock(struct extent_buffer *eb)
> */
> int btrfs_tree_read_lock_atomic(struct extent_buffer *eb)
> {
> - if (atomic_read(&eb->blocking_writers))
> + if (eb->blocking_writers)
> return 0;
>
> read_lock(&eb->lock);
> - if (atomic_read(&eb->blocking_writers)) {
> + if (eb->blocking_writers) {
> read_unlock(&eb->lock);
> return 0;
> }
> @@ -223,13 +220,13 @@ int btrfs_tree_read_lock_atomic(struct extent_buffer *eb)
> */
> int btrfs_try_tree_read_lock(struct extent_buffer *eb)
> {
> - if (atomic_read(&eb->blocking_writers))
> + if (eb->blocking_writers)
> return 0;
>
> if (!read_trylock(&eb->lock))
> return 0;
>
> - if (atomic_read(&eb->blocking_writers)) {
> + if (eb->blocking_writers) {
> read_unlock(&eb->lock);
> return 0;
> }
> @@ -245,13 +242,11 @@ int btrfs_try_tree_read_lock(struct extent_buffer *eb)
> */
> int btrfs_try_tree_write_lock(struct extent_buffer *eb)
> {
> - if (atomic_read(&eb->blocking_writers) ||
> - atomic_read(&eb->blocking_readers))
> + if (eb->blocking_writers || atomic_read(&eb->blocking_readers))
> return 0;
>
> write_lock(&eb->lock);
> - if (atomic_read(&eb->blocking_writers) ||
> - atomic_read(&eb->blocking_readers)) {
> + if (eb->blocking_writers || atomic_read(&eb->blocking_readers)) {
> write_unlock(&eb->lock);
> return 0;
> }
> @@ -322,10 +317,9 @@ void btrfs_tree_lock(struct extent_buffer *eb)
> WARN_ON(eb->lock_owner == current->pid);
> again:
> wait_event(eb->read_lock_wq, atomic_read(&eb->blocking_readers) == 0);
> - wait_event(eb->write_lock_wq, atomic_read(&eb->blocking_writers) == 0);
> + wait_event(eb->write_lock_wq, eb->blocking_writers == 0);
> write_lock(&eb->lock);
> - if (atomic_read(&eb->blocking_readers) ||
> - atomic_read(&eb->blocking_writers)) {
> + if (atomic_read(&eb->blocking_readers) || eb->blocking_writers) {
> write_unlock(&eb->lock);
> goto again;
> }
> @@ -340,7 +334,7 @@ void btrfs_tree_lock(struct extent_buffer *eb)
> */
> void btrfs_tree_unlock(struct extent_buffer *eb)
> {
> - int blockers = atomic_read(&eb->blocking_writers);
> + int blockers = eb->blocking_writers;
>
> BUG_ON(blockers > 1);
>
> @@ -351,7 +345,7 @@ void btrfs_tree_unlock(struct extent_buffer *eb)
>
> if (blockers) {
> btrfs_assert_no_spinning_writers(eb);
> - atomic_dec(&eb->blocking_writers);
> + eb->blocking_writers--;
> /* Use the lighter barrier after atomic */
> smp_mb__after_atomic();
> cond_wake_up_nomb(&eb->write_lock_wq);
> diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
> index 1141ca5fae6a..7cb4f1fbe043 100644
> --- a/fs/btrfs/print-tree.c
> +++ b/fs/btrfs/print-tree.c
> @@ -155,7 +155,7 @@ static void print_eb_refs_lock(struct extent_buffer *eb)
> "refs %u lock (w:%d r:%d bw:%d br:%d sw:%d sr:%d) lock_owner %u current %u",
> atomic_read(&eb->refs), atomic_read(&eb->write_locks),
> atomic_read(&eb->read_locks),
> - atomic_read(&eb->blocking_writers),
> + eb->blocking_writers,
> atomic_read(&eb->blocking_readers),
> atomic_read(&eb->spinning_writers),
> atomic_read(&eb->spinning_readers),
>
next prev parent reply other threads:[~2019-05-31 8:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-30 16:30 [PATCH 0/3] Extent buffer lock cleanups David Sterba
2019-05-30 16:31 ` [PATCH 1/3] btrfs: switch extent_buffer blocking_writers from atomic to int David Sterba
2019-05-31 8:29 ` Nikolay Borisov [this message]
2019-05-30 16:31 ` [PATCH 2/3] btrfs: switch extent_buffer spinning_writers " David Sterba
2019-05-31 9:19 ` Nikolay Borisov
2019-05-31 11:28 ` David Sterba
2019-05-30 16:31 ` [PATCH 3/3] btrfs: switch extent_buffer write_locks " David Sterba
2019-05-31 9:20 ` Nikolay Borisov
2019-06-07 13:31 ` [PATCH 0/3] Extent buffer lock cleanups 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=ed094516-d79b-0981-5f1e-52002db1d874@suse.com \
--to=nborisov@suse.com \
--cc=dsterba@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