* [PATCH 0/3] Extent buffer lock cleanups
@ 2019-05-30 16:30 David Sterba
2019-05-30 16:31 ` [PATCH 1/3] btrfs: switch extent_buffer blocking_writers from atomic to int David Sterba
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: David Sterba @ 2019-05-30 16:30 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
There are 3 atomics that don't need to be, all related to writes where
the exclusivity is guaranteed by the lock.
David Sterba (3):
btrfs: switch extent_buffer blocking_writers from atomic to int
btrfs: switch extent_buffer spinning_writers from atomic to int
btrfs: switch extent_buffer write_locks from atomic to int
fs/btrfs/extent_io.c | 6 ++---
fs/btrfs/extent_io.h | 6 ++---
fs/btrfs/locking.c | 62 +++++++++++++++++++------------------------
fs/btrfs/print-tree.c | 6 ++---
4 files changed, 37 insertions(+), 43 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] btrfs: switch extent_buffer blocking_writers from atomic to int
2019-05-30 16:30 [PATCH 0/3] Extent buffer lock cleanups David Sterba
@ 2019-05-30 16:31 ` David Sterba
2019-05-31 8:29 ` Nikolay Borisov
2019-05-30 16:31 ` [PATCH 2/3] btrfs: switch extent_buffer spinning_writers " David Sterba
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2019-05-30 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
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>
---
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),
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] btrfs: switch extent_buffer spinning_writers from atomic to int
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-30 16:31 ` David Sterba
2019-05-31 9:19 ` Nikolay Borisov
2019-05-30 16:31 ` [PATCH 3/3] btrfs: switch extent_buffer write_locks " David Sterba
2019-06-07 13:31 ` [PATCH 0/3] Extent buffer lock cleanups David Sterba
3 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2019-05-30 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The spinning_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>
---
fs/btrfs/extent_io.c | 2 +-
fs/btrfs/extent_io.h | 2 +-
fs/btrfs/locking.c | 10 +++++-----
fs/btrfs/print-tree.c | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 57b6de9df7c4..71ee9e976307 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4842,7 +4842,7 @@ __alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start,
BUG_ON(len > MAX_INLINE_EXTENT_BUFFER_SIZE);
#ifdef CONFIG_BTRFS_DEBUG
- atomic_set(&eb->spinning_writers, 0);
+ eb->spinning_writers = 0;
atomic_set(&eb->spinning_readers, 0);
atomic_set(&eb->read_locks, 0);
atomic_set(&eb->write_locks, 0);
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 201da61dfc21..5616b96c365d 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -187,7 +187,7 @@ struct extent_buffer {
wait_queue_head_t read_lock_wq;
struct page *pages[INLINE_EXTENT_BUFFER_PAGES];
#ifdef CONFIG_BTRFS_DEBUG
- atomic_t spinning_writers;
+ int spinning_writers;
atomic_t spinning_readers;
atomic_t read_locks;
atomic_t write_locks;
diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
index 5feb01147e19..270667627977 100644
--- a/fs/btrfs/locking.c
+++ b/fs/btrfs/locking.c
@@ -15,19 +15,19 @@
#ifdef CONFIG_BTRFS_DEBUG
static void btrfs_assert_spinning_writers_get(struct extent_buffer *eb)
{
- WARN_ON(atomic_read(&eb->spinning_writers));
- atomic_inc(&eb->spinning_writers);
+ WARN_ON(eb->spinning_writers);
+ eb->spinning_writers++;
}
static void btrfs_assert_spinning_writers_put(struct extent_buffer *eb)
{
- WARN_ON(atomic_read(&eb->spinning_writers) != 1);
- atomic_dec(&eb->spinning_writers);
+ WARN_ON(eb->spinning_writers != 1);
+ eb->spinning_writers--;
}
static void btrfs_assert_no_spinning_writers(struct extent_buffer *eb)
{
- WARN_ON(atomic_read(&eb->spinning_writers));
+ WARN_ON(eb->spinning_writers);
}
static void btrfs_assert_spinning_readers_get(struct extent_buffer *eb)
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index 7cb4f1fbe043..c5cc435ed39a 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -157,7 +157,7 @@ static void print_eb_refs_lock(struct extent_buffer *eb)
atomic_read(&eb->read_locks),
eb->blocking_writers,
atomic_read(&eb->blocking_readers),
- atomic_read(&eb->spinning_writers),
+ eb->spinning_writers,
atomic_read(&eb->spinning_readers),
eb->lock_owner, current->pid);
#endif
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] btrfs: switch extent_buffer write_locks from atomic to int
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-30 16:31 ` [PATCH 2/3] btrfs: switch extent_buffer spinning_writers " David Sterba
@ 2019-05-30 16:31 ` David Sterba
2019-05-31 9:20 ` Nikolay Borisov
2019-06-07 13:31 ` [PATCH 0/3] Extent buffer lock cleanups David Sterba
3 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2019-05-30 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The write_locks 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>
---
fs/btrfs/extent_io.c | 2 +-
fs/btrfs/extent_io.h | 2 +-
fs/btrfs/locking.c | 6 +++---
fs/btrfs/print-tree.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 71ee9e976307..6d75d4dcf473 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4845,7 +4845,7 @@ __alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start,
eb->spinning_writers = 0;
atomic_set(&eb->spinning_readers, 0);
atomic_set(&eb->read_locks, 0);
- atomic_set(&eb->write_locks, 0);
+ eb->write_locks = 0;
#endif
return eb;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 5616b96c365d..844e595cde5b 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -190,7 +190,7 @@ struct extent_buffer {
int spinning_writers;
atomic_t spinning_readers;
atomic_t read_locks;
- atomic_t write_locks;
+ int write_locks;
struct list_head leak_list;
#endif
};
diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
index 270667627977..98fccce4208c 100644
--- a/fs/btrfs/locking.c
+++ b/fs/btrfs/locking.c
@@ -58,17 +58,17 @@ static void btrfs_assert_tree_read_locked(struct extent_buffer *eb)
static void btrfs_assert_tree_write_locks_get(struct extent_buffer *eb)
{
- atomic_inc(&eb->write_locks);
+ eb->write_locks++;
}
static void btrfs_assert_tree_write_locks_put(struct extent_buffer *eb)
{
- atomic_dec(&eb->write_locks);
+ eb->write_locks--;
}
void btrfs_assert_tree_locked(struct extent_buffer *eb)
{
- BUG_ON(!atomic_read(&eb->write_locks));
+ BUG_ON(!eb->write_locks);
}
#else
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index c5cc435ed39a..9cb50577d982 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -153,7 +153,7 @@ static void print_eb_refs_lock(struct extent_buffer *eb)
#ifdef CONFIG_BTRFS_DEBUG
btrfs_info(eb->fs_info,
"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->refs), eb->write_locks,
atomic_read(&eb->read_locks),
eb->blocking_writers,
atomic_read(&eb->blocking_readers),
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] btrfs: switch extent_buffer blocking_writers from atomic to int
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
0 siblings, 0 replies; 9+ messages in thread
From: Nikolay Borisov @ 2019-05-31 8:29 UTC (permalink / raw)
To: David Sterba, linux-btrfs
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),
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] btrfs: switch extent_buffer spinning_writers from atomic to int
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
0 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2019-05-31 9:19 UTC (permalink / raw)
To: David Sterba, linux-btrfs
On 30.05.19 г. 19:31 ч., David Sterba wrote:
> The spinning_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>
> ---
> fs/btrfs/extent_io.c | 2 +-
> fs/btrfs/extent_io.h | 2 +-
> fs/btrfs/locking.c | 10 +++++-----
> fs/btrfs/print-tree.c | 2 +-
> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 57b6de9df7c4..71ee9e976307 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4842,7 +4842,7 @@ __alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start,
> BUG_ON(len > MAX_INLINE_EXTENT_BUFFER_SIZE);
>
> #ifdef CONFIG_BTRFS_DEBUG
> - atomic_set(&eb->spinning_writers, 0);
> + eb->spinning_writers = 0;
> atomic_set(&eb->spinning_readers, 0);
> atomic_set(&eb->read_locks, 0);
> atomic_set(&eb->write_locks, 0);
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 201da61dfc21..5616b96c365d 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -187,7 +187,7 @@ struct extent_buffer {
> wait_queue_head_t read_lock_wq;
> struct page *pages[INLINE_EXTENT_BUFFER_PAGES];
> #ifdef CONFIG_BTRFS_DEBUG
> - atomic_t spinning_writers;
> + int spinning_writers;
> atomic_t spinning_readers;
> atomic_t read_locks;
> atomic_t write_locks;
> diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
> index 5feb01147e19..270667627977 100644
> --- a/fs/btrfs/locking.c
> +++ b/fs/btrfs/locking.c
> @@ -15,19 +15,19 @@
> #ifdef CONFIG_BTRFS_DEBUG
> static void btrfs_assert_spinning_writers_get(struct extent_buffer *eb)
> {
> - WARN_ON(atomic_read(&eb->spinning_writers));
> - atomic_inc(&eb->spinning_writers);
> + WARN_ON(eb->spinning_writers);
> + eb->spinning_writers++;
> }
>
> static void btrfs_assert_spinning_writers_put(struct extent_buffer *eb)
> {
> - WARN_ON(atomic_read(&eb->spinning_writers) != 1);
> - atomic_dec(&eb->spinning_writers);
> + WARN_ON(eb->spinning_writers != 1);
> + eb->spinning_writers--;
> }
>
> static void btrfs_assert_no_spinning_writers(struct extent_buffer *eb)
> {
> - WARN_ON(atomic_read(&eb->spinning_writers));
> + WARN_ON(eb->spinning_writers);
> }
IMO longterm it will be good if those debug functions contained
lockdep_assert_held_exclusive/read macros for posterity.
>
> static void btrfs_assert_spinning_readers_get(struct extent_buffer *eb)
> diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
> index 7cb4f1fbe043..c5cc435ed39a 100644
> --- a/fs/btrfs/print-tree.c
> +++ b/fs/btrfs/print-tree.c
> @@ -157,7 +157,7 @@ static void print_eb_refs_lock(struct extent_buffer *eb)
> atomic_read(&eb->read_locks),
> eb->blocking_writers,
> atomic_read(&eb->blocking_readers),
> - atomic_read(&eb->spinning_writers),
> + eb->spinning_writers,
> atomic_read(&eb->spinning_readers),
> eb->lock_owner, current->pid);
> #endif
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] btrfs: switch extent_buffer write_locks from atomic to int
2019-05-30 16:31 ` [PATCH 3/3] btrfs: switch extent_buffer write_locks " David Sterba
@ 2019-05-31 9:20 ` Nikolay Borisov
0 siblings, 0 replies; 9+ messages in thread
From: Nikolay Borisov @ 2019-05-31 9:20 UTC (permalink / raw)
To: David Sterba, linux-btrfs
On 30.05.19 г. 19:31 ч., David Sterba wrote:
> The write_locks 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>
Generally looks good, though my remark for patch2 remains.
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/extent_io.c | 2 +-
> fs/btrfs/extent_io.h | 2 +-
> fs/btrfs/locking.c | 6 +++---
> fs/btrfs/print-tree.c | 2 +-
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 71ee9e976307..6d75d4dcf473 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4845,7 +4845,7 @@ __alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start,
> eb->spinning_writers = 0;
> atomic_set(&eb->spinning_readers, 0);
> atomic_set(&eb->read_locks, 0);
> - atomic_set(&eb->write_locks, 0);
> + eb->write_locks = 0;
> #endif
>
> return eb;
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 5616b96c365d..844e595cde5b 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -190,7 +190,7 @@ struct extent_buffer {
> int spinning_writers;
> atomic_t spinning_readers;
> atomic_t read_locks;
> - atomic_t write_locks;
> + int write_locks;
> struct list_head leak_list;
> #endif
> };
> diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
> index 270667627977..98fccce4208c 100644
> --- a/fs/btrfs/locking.c
> +++ b/fs/btrfs/locking.c
> @@ -58,17 +58,17 @@ static void btrfs_assert_tree_read_locked(struct extent_buffer *eb)
>
> static void btrfs_assert_tree_write_locks_get(struct extent_buffer *eb)
> {
> - atomic_inc(&eb->write_locks);
> + eb->write_locks++;
> }
>
> static void btrfs_assert_tree_write_locks_put(struct extent_buffer *eb)
> {
> - atomic_dec(&eb->write_locks);
> + eb->write_locks--;
> }
>
> void btrfs_assert_tree_locked(struct extent_buffer *eb)
> {
> - BUG_ON(!atomic_read(&eb->write_locks));
> + BUG_ON(!eb->write_locks);
> }
>
> #else
> diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
> index c5cc435ed39a..9cb50577d982 100644
> --- a/fs/btrfs/print-tree.c
> +++ b/fs/btrfs/print-tree.c
> @@ -153,7 +153,7 @@ static void print_eb_refs_lock(struct extent_buffer *eb)
> #ifdef CONFIG_BTRFS_DEBUG
> btrfs_info(eb->fs_info,
> "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->refs), eb->write_locks,
> atomic_read(&eb->read_locks),
> eb->blocking_writers,
> atomic_read(&eb->blocking_readers),
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] btrfs: switch extent_buffer spinning_writers from atomic to int
2019-05-31 9:19 ` Nikolay Borisov
@ 2019-05-31 11:28 ` David Sterba
0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-05-31 11:28 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: David Sterba, linux-btrfs
On Fri, May 31, 2019 at 12:19:15PM +0300, Nikolay Borisov wrote:
> > {
> > - WARN_ON(atomic_read(&eb->spinning_writers));
> > - atomic_inc(&eb->spinning_writers);
> > + WARN_ON(eb->spinning_writers);
> > + eb->spinning_writers++;
> > }
> >
> > static void btrfs_assert_spinning_writers_put(struct extent_buffer *eb)
> > {
> > - WARN_ON(atomic_read(&eb->spinning_writers) != 1);
> > - atomic_dec(&eb->spinning_writers);
> > + WARN_ON(eb->spinning_writers != 1);
> > + eb->spinning_writers--;
> > }
> >
> > static void btrfs_assert_no_spinning_writers(struct extent_buffer *eb)
> > {
> > - WARN_ON(atomic_read(&eb->spinning_writers));
> > + WARN_ON(eb->spinning_writers);
> > }
>
> IMO longterm it will be good if those debug functions contained
> lockdep_assert_held_exclusive/read macros for posterity.
The functions are not public and used only inside implementation of
locks, so the chances of wrong use are low so I don't see much value
adding it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] Extent buffer lock cleanups
2019-05-30 16:30 [PATCH 0/3] Extent buffer lock cleanups David Sterba
` (2 preceding siblings ...)
2019-05-30 16:31 ` [PATCH 3/3] btrfs: switch extent_buffer write_locks " David Sterba
@ 2019-06-07 13:31 ` David Sterba
3 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-06-07 13:31 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On Thu, May 30, 2019 at 06:30:57PM +0200, David Sterba wrote:
> There are 3 atomics that don't need to be, all related to writes where
> the exclusivity is guaranteed by the lock.
>
> David Sterba (3):
> btrfs: switch extent_buffer blocking_writers from atomic to int
> btrfs: switch extent_buffer spinning_writers from atomic to int
> btrfs: switch extent_buffer write_locks from atomic to int
>
> fs/btrfs/extent_io.c | 6 ++---
> fs/btrfs/extent_io.h | 6 ++---
> fs/btrfs/locking.c | 62 +++++++++++++++++++------------------------
> fs/btrfs/print-tree.c | 6 ++---
> 4 files changed, 37 insertions(+), 43 deletions(-)
Added to misc-next.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-06-07 13:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).