[PATCH 0/6] Shrink some data structures

[PATCH 0/6] Shrink some data structures

[David Sterba]

Unused members removed, other reordered.

struct btrfs_transaction:

-       /* size: 432, cachelines: 7, members: 27 */
-       /* sum members: 416, holes: 4, sum holes: 16 */
-       /* last cacheline: 48 bytes */
+       /* size: 416, cachelines: 7, members: 27 */
+       /* sum members: 412, holes: 1, sum holes: 4 */
+       /* last cacheline: 32 bytes */

struct btrfs_trans_handle:

-       /* size: 120, cachelines: 2, members: 20 */
-       /* sum members: 117, holes: 1, sum holes: 3 */
-       /* last cacheline: 56 bytes */
+       /* size: 104, cachelines: 2, members: 19 */
+       /* last cacheline: 40 bytes *

And incidentally the .text size goes down as well:

   text    data     bss     dec     hex filename
 985430   75100   18560 1079090  107732 pre/btrfs.ko
 985243   75100   18560 1078903  107677 post/btrfs.ko

David Sterba (6):
  btrfs: switch btrfs_trans_handle::adding_csums to bool
  btrfs: remove unused member of btrfs_trans_handle
  btrfs: switch to refcount_t type for btrfs_trans_handle::use_count
  btrfs: reoder btrfs_trans_handle members for better packing
  btrfs: use narrower type for btrfs_transaction::num_dirty_bgs
  btrfs: reoder btrfs_transaction members for better packing

 fs/btrfs/extent-tree.c |  2 +-
 fs/btrfs/inode.c       |  4 ++--
 fs/btrfs/transaction.c | 12 ++++++------
 fs/btrfs/transaction.h | 11 +++++------
 4 files changed, 14 insertions(+), 15 deletions(-)

-- 
2.14.3

[PATCH 1/6] btrfs: switch btrfs_trans_handle::adding_csums to bool

[David Sterba]

The semantics of adding_csums matches bool, 'short' was most likely used
to save space in a698d0755adb6f2 ("Btrfs: add a type field for the
transaction handle").

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/inode.c       | 4 ++--
 fs/btrfs/transaction.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f352662e066f..3a32e782ccc4 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2036,10 +2036,10 @@ static noinline int add_pending_csums(struct btrfs_trans_handle *trans,
 	struct btrfs_ordered_sum *sum;
 
 	list_for_each_entry(sum, list, list) {
-		trans->adding_csums = 1;
+		trans->adding_csums = true;
 		btrfs_csum_file_blocks(trans,
 		       BTRFS_I(inode)->root->fs_info->csum_root, sum);
-		trans->adding_csums = 0;
+		trans->adding_csums = false;
 	}
 	return 0;
 }
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index c55e44560103..a673142c003e 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -118,7 +118,7 @@ struct btrfs_trans_handle {
 	struct btrfs_block_rsv *block_rsv;
 	struct btrfs_block_rsv *orig_rsv;
 	short aborted;
-	short adding_csums;
+	bool adding_csums;
 	bool allocating_chunk;
 	bool can_flush_pending_bgs;
 	bool reloc_reserved;
-- 
2.14.3

[PATCH 2/6] btrfs: remove unused member of btrfs_trans_handle

[David Sterba]

Last user was removed in a monster commit a22285a6a32390195235171
("Btrfs: Integrate metadata reservation with start_transaction") in
2010.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/transaction.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index a673142c003e..c48a4a03f1b4 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -112,7 +112,6 @@ struct btrfs_trans_handle {
 	u64 bytes_reserved;
 	u64 chunk_bytes_reserved;
 	unsigned long use_count;
-	unsigned long blocks_reserved;
 	unsigned long delayed_ref_updates;
 	struct btrfs_transaction *transaction;
 	struct btrfs_block_rsv *block_rsv;
-- 
2.14.3

[PATCH 3/6] btrfs: switch to refcount_t type for btrfs_trans_handle::use_count

[David Sterba]

The use_count is a reference counter, we can use the refcount_t type,
though we don't use the atomicity. This is not a performance critical
code and we could catch the underflows. The type is changed from long,
but the number of references will fit an int.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/transaction.c | 12 ++++++------
 fs/btrfs/transaction.h |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index dac688c696c3..6348573e26a7 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -495,8 +495,8 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
 	if (current->journal_info) {
 		WARN_ON(type & TRANS_EXTWRITERS);
 		h = current->journal_info;
-		h->use_count++;
-		WARN_ON(h->use_count > 2);
+		refcount_inc(&h->use_count);
+		WARN_ON(refcount_read(&h->use_count) > 2);
 		h->orig_rsv = h->block_rsv;
 		h->block_rsv = NULL;
 		goto got_it;
@@ -567,7 +567,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items,
 	h->transid = cur_trans->transid;
 	h->transaction = cur_trans;
 	h->root = root;
-	h->use_count = 1;
+	refcount_set(&h->use_count, 1);
 	h->fs_info = root->fs_info;
 
 	h->type = type;
@@ -837,8 +837,8 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
 	int err = 0;
 	int must_run_delayed_refs = 0;
 
-	if (trans->use_count > 1) {
-		trans->use_count--;
+	if (refcount_read(&trans->use_count) > 1) {
+		refcount_dec(&trans->use_count);
 		trans->block_rsv = trans->orig_rsv;
 		return 0;
 	}
@@ -1868,7 +1868,7 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans,
 	struct btrfs_transaction *cur_trans = trans->transaction;
 	DEFINE_WAIT(wait);
 
-	WARN_ON(trans->use_count > 1);
+	WARN_ON(refcount_read(&trans->use_count) > 1);
 
 	btrfs_abort_transaction(trans, err);
 
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index c48a4a03f1b4..afa88f035654 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -111,7 +111,7 @@ struct btrfs_trans_handle {
 	u64 transid;
 	u64 bytes_reserved;
 	u64 chunk_bytes_reserved;
-	unsigned long use_count;
+	refcount_t use_count;
 	unsigned long delayed_ref_updates;
 	struct btrfs_transaction *transaction;
 	struct btrfs_block_rsv *block_rsv;
-- 
2.14.3

[PATCH 4/6] btrfs: reoder btrfs_trans_handle members for better packing

[David Sterba]

Recent updates to the structure left some holes, reorder the types so
the packing is tight. The size goes from 112 to 104 on 64bit.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/transaction.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index afa88f035654..edf53112a6f2 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -111,11 +111,12 @@ struct btrfs_trans_handle {
 	u64 transid;
 	u64 bytes_reserved;
 	u64 chunk_bytes_reserved;
-	refcount_t use_count;
 	unsigned long delayed_ref_updates;
 	struct btrfs_transaction *transaction;
 	struct btrfs_block_rsv *block_rsv;
 	struct btrfs_block_rsv *orig_rsv;
+	refcount_t use_count;
+	unsigned int type;
 	short aborted;
 	bool adding_csums;
 	bool allocating_chunk;
@@ -123,7 +124,6 @@ struct btrfs_trans_handle {
 	bool reloc_reserved;
 	bool sync;
 	bool dirty;
-	unsigned int type;
 	struct btrfs_root *root;
 	struct btrfs_fs_info *fs_info;
 	struct list_head new_bgs;
-- 
2.14.3

[PATCH 5/6] btrfs: use narrower type for btrfs_transaction::num_dirty_bgs

[David Sterba]

The u64 is an overkill here, we could not possibly create that many
blockgroups in one transaction.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-tree.c | 2 +-
 fs/btrfs/transaction.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 11b7d613140b..d59a65852447 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2894,7 +2894,7 @@ int btrfs_check_space_for_delayed_refs(struct btrfs_trans_handle *trans,
 	struct btrfs_block_rsv *global_rsv;
 	u64 num_heads = trans->transaction->delayed_refs.num_heads_ready;
 	u64 csum_bytes = trans->transaction->delayed_refs.pending_csums;
-	u64 num_dirty_bgs = trans->transaction->num_dirty_bgs;
+	unsigned int num_dirty_bgs = trans->transaction->num_dirty_bgs;
 	u64 num_bytes, num_dirty_bgs_bytes;
 	int ret = 0;
 
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index edf53112a6f2..1805fd101767 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -70,7 +70,7 @@ struct btrfs_transaction {
 	struct list_head dirty_bgs;
 	struct list_head io_bgs;
 	struct list_head dropped_roots;
-	u64 num_dirty_bgs;
+	unsigned int num_dirty_bgs;
 
 	/*
 	 * we need to make sure block group deletion doesn't race with
-- 
2.14.3

[PATCH 6/6] btrfs: reoder btrfs_transaction members for better packing

[David Sterba]

There are now 20 bytes of holes, we can reduce that to 4 by minor
changes. Moving 'aborted' to the status and flags is also more logical,
similar for num_dirty_bgs. The size goes from 432 to 416.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/transaction.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 1805fd101767..6beee072b1bd 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -58,6 +58,7 @@ struct btrfs_transaction {
 
 	/* Be protected by fs_info->trans_lock when we want to change it. */
 	enum btrfs_trans_state state;
+	int aborted;
 	struct list_head list;
 	struct extent_io_tree dirty_pages;
 	unsigned long start_time;
@@ -70,7 +71,6 @@ struct btrfs_transaction {
 	struct list_head dirty_bgs;
 	struct list_head io_bgs;
 	struct list_head dropped_roots;
-	unsigned int num_dirty_bgs;
 
 	/*
 	 * we need to make sure block group deletion doesn't race with
@@ -79,11 +79,11 @@ struct btrfs_transaction {
 	 */
 	struct mutex cache_write_mutex;
 	spinlock_t dirty_bgs_lock;
+	unsigned int num_dirty_bgs;
 	/* Protected by spin lock fs_info->unused_bgs_lock. */
 	struct list_head deleted_bgs;
 	spinlock_t dropped_roots_lock;
 	struct btrfs_delayed_ref_root delayed_refs;
-	int aborted;
 	struct btrfs_fs_info *fs_info;
 };
 
-- 
2.14.3

Re: [PATCH 6/6] btrfs: reoder btrfs_transaction members for better packing

[Liu Bo]

On Wed, Nov 15, 2017 at 06:42:06PM +0100, David Sterba wrote:
> There are now 20 bytes of holes, we can reduce that to 4 by minor
> changes. Moving 'aborted' to the status and flags is also more logical,
> similar for num_dirty_bgs. The size goes from 432 to 416.
> 
Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

Thanks,

-liubo
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/transaction.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 1805fd101767..6beee072b1bd 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -58,6 +58,7 @@ struct btrfs_transaction {
>  
>  	/* Be protected by fs_info->trans_lock when we want to change it. */
>  	enum btrfs_trans_state state;
> +	int aborted;
>  	struct list_head list;
>  	struct extent_io_tree dirty_pages;
>  	unsigned long start_time;
> @@ -70,7 +71,6 @@ struct btrfs_transaction {
>  	struct list_head dirty_bgs;
>  	struct list_head io_bgs;
>  	struct list_head dropped_roots;
> -	unsigned int num_dirty_bgs;
>  
>  	/*
>  	 * we need to make sure block group deletion doesn't race with
> @@ -79,11 +79,11 @@ struct btrfs_transaction {
>  	 */
>  	struct mutex cache_write_mutex;
>  	spinlock_t dirty_bgs_lock;
> +	unsigned int num_dirty_bgs;
>  	/* Protected by spin lock fs_info->unused_bgs_lock. */
>  	struct list_head deleted_bgs;
>  	spinlock_t dropped_roots_lock;
>  	struct btrfs_delayed_ref_root delayed_refs;
> -	int aborted;
>  	struct btrfs_fs_info *fs_info;
>  };
>  
> -- 
> 2.14.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html