* [PATCH 01/11] btrfs: use struct_size to calculate size of raid hash table
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:38 ` Johannes Thumshirn
2020-02-21 16:31 ` [PATCH 02/11] btrfs: move mapping of block for discard to its caller David Sterba
` (9 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The struct_size macro does the same calculation and is safe regarding
overflows. Though we're not expecting them to happen, use the helper for
clarity.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/raid56.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 406b1efd3ba5..c870ef70f817 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -206,7 +206,6 @@ int btrfs_alloc_stripe_hash_table(struct btrfs_fs_info *info)
struct btrfs_stripe_hash *h;
int num_entries = 1 << BTRFS_STRIPE_HASH_TABLE_BITS;
int i;
- int table_size;
if (info->stripe_hash_table)
return 0;
@@ -218,8 +217,7 @@ int btrfs_alloc_stripe_hash_table(struct btrfs_fs_info *info)
* Try harder to allocate and fallback to vmalloc to lower the chance
* of a failing mount.
*/
- table_size = sizeof(*table) + sizeof(*h) * num_entries;
- table = kvzalloc(table_size, GFP_KERNEL);
+ table = kvzalloc(struct_size(table, table, num_entries), GFP_KERNEL);
if (!table)
return -ENOMEM;
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 02/11] btrfs: move mapping of block for discard to its caller
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
2020-02-21 16:31 ` [PATCH 01/11] btrfs: use struct_size to calculate size of raid hash table David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:40 ` Johannes Thumshirn
2020-02-24 3:31 ` Anand Jain
2020-02-21 16:31 ` [PATCH 03/11] btrfs: open code trivial helper btrfs_header_fsid David Sterba
` (8 subsequent siblings)
10 siblings, 2 replies; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
There's a simple forwarded call based on the operation that would better
fit the caller btrfs_map_block that's until now a trivial wrapper.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/volumes.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d8a88866aaa3..997f2c70cb6c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5953,10 +5953,7 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
struct btrfs_io_geometry geom;
ASSERT(bbio_ret);
-
- if (op == BTRFS_MAP_DISCARD)
- return __btrfs_map_block_for_discard(fs_info, logical,
- length, bbio_ret);
+ ASSERT(op != BTRFS_MAP_DISCARD);
ret = btrfs_get_io_geometry(fs_info, op, logical, *length, &geom);
if (ret < 0)
@@ -6186,6 +6183,10 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
u64 logical, u64 *length,
struct btrfs_bio **bbio_ret, int mirror_num)
{
+ if (op == BTRFS_MAP_DISCARD)
+ return __btrfs_map_block_for_discard(fs_info, logical,
+ length, bbio_ret);
+
return __btrfs_map_block(fs_info, op, logical, length, bbio_ret,
mirror_num, 0);
}
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 03/11] btrfs: open code trivial helper btrfs_header_fsid
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
2020-02-21 16:31 ` [PATCH 01/11] btrfs: use struct_size to calculate size of raid hash table David Sterba
2020-02-21 16:31 ` [PATCH 02/11] btrfs: move mapping of block for discard to its caller David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:41 ` Johannes Thumshirn
2020-02-21 16:31 ` [PATCH 04/11] btrfs: open code trivial helper btrfs_header_chunk_tree_uuid David Sterba
` (7 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The helper btrfs_header_fsid follows naming convention of other struct
accessors but does something compeletly different. As the offsetof
calculation is clear in the context of extent buffer operations we can
remove it.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/ctree.h | 5 -----
fs/btrfs/disk-io.c | 6 ++++--
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index bb237d577725..6f4272006029 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1976,11 +1976,6 @@ static inline void btrfs_set_header_backref_rev(struct extent_buffer *eb,
btrfs_set_header_flags(eb, flags);
}
-static inline unsigned long btrfs_header_fsid(void)
-{
- return offsetof(struct btrfs_header, fsid);
-}
-
static inline unsigned long btrfs_header_chunk_tree_uuid(const struct extent_buffer *eb)
{
return offsetof(struct btrfs_header, chunk_tree_uuid);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 756bf2ab64cd..63d009816264 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -541,7 +541,8 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
return -EUCLEAN;
ASSERT(memcmp_extent_buffer(eb, fs_info->fs_devices->metadata_uuid,
- btrfs_header_fsid(), BTRFS_FSID_SIZE) == 0);
+ offsetof(struct btrfs_header, fsid),
+ BTRFS_FSID_SIZE) == 0);
if (csum_tree_block(eb, result))
return -EINVAL;
@@ -571,7 +572,8 @@ static int check_tree_block_fsid(struct extent_buffer *eb)
u8 fsid[BTRFS_FSID_SIZE];
int ret = 1;
- read_extent_buffer(eb, fsid, btrfs_header_fsid(), BTRFS_FSID_SIZE);
+ read_extent_buffer(eb, fsid, offsetof(struct btrfs_header, fsid),
+ BTRFS_FSID_SIZE);
while (fs_devices) {
u8 *metadata_uuid;
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 04/11] btrfs: open code trivial helper btrfs_header_chunk_tree_uuid
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (2 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 03/11] btrfs: open code trivial helper btrfs_header_fsid David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:42 ` Johannes Thumshirn
2020-02-21 16:31 ` [PATCH 05/11] btrfs: simplify parameters of btrfs_set_disk_extent_flags David Sterba
` (6 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The helper btrfs_header_chunk_tree_uuid follows naming convention of
other struct accessors but does something compeletly different. As the
offsetof calculation is clear in the context of extent buffer operations
we can remove it.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/ctree.h | 5 -----
fs/btrfs/disk-io.c | 3 ++-
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 6f4272006029..d7e54cbc8dce 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1976,11 +1976,6 @@ static inline void btrfs_set_header_backref_rev(struct extent_buffer *eb,
btrfs_set_header_flags(eb, flags);
}
-static inline unsigned long btrfs_header_chunk_tree_uuid(const struct extent_buffer *eb)
-{
- return offsetof(struct btrfs_header, chunk_tree_uuid);
-}
-
static inline int btrfs_is_leaf(const struct extent_buffer *eb)
{
return btrfs_header_level(eb) == 0;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 63d009816264..77e1b66ebcfb 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3093,7 +3093,8 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
chunk_root->commit_root = btrfs_root_node(chunk_root);
read_extent_buffer(chunk_root->node, fs_info->chunk_tree_uuid,
- btrfs_header_chunk_tree_uuid(chunk_root->node), BTRFS_UUID_SIZE);
+ offsetof(struct btrfs_header, chunk_tree_uuid),
+ BTRFS_UUID_SIZE);
ret = btrfs_read_chunk_tree(fs_info);
if (ret) {
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 05/11] btrfs: simplify parameters of btrfs_set_disk_extent_flags
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (3 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 04/11] btrfs: open code trivial helper btrfs_header_chunk_tree_uuid David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:45 ` Johannes Thumshirn
2020-02-24 3:43 ` Anand Jain
2020-02-21 16:31 ` [PATCH 06/11] btrfs: adjust message level for unrecognized mount option David Sterba
` (5 subsequent siblings)
10 siblings, 2 replies; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
All callers pass extent buffer start and length so the extent buffer
itself should work fine.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/ctree.c | 4 +---
fs/btrfs/ctree.h | 2 +-
fs/btrfs/extent-tree.c | 7 +++----
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index b62721ac5ee8..f948435e87df 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -925,9 +925,7 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans,
if (new_flags != 0) {
int level = btrfs_header_level(buf);
- ret = btrfs_set_disk_extent_flags(trans,
- buf->start,
- buf->len,
+ ret = btrfs_set_disk_extent_flags(trans, buf,
new_flags, level, 0);
if (ret)
return ret;
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d7e54cbc8dce..2910ac257a5d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2485,7 +2485,7 @@ int btrfs_inc_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
int btrfs_dec_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root,
struct extent_buffer *buf, int full_backref);
int btrfs_set_disk_extent_flags(struct btrfs_trans_handle *trans,
- u64 bytenr, u64 num_bytes, u64 flags,
+ struct extent_buffer *eb, u64 flags,
int level, int is_data);
int btrfs_free_extent(struct btrfs_trans_handle *trans, struct btrfs_ref *ref);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 7eef91d6c2b6..7affa6342688 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2231,7 +2231,7 @@ int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
}
int btrfs_set_disk_extent_flags(struct btrfs_trans_handle *trans,
- u64 bytenr, u64 num_bytes, u64 flags,
+ struct extent_buffer *eb, u64 flags,
int level, int is_data)
{
struct btrfs_delayed_extent_op *extent_op;
@@ -2247,7 +2247,7 @@ int btrfs_set_disk_extent_flags(struct btrfs_trans_handle *trans,
extent_op->is_data = is_data ? true : false;
extent_op->level = level;
- ret = btrfs_add_delayed_extent_op(trans, bytenr, num_bytes, extent_op);
+ ret = btrfs_add_delayed_extent_op(trans, eb->start, eb->len, extent_op);
if (ret)
btrfs_free_delayed_extent_op(extent_op);
return ret;
@@ -4741,8 +4741,7 @@ static noinline int walk_down_proc(struct btrfs_trans_handle *trans,
BUG_ON(ret); /* -ENOMEM */
ret = btrfs_dec_ref(trans, root, eb, 0);
BUG_ON(ret); /* -ENOMEM */
- ret = btrfs_set_disk_extent_flags(trans, eb->start,
- eb->len, flag,
+ ret = btrfs_set_disk_extent_flags(trans, eb, flag,
btrfs_header_level(eb), 0);
BUG_ON(ret); /* -ENOMEM */
wc->flags[level] |= flag;
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 06/11] btrfs: adjust message level for unrecognized mount option
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (4 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 05/11] btrfs: simplify parameters of btrfs_set_disk_extent_flags David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:46 ` Johannes Thumshirn
2020-02-24 3:44 ` Anand Jain
2020-02-21 16:31 ` [PATCH 07/11] btrfs: raid56: simplify sort_parity_stripes David Sterba
` (4 subsequent siblings)
10 siblings, 2 replies; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
An unrecognized option is a failure that should get user/administrator
attention, the info level is often below what gets logged, so make it
error.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 5c16b4bcde9b..e6784cd3f179 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -873,7 +873,7 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
break;
#endif
case Opt_err:
- btrfs_info(info, "unrecognized mount option '%s'", p);
+ btrfs_err(info, "unrecognized mount option '%s'", p);
ret = -EINVAL;
goto out;
default:
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 07/11] btrfs: raid56: simplify sort_parity_stripes
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (5 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 06/11] btrfs: adjust message level for unrecognized mount option David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:47 ` Johannes Thumshirn
2020-02-21 16:31 ` [PATCH 08/11] btrfs: replace u_long type cast with unsigned long David Sterba
` (3 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
Remove trivial comprator and open coded swap of two values.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/volumes.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 997f2c70cb6c..945b89e2104f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5428,31 +5428,19 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
return preferred_mirror;
}
-static inline int parity_smaller(u64 a, u64 b)
-{
- return a > b;
-}
-
/* Bubble-sort the stripe set to put the parity/syndrome stripes last */
static void sort_parity_stripes(struct btrfs_bio *bbio, int num_stripes)
{
- struct btrfs_bio_stripe s;
int i;
- u64 l;
int again = 1;
while (again) {
again = 0;
for (i = 0; i < num_stripes - 1; i++) {
- if (parity_smaller(bbio->raid_map[i],
- bbio->raid_map[i+1])) {
- s = bbio->stripes[i];
- l = bbio->raid_map[i];
- bbio->stripes[i] = bbio->stripes[i+1];
- bbio->raid_map[i] = bbio->raid_map[i+1];
- bbio->stripes[i+1] = s;
- bbio->raid_map[i+1] = l;
-
+ /* Swap if parity is on a smaller index */
+ if (bbio->raid_map[i] > bbio->raid_map[i + 1]) {
+ swap(bbio->stripes[i], bbio->stripes[i + 1]);
+ swap(bbio->raid_map[i], bbio->raid_map[i + 1]);
again = 1;
}
}
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 08/11] btrfs: replace u_long type cast with unsigned long
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (6 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 07/11] btrfs: raid56: simplify sort_parity_stripes David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:48 ` Johannes Thumshirn
2020-02-24 20:09 ` Christoph Hellwig
2020-02-21 16:31 ` [PATCH 09/11] btrfs: adjust delayed refs message level David Sterba
` (2 subsequent siblings)
10 siblings, 2 replies; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
We don't use the u_XX types anywhere, though they're defined.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/volumes.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 945b89e2104f..fe1f609c736b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6269,8 +6269,8 @@ static void submit_stripe_bio(struct btrfs_bio *bbio, struct bio *bio,
btrfs_debug_in_rcu(fs_info,
"btrfs_map_bio: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u",
bio_op(bio), bio->bi_opf, (u64)bio->bi_iter.bi_sector,
- (u_long)dev->bdev->bd_dev, rcu_str_deref(dev->name), dev->devid,
- bio->bi_iter.bi_size);
+ (unsigned long)dev->bdev->bd_dev, rcu_str_deref(dev->name),
+ dev->devid, bio->bi_iter.bi_size);
bio_set_dev(bio, dev->bdev);
btrfs_bio_counter_inc_noblocked(fs_info);
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 08/11] btrfs: replace u_long type cast with unsigned long
2020-02-21 16:31 ` [PATCH 08/11] btrfs: replace u_long type cast with unsigned long David Sterba
@ 2020-02-22 8:48 ` Johannes Thumshirn
2020-02-24 20:09 ` Christoph Hellwig
1 sibling, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2020-02-22 8:48 UTC (permalink / raw)
To: David Sterba, linux-btrfs@vger.kernel.org
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 08/11] btrfs: replace u_long type cast with unsigned long
2020-02-21 16:31 ` [PATCH 08/11] btrfs: replace u_long type cast with unsigned long David Sterba
2020-02-22 8:48 ` Johannes Thumshirn
@ 2020-02-24 20:09 ` Christoph Hellwig
1 sibling, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2020-02-24 20:09 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On Fri, Feb 21, 2020 at 05:31:17PM +0100, David Sterba wrote:
> We don't use the u_XX types anywhere, though they're defined.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/volumes.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 945b89e2104f..fe1f609c736b 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6269,8 +6269,8 @@ static void submit_stripe_bio(struct btrfs_bio *bbio, struct bio *bio,
> btrfs_debug_in_rcu(fs_info,
> "btrfs_map_bio: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u",
> bio_op(bio), bio->bi_opf, (u64)bio->bi_iter.bi_sector,
> - (u_long)dev->bdev->bd_dev, rcu_str_deref(dev->name), dev->devid,
> - bio->bi_iter.bi_size);
> + (unsigned long)dev->bdev->bd_dev, rcu_str_deref(dev->name),
> + dev->devid, bio->bi_iter.bi_size);
Mostly we just print major and minor separately, which would remove the
cast entirely.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 09/11] btrfs: adjust delayed refs message level
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (7 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 08/11] btrfs: replace u_long type cast with unsigned long David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:48 ` Johannes Thumshirn
2020-02-24 3:46 ` Anand Jain
2020-02-21 16:31 ` [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction David Sterba
2020-02-21 16:31 ` [PATCH 11/11] btrfs: reduce pointer intdirections in btree_readpage_end_io_hook David Sterba
10 siblings, 2 replies; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The message seems to be for debugging and has little value for users.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/disk-io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 77e1b66ebcfb..ab8d975f071c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4280,7 +4280,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
spin_lock(&delayed_refs->lock);
if (atomic_read(&delayed_refs->num_entries) == 0) {
spin_unlock(&delayed_refs->lock);
- btrfs_info(fs_info, "delayed_refs has NO entry");
+ btrfs_debug(fs_info, "delayed_refs has NO entry");
return ret;
}
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (8 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 09/11] btrfs: adjust delayed refs message level David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:50 ` Johannes Thumshirn
` (3 more replies)
2020-02-21 16:31 ` [PATCH 11/11] btrfs: reduce pointer intdirections in btree_readpage_end_io_hook David Sterba
10 siblings, 4 replies; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The tree_log_mutex and reloc_mutex locks are properly nested so we can
simplify error handling and add labels for them. This reduces line count
of the function.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/transaction.c | 57 +++++++++++++++---------------------------
1 file changed, 20 insertions(+), 37 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index fdfdfc426539..3610b6fec627 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -2194,10 +2194,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
* core function of the snapshot creation.
*/
ret = create_pending_snapshots(trans);
- if (ret) {
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_reloc;
/*
* We insert the dir indexes of the snapshots and update the inode
@@ -2210,16 +2208,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
* the nodes and leaves.
*/
ret = btrfs_run_delayed_items(trans);
- if (ret) {
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_reloc;
ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
- if (ret) {
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_reloc;
/*
* make sure none of the code above managed to slip in a
@@ -2245,11 +2239,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
mutex_lock(&fs_info->tree_log_mutex);
ret = commit_fs_roots(trans);
- if (ret) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_reloc;
/*
* Since the transaction is done, we can apply the pending changes
@@ -2267,29 +2258,20 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
* new delayed refs. Must handle them or qgroup can be wrong.
*/
ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
- if (ret) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_tree_log;
/*
* Since fs roots are all committed, we can get a quite accurate
* new_roots. So let's do quota accounting.
*/
ret = btrfs_qgroup_account_extents(trans);
- if (ret < 0) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret < 0)
+ goto unlock_tree_log;
ret = commit_cowonly_roots(trans);
- if (ret) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_tree_log;
/*
* The tasks which save the space cache and inode cache may also
@@ -2297,9 +2279,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
*/
if (TRANS_ABORTED(cur_trans)) {
ret = cur_trans->aborted;
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
+ goto unlock_tree_log;
}
btrfs_prepare_extent_commit(fs_info);
@@ -2346,8 +2326,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
if (ret) {
btrfs_handle_fs_error(fs_info, ret,
"Error while writing out transaction");
- mutex_unlock(&fs_info->tree_log_mutex);
- goto scrub_continue;
+ goto unlock_tree_log;
}
ret = write_all_supers(fs_info, 0);
@@ -2394,6 +2373,10 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
return ret;
+unlock_tree_log:
+ mutex_unlock(&fs_info->tree_log_mutex);
+unlock_reloc:
+ mutex_unlock(&fs_info->reloc_mutex);
scrub_continue:
btrfs_scrub_continue(fs_info);
cleanup_transaction:
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction
2020-02-21 16:31 ` [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction David Sterba
@ 2020-02-22 8:50 ` Johannes Thumshirn
2020-02-24 3:56 ` Anand Jain
` (2 subsequent siblings)
3 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2020-02-22 8:50 UTC (permalink / raw)
To: David Sterba, linux-btrfs@vger.kernel.org
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction
2020-02-21 16:31 ` [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction David Sterba
2020-02-22 8:50 ` Johannes Thumshirn
@ 2020-02-24 3:56 ` Anand Jain
2020-02-24 15:02 ` David Sterba
2020-02-24 15:13 ` [PATCH v2] " David Sterba
3 siblings, 0 replies; 34+ messages in thread
From: Anand Jain @ 2020-02-24 3:56 UTC (permalink / raw)
To: David Sterba, linux-btrfs
On 2/22/20 12:31 AM, David Sterba wrote:
> The tree_log_mutex and reloc_mutex locks are properly nested so we can
> simplify error handling and add labels for them. This reduces line count
> of the function.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/transaction.c | 57 +++++++++++++++---------------------------
> 1 file changed, 20 insertions(+), 37 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index fdfdfc426539..3610b6fec627 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -2194,10 +2194,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> * core function of the snapshot creation.
> */
> ret = create_pending_snapshots(trans);
> - if (ret) {
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> - }
> + if (ret)
> + goto unlock_reloc;
>
> /*
> * We insert the dir indexes of the snapshots and update the inode
> @@ -2210,16 +2208,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> * the nodes and leaves.
> */
> ret = btrfs_run_delayed_items(trans);
> - if (ret) {
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> - }
> + if (ret)
> + goto unlock_reloc;
>
> ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
> - if (ret) {
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> - }
> + if (ret)
> + goto unlock_reloc;
>
> /*
> * make sure none of the code above managed to slip in a
> @@ -2245,11 +2239,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> mutex_lock(&fs_info->tree_log_mutex);
>
> ret = commit_fs_roots(trans);
> - if (ret) {
> - mutex_unlock(&fs_info->tree_log_mutex);
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> - }
> + if (ret)
> + goto unlock_reloc;
goto unlock_tree_log;
Otherwise looks good. When fixed this you may add.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Thanks, Anand
> /*
> * Since the transaction is done, we can apply the pending changes
> @@ -2267,29 +2258,20 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> * new delayed refs. Must handle them or qgroup can be wrong.
> */
> ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
> - if (ret) {
> - mutex_unlock(&fs_info->tree_log_mutex);
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> - }
> + if (ret)
> + goto unlock_tree_log;
>
> /*
> * Since fs roots are all committed, we can get a quite accurate
> * new_roots. So let's do quota accounting.
> */
> ret = btrfs_qgroup_account_extents(trans);
> - if (ret < 0) {
> - mutex_unlock(&fs_info->tree_log_mutex);
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> - }
> + if (ret < 0)
> + goto unlock_tree_log;
>
> ret = commit_cowonly_roots(trans);
> - if (ret) {
> - mutex_unlock(&fs_info->tree_log_mutex);
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> - }
> + if (ret)
> + goto unlock_tree_log;
>
> /*
> * The tasks which save the space cache and inode cache may also
> @@ -2297,9 +2279,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> */
> if (TRANS_ABORTED(cur_trans)) {
> ret = cur_trans->aborted;
> - mutex_unlock(&fs_info->tree_log_mutex);
> - mutex_unlock(&fs_info->reloc_mutex);
> - goto scrub_continue;
> + goto unlock_tree_log;
> }
>
> btrfs_prepare_extent_commit(fs_info);
> @@ -2346,8 +2326,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> if (ret) {
> btrfs_handle_fs_error(fs_info, ret,
> "Error while writing out transaction");
> - mutex_unlock(&fs_info->tree_log_mutex);
> - goto scrub_continue;
> + goto unlock_tree_log;
> }
>
> ret = write_all_supers(fs_info, 0);
> @@ -2394,6 +2373,10 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
>
> return ret;
>
> +unlock_tree_log:
> + mutex_unlock(&fs_info->tree_log_mutex);
> +unlock_reloc:
> + mutex_unlock(&fs_info->reloc_mutex);
> scrub_continue:
> btrfs_scrub_continue(fs_info);
> cleanup_transaction:
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction
2020-02-21 16:31 ` [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction David Sterba
2020-02-22 8:50 ` Johannes Thumshirn
2020-02-24 3:56 ` Anand Jain
@ 2020-02-24 15:02 ` David Sterba
2020-02-24 15:13 ` [PATCH v2] " David Sterba
3 siblings, 0 replies; 34+ messages in thread
From: David Sterba @ 2020-02-24 15:02 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs
On Fri, Feb 21, 2020 at 05:31:22PM +0100, David Sterba wrote:
> btrfs_prepare_extent_commit(fs_info);
> @@ -2346,8 +2326,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
> if (ret) {
> btrfs_handle_fs_error(fs_info, ret,
> "Error while writing out transaction");
> - mutex_unlock(&fs_info->tree_log_mutex);
> - goto scrub_continue;
> + goto unlock_tree_log;
Hm and this one is also wrong, in other cases that jump to
unlock_tree_log the unlocking order is tree_log_mutex/reloc_mutex, while
a few lines before there is unlock of reloc_mutex (while tree_log_mutex
is still held). This means the unlocking order is reversed compared to
the other cases and we can't jump to the label as this would lead to
double unlock of reloc_mutex.
So the above hunk must stay as is, with a comment.
> }
>
> ret = write_all_supers(fs_info, 0);
> @@ -2394,6 +2373,10 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
>
> return ret;
>
> +unlock_tree_log:
> + mutex_unlock(&fs_info->tree_log_mutex);
> +unlock_reloc:
> + mutex_unlock(&fs_info->reloc_mutex);
> scrub_continue:
> btrfs_scrub_continue(fs_info);
> cleanup_transaction:
> --
> 2.25.0
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH v2] btrfs: merge unlocking to common exit block in btrfs_commit_transaction
2020-02-21 16:31 ` [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction David Sterba
` (2 preceding siblings ...)
2020-02-24 15:02 ` David Sterba
@ 2020-02-24 15:13 ` David Sterba
2020-02-25 4:00 ` Anand Jain
3 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2020-02-24 15:13 UTC (permalink / raw)
To: linux-btrfs; +Cc: Johannes.Thumshirn, anand.jain, David Sterba
The tree_log_mutex and reloc_mutex locks are properly nested so we can
simplify error handling and add labels for them. This reduces line count
of the function.
Signed-off-by: David Sterba <dsterba@suse.com>
---
v2:
- fixed label after commit_fs_roots, to point to unlock_tree_log
- added comment after btrfs_handle_fs_error and keep the locks as is
fs/btrfs/transaction.c | 58 +++++++++++++++++-------------------------
1 file changed, 23 insertions(+), 35 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index fdfdfc426539..702e0f2b8307 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -2194,10 +2194,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
* core function of the snapshot creation.
*/
ret = create_pending_snapshots(trans);
- if (ret) {
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_reloc;
/*
* We insert the dir indexes of the snapshots and update the inode
@@ -2210,16 +2208,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
* the nodes and leaves.
*/
ret = btrfs_run_delayed_items(trans);
- if (ret) {
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_reloc;
ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
- if (ret) {
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_reloc;
/*
* make sure none of the code above managed to slip in a
@@ -2245,11 +2239,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
mutex_lock(&fs_info->tree_log_mutex);
ret = commit_fs_roots(trans);
- if (ret) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_tree_log;
/*
* Since the transaction is done, we can apply the pending changes
@@ -2267,29 +2258,20 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
* new delayed refs. Must handle them or qgroup can be wrong.
*/
ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
- if (ret) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_tree_log;
/*
* Since fs roots are all committed, we can get a quite accurate
* new_roots. So let's do quota accounting.
*/
ret = btrfs_qgroup_account_extents(trans);
- if (ret < 0) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret < 0)
+ goto unlock_tree_log;
ret = commit_cowonly_roots(trans);
- if (ret) {
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
- }
+ if (ret)
+ goto unlock_tree_log;
/*
* The tasks which save the space cache and inode cache may also
@@ -2297,9 +2279,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
*/
if (TRANS_ABORTED(cur_trans)) {
ret = cur_trans->aborted;
- mutex_unlock(&fs_info->tree_log_mutex);
- mutex_unlock(&fs_info->reloc_mutex);
- goto scrub_continue;
+ goto unlock_tree_log;
}
btrfs_prepare_extent_commit(fs_info);
@@ -2346,6 +2326,10 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
if (ret) {
btrfs_handle_fs_error(fs_info, ret,
"Error while writing out transaction");
+ /*
+ * reloc_mutex has been unlocked, tree_log_mutex is still held
+ * but we can't jump to unlock_tree_log causing double unlock
+ */
mutex_unlock(&fs_info->tree_log_mutex);
goto scrub_continue;
}
@@ -2394,6 +2378,10 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
return ret;
+unlock_tree_log:
+ mutex_unlock(&fs_info->tree_log_mutex);
+unlock_reloc:
+ mutex_unlock(&fs_info->reloc_mutex);
scrub_continue:
btrfs_scrub_continue(fs_info);
cleanup_transaction:
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v2] btrfs: merge unlocking to common exit block in btrfs_commit_transaction
2020-02-24 15:13 ` [PATCH v2] " David Sterba
@ 2020-02-25 4:00 ` Anand Jain
0 siblings, 0 replies; 34+ messages in thread
From: Anand Jain @ 2020-02-25 4:00 UTC (permalink / raw)
To: David Sterba, linux-btrfs; +Cc: Johannes.Thumshirn
On 2/24/20 11:13 PM, David Sterba wrote:
> The tree_log_mutex and reloc_mutex locks are properly nested so we can
> simplify error handling and add labels for them. This reduces line count
> of the function.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>
> v2:
> - fixed label after commit_fs_roots, to point to unlock_tree_log
> - added comment after btrfs_handle_fs_error and keep the locks as is
Reviewed-by: Anand Jain <anand.jain@oracle.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 11/11] btrfs: reduce pointer intdirections in btree_readpage_end_io_hook
2020-02-21 16:30 [PATCH 00/11] Minor cleanups David Sterba
` (9 preceding siblings ...)
2020-02-21 16:31 ` [PATCH 10/11] btrfs: merge unlocking to common exit block in btrfs_commit_transaction David Sterba
@ 2020-02-21 16:31 ` David Sterba
2020-02-22 8:54 ` Johannes Thumshirn
10 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2020-02-21 16:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
All we need to read is checksum size from fs_info superblock, and
fs_info is provided by extent buffer so we can get rid of the wild
pointer indirections from page/inode/root.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/disk-io.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index ab8d975f071c..bccdb55ececf 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -604,9 +604,8 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
u64 found_start;
int found_level;
struct extent_buffer *eb;
- struct btrfs_root *root = BTRFS_I(page->mapping->host)->root;
- struct btrfs_fs_info *fs_info = root->fs_info;
- u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+ struct btrfs_fs_info *fs_info;
+ u16 csum_size;
int ret = 0;
u8 result[BTRFS_CSUM_SIZE];
int reads_done;
@@ -615,6 +614,8 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
goto out;
eb = (struct extent_buffer *)page->private;
+ fs_info = eb->fs_info;
+ csum_size = btrfs_super_csum_size(fs_info->super_copy);
/* the pending IO might have been the only thing that kept this buffer
* in memory. Make sure we have a ref for all this other checks
--
2.25.0
^ permalink raw reply related [flat|nested] 34+ messages in thread