* [PATCH 0/5] More lockdep annotations
@ 2019-05-31 17:01 David Sterba
2019-05-31 17:01 ` [PATCH 1/5] btrfs: tests: add locks around add_extent_mapping David Sterba
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: David Sterba @ 2019-05-31 17:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
Lockdep annotations are better than comments about necessary locks.
David Sterba (5):
btrfs: tests: add locks around add_extent_mapping
btrfs: assert extent map tree lock in add_extent_mapping
btrfs: assert tree mod log lock in __tree_mod_log_insert
btrfs: assert delayed ref lock in btrfs_find_delayed_ref_head
btrfs: assert bio list lock in merge_rbio
fs/btrfs/ctree.c | 4 ++--
fs/btrfs/delayed-ref.c | 7 ++++---
fs/btrfs/extent_map.c | 2 ++
fs/btrfs/raid56.c | 4 ++--
fs/btrfs/tests/extent-map-tests.c | 22 ++++++++++++++++++++++
5 files changed, 32 insertions(+), 7 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/5] btrfs: tests: add locks around add_extent_mapping
2019-05-31 17:01 [PATCH 0/5] More lockdep annotations David Sterba
@ 2019-05-31 17:01 ` David Sterba
2019-05-31 17:01 ` [PATCH 2/5] btrfs: assert extent map tree lock in add_extent_mapping David Sterba
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-05-31 17:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
There are no concerns about locking during the selftests so the locks
are not necessary, but following patches will add lockdep assertions to
add_extent_mapping so this is needed in tests too.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/tests/extent-map-tests.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c
index 87aeabe9d610..4a7f796c9900 100644
--- a/fs/btrfs/tests/extent-map-tests.c
+++ b/fs/btrfs/tests/extent-map-tests.c
@@ -66,7 +66,9 @@ static int test_case_1(struct btrfs_fs_info *fs_info,
em->len = SZ_16K;
em->block_start = 0;
em->block_len = SZ_16K;
+ write_lock(&em_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
+ write_unlock(&em_tree->lock);
if (ret < 0) {
test_err("cannot add extent range [0, 16K)");
goto out;
@@ -85,7 +87,9 @@ static int test_case_1(struct btrfs_fs_info *fs_info,
em->len = SZ_4K;
em->block_start = SZ_32K; /* avoid merging */
em->block_len = SZ_4K;
+ write_lock(&em_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
+ write_unlock(&em_tree->lock);
if (ret < 0) {
test_err("cannot add extent range [16K, 20K)");
goto out;
@@ -104,7 +108,9 @@ static int test_case_1(struct btrfs_fs_info *fs_info,
em->len = len;
em->block_start = start;
em->block_len = len;
+ write_lock(&em_tree->lock);
ret = btrfs_add_extent_mapping(fs_info, em_tree, &em, em->start, em->len);
+ write_unlock(&em_tree->lock);
if (ret) {
test_err("case1 [%llu %llu]: ret %d", start, start + len, ret);
goto out;
@@ -148,7 +154,9 @@ static int test_case_2(struct btrfs_fs_info *fs_info,
em->len = SZ_1K;
em->block_start = EXTENT_MAP_INLINE;
em->block_len = (u64)-1;
+ write_lock(&em_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
+ write_unlock(&em_tree->lock);
if (ret < 0) {
test_err("cannot add extent range [0, 1K)");
goto out;
@@ -167,7 +175,9 @@ static int test_case_2(struct btrfs_fs_info *fs_info,
em->len = SZ_4K;
em->block_start = SZ_4K;
em->block_len = SZ_4K;
+ write_lock(&em_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
+ write_unlock(&em_tree->lock);
if (ret < 0) {
test_err("cannot add extent range [4K, 8K)");
goto out;
@@ -186,7 +196,9 @@ static int test_case_2(struct btrfs_fs_info *fs_info,
em->len = SZ_1K;
em->block_start = EXTENT_MAP_INLINE;
em->block_len = (u64)-1;
+ write_lock(&em_tree->lock);
ret = btrfs_add_extent_mapping(fs_info, em_tree, &em, em->start, em->len);
+ write_unlock(&em_tree->lock);
if (ret) {
test_err("case2 [0 1K]: ret %d", ret);
goto out;
@@ -225,7 +237,9 @@ static int __test_case_3(struct btrfs_fs_info *fs_info,
em->len = SZ_4K;
em->block_start = SZ_4K;
em->block_len = SZ_4K;
+ write_lock(&em_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
+ write_unlock(&em_tree->lock);
if (ret < 0) {
test_err("cannot add extent range [4K, 8K)");
goto out;
@@ -244,7 +258,9 @@ static int __test_case_3(struct btrfs_fs_info *fs_info,
em->len = SZ_16K;
em->block_start = 0;
em->block_len = SZ_16K;
+ write_lock(&em_tree->lock);
ret = btrfs_add_extent_mapping(fs_info, em_tree, &em, start, len);
+ write_unlock(&em_tree->lock);
if (ret) {
test_err("case3 [0x%llx 0x%llx): ret %d",
start, start + len, ret);
@@ -320,7 +336,9 @@ static int __test_case_4(struct btrfs_fs_info *fs_info,
em->len = SZ_8K;
em->block_start = 0;
em->block_len = SZ_8K;
+ write_lock(&em_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
+ write_unlock(&em_tree->lock);
if (ret < 0) {
test_err("cannot add extent range [0, 8K)");
goto out;
@@ -339,7 +357,9 @@ static int __test_case_4(struct btrfs_fs_info *fs_info,
em->len = 24 * SZ_1K;
em->block_start = SZ_16K; /* avoid merging */
em->block_len = 24 * SZ_1K;
+ write_lock(&em_tree->lock);
ret = add_extent_mapping(em_tree, em, 0);
+ write_unlock(&em_tree->lock);
if (ret < 0) {
test_err("cannot add extent range [8K, 32K)");
goto out;
@@ -357,7 +377,9 @@ static int __test_case_4(struct btrfs_fs_info *fs_info,
em->len = SZ_32K;
em->block_start = 0;
em->block_len = SZ_32K;
+ write_lock(&em_tree->lock);
ret = btrfs_add_extent_mapping(fs_info, em_tree, &em, start, len);
+ write_unlock(&em_tree->lock);
if (ret) {
test_err("case4 [0x%llx 0x%llx): ret %d",
start, len, ret);
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/5] btrfs: assert extent map tree lock in add_extent_mapping
2019-05-31 17:01 [PATCH 0/5] More lockdep annotations David Sterba
2019-05-31 17:01 ` [PATCH 1/5] btrfs: tests: add locks around add_extent_mapping David Sterba
@ 2019-05-31 17:01 ` David Sterba
2019-05-31 17:01 ` [PATCH 3/5] btrfs: assert tree mod log lock in __tree_mod_log_insert David Sterba
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-05-31 17:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
As add_extent_mapping is called from several functions, let's add the
lock annotation. The tree is going to be modified so it must be the
exclusive lock.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/extent_map.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 9558d79faf1e..a73af4159495 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -384,6 +384,8 @@ int add_extent_mapping(struct extent_map_tree *tree,
{
int ret = 0;
+ lockdep_assert_held_exclusive(&tree->lock);
+
ret = tree_insert(&tree->map, em);
if (ret)
goto out;
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/5] btrfs: assert tree mod log lock in __tree_mod_log_insert
2019-05-31 17:01 [PATCH 0/5] More lockdep annotations David Sterba
2019-05-31 17:01 ` [PATCH 1/5] btrfs: tests: add locks around add_extent_mapping David Sterba
2019-05-31 17:01 ` [PATCH 2/5] btrfs: assert extent map tree lock in add_extent_mapping David Sterba
@ 2019-05-31 17:01 ` David Sterba
2019-05-31 17:01 ` [PATCH 4/5] btrfs: assert delayed ref lock in btrfs_find_delayed_ref_head David Sterba
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-05-31 17:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The tree is going to be modified so it must be the exclusive lock.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/ctree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 5df76c17775a..99a585ede79d 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -376,8 +376,6 @@ void btrfs_put_tree_mod_seq(struct btrfs_fs_info *fs_info,
* The 'start address' is the logical address of the *new* root node
* for root replace operations, or the logical address of the affected
* block for all other operations.
- *
- * Note: must be called with write lock for fs_info::tree_mod_log_lock.
*/
static noinline int
__tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm)
@@ -387,6 +385,8 @@ __tree_mod_log_insert(struct btrfs_fs_info *fs_info, struct tree_mod_elem *tm)
struct rb_node *parent = NULL;
struct tree_mod_elem *cur;
+ lockdep_assert_held_exclusive(&fs_info->tree_mod_log_lock);
+
tm->seq = btrfs_inc_tree_mod_seq(fs_info);
tm_root = &fs_info->tree_mod_log;
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/5] btrfs: assert delayed ref lock in btrfs_find_delayed_ref_head
2019-05-31 17:01 [PATCH 0/5] More lockdep annotations David Sterba
` (2 preceding siblings ...)
2019-05-31 17:01 ` [PATCH 3/5] btrfs: assert tree mod log lock in __tree_mod_log_insert David Sterba
@ 2019-05-31 17:01 ` David Sterba
2019-05-31 17:01 ` [PATCH 5/5] btrfs: assert bio list lock in merge_rbio David Sterba
2019-06-05 7:34 ` [PATCH 0/5] More lockdep annotations Nikolay Borisov
5 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-05-31 17:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
Turn the comment about required lock into an assertion.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/delayed-ref.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index a73fc23e2961..a94fae897b3f 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -957,13 +957,14 @@ int btrfs_add_delayed_extent_op(struct btrfs_trans_handle *trans,
}
/*
- * this does a simple search for the head node for a given extent.
- * It must be called with the delayed ref spinlock held, and it returns
- * the head node if any where found, or NULL if not.
+ * This does a simple search for the head node for a given extent. Returns the
+ * head node if found, or NULL if not.
*/
struct btrfs_delayed_ref_head *
btrfs_find_delayed_ref_head(struct btrfs_delayed_ref_root *delayed_refs, u64 bytenr)
{
+ lockdep_assert_held(&delayed_refs->lock);
+
return find_ref_head(delayed_refs, bytenr, false);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/5] btrfs: assert bio list lock in merge_rbio
2019-05-31 17:01 [PATCH 0/5] More lockdep annotations David Sterba
` (3 preceding siblings ...)
2019-05-31 17:01 ` [PATCH 4/5] btrfs: assert delayed ref lock in btrfs_find_delayed_ref_head David Sterba
@ 2019-05-31 17:01 ` David Sterba
2019-06-05 7:34 ` [PATCH 0/5] More lockdep annotations Nikolay Borisov
5 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-05-31 17:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
Turn the comment about required lock into an assertion.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/raid56.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 67a6f7d47402..505979d867e0 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -310,12 +310,12 @@ static void steal_rbio(struct btrfs_raid_bio *src, struct btrfs_raid_bio *dest)
* merging means we take the bio_list from the victim and
* splice it into the destination. The victim should
* be discarded afterwards.
- *
- * must be called with dest->rbio_list_lock held
*/
static void merge_rbio(struct btrfs_raid_bio *dest,
struct btrfs_raid_bio *victim)
{
+ lockdep_assert_held(&dest->bio_list_lock);
+
bio_list_merge(&dest->bio_list, &victim->bio_list);
dest->bio_list_bytes += victim->bio_list_bytes;
dest->generic_bio_cnt += victim->generic_bio_cnt;
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/5] More lockdep annotations
2019-05-31 17:01 [PATCH 0/5] More lockdep annotations David Sterba
` (4 preceding siblings ...)
2019-05-31 17:01 ` [PATCH 5/5] btrfs: assert bio list lock in merge_rbio David Sterba
@ 2019-06-05 7:34 ` Nikolay Borisov
2019-06-07 13:31 ` David Sterba
5 siblings, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2019-06-05 7:34 UTC (permalink / raw)
To: David Sterba, linux-btrfs
On 31.05.19 г. 20:01 ч., David Sterba wrote:
> Lockdep annotations are better than comments about necessary locks.
>
> David Sterba (5):
> btrfs: tests: add locks around add_extent_mapping
> btrfs: assert extent map tree lock in add_extent_mapping
> btrfs: assert tree mod log lock in __tree_mod_log_insert
> btrfs: assert delayed ref lock in btrfs_find_delayed_ref_head
> btrfs: assert bio list lock in merge_rbio
>
> fs/btrfs/ctree.c | 4 ++--
> fs/btrfs/delayed-ref.c | 7 ++++---
> fs/btrfs/extent_map.c | 2 ++
> fs/btrfs/raid56.c | 4 ++--
> fs/btrfs/tests/extent-map-tests.c | 22 ++++++++++++++++++++++
> 5 files changed, 32 insertions(+), 7 deletions(-)
>
For the whole series :
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/5] More lockdep annotations
2019-06-05 7:34 ` [PATCH 0/5] More lockdep annotations Nikolay Borisov
@ 2019-06-07 13:31 ` David Sterba
0 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2019-06-07 13:31 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: David Sterba, linux-btrfs
On Wed, Jun 05, 2019 at 10:34:24AM +0300, Nikolay Borisov wrote:
>
>
> On 31.05.19 г. 20:01 ч., David Sterba wrote:
> > Lockdep annotations are better than comments about necessary locks.
> >
> > David Sterba (5):
> > btrfs: tests: add locks around add_extent_mapping
> > btrfs: assert extent map tree lock in add_extent_mapping
> > btrfs: assert tree mod log lock in __tree_mod_log_insert
> > btrfs: assert delayed ref lock in btrfs_find_delayed_ref_head
> > btrfs: assert bio list lock in merge_rbio
> >
> > fs/btrfs/ctree.c | 4 ++--
> > fs/btrfs/delayed-ref.c | 7 ++++---
> > fs/btrfs/extent_map.c | 2 ++
> > fs/btrfs/raid56.c | 4 ++--
> > fs/btrfs/tests/extent-map-tests.c | 22 ++++++++++++++++++++++
> > 5 files changed, 32 insertions(+), 7 deletions(-)
> >
>
>
> For the whole series :
>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Thanks, series moved to misc-next.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-07 13:30 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-31 17:01 [PATCH 0/5] More lockdep annotations David Sterba
2019-05-31 17:01 ` [PATCH 1/5] btrfs: tests: add locks around add_extent_mapping David Sterba
2019-05-31 17:01 ` [PATCH 2/5] btrfs: assert extent map tree lock in add_extent_mapping David Sterba
2019-05-31 17:01 ` [PATCH 3/5] btrfs: assert tree mod log lock in __tree_mod_log_insert David Sterba
2019-05-31 17:01 ` [PATCH 4/5] btrfs: assert delayed ref lock in btrfs_find_delayed_ref_head David Sterba
2019-05-31 17:01 ` [PATCH 5/5] btrfs: assert bio list lock in merge_rbio David Sterba
2019-06-05 7:34 ` [PATCH 0/5] More lockdep annotations Nikolay Borisov
2019-06-07 13:31 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.