diff mbox

[OPW,kernel,v2,3/3] fs: btrfs: Pack struct btrfs_delayed_root

Message ID 5222f758b0d98cd2640d30c5f787def781bb2ea7.1382439014.git.rashika.kheria@gmail.com
State New, archived
Headers show

Commit Message

Rashika Oct. 22, 2013, 10:54 a.m. UTC
Packs the structure btrfs_delayed_root in delayed-inode.h to eliminate holes detected
by pahole in order to reduce space wastage.

Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
---

This revision fixes the following issues of the previous revision-
Unnecessary swap of variables

 fs/btrfs/delayed-inode.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Rusty Russell Oct. 24, 2013, 1:55 a.m. UTC | #1
Rashika Kheria <rashika.kheria@gmail.com> writes:
> Packs the structure btrfs_delayed_root in delayed-inode.h to eliminate holes detected
> by pahole in order to reduce space wastage.
>
> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>

Reviewed-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks!
Rusty.

> ---
>
> This revision fixes the following issues of the previous revision-
> Unnecessary swap of variables
>
>  fs/btrfs/delayed-inode.h |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/delayed-inode.h b/fs/btrfs/delayed-inode.h
> index a4b38f9..e511982 100644
> --- a/fs/btrfs/delayed-inode.h
> +++ b/fs/btrfs/delayed-inode.h
> @@ -34,18 +34,18 @@
>  #define BTRFS_DELAYED_DELETION_ITEM	2
>  
>  struct btrfs_delayed_root {
> -	spinlock_t lock;
>  	struct list_head node_list;
>  	/*
>  	 * Used for delayed nodes which is waiting to be dealt with by the
>  	 * worker. If the delayed node is inserted into the work queue, we
>  	 * drop it from this list.
>  	 */
> +	wait_queue_head_t wait;
>  	struct list_head prepare_list;
>  	atomic_t items;		/* for delayed items */
>  	atomic_t items_seq;	/* for delayed items */
>  	int nodes;		/* for delayed nodes */
> -	wait_queue_head_t wait;
> +	spinlock_t lock;
>  };
>  
>  struct btrfs_delayed_node {
> -- 
> 1.7.9.5
>
> -- 
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
Zach Brown Oct. 29, 2013, 11:44 p.m. UTC | #2
On Tue, Oct 22, 2013 at 04:24:46PM +0530, Rashika Kheria wrote:
> Packs the structure btrfs_delayed_root in delayed-inode.h to eliminate holes detected
> by pahole in order to reduce space wastage.

Why do you think this patch eliminates holes?

Come to think of it, you should include the pahole output that
summarizes the structure you're changing before and after your changes
in the commit message.   Something like this:

Before:
        /* size: 208, cachelines: 4, members: 7 */
        /* sum members: 204, holes: 1, sum holes: 4 */
After:
        /* size: 208, cachelines: 4, members: 7 */
        /* sum members: 204, holes: 1, sum holes: 4 */

:)

> -	spinlock_t lock;
>  	struct list_head node_list;
>  	/*
>  	 * Used for delayed nodes which is waiting to be dealt with by the
>  	 * worker. If the delayed node is inserted into the work queue, we
>  	 * drop it from this list.
>  	 */
> +	wait_queue_head_t wait;
>  	struct list_head prepare_list;

(and it was wrong to insert the wait between the prepare_list and the
comment above it that describes it.)

>  	atomic_t items;		/* for delayed items */
>  	atomic_t items_seq;	/* for delayed items */
>  	int nodes;		/* for delayed nodes */
> -	wait_queue_head_t wait;
> +	spinlock_t lock;

The problem here is that all the fields are 8byte aligned on 64bit,
except for the three atomic_t and ints.  No amount of shuffling is going
to change that.  You'll always have at least a 4 byte hole.

So just drop this patch.  It'd take a lot more work to shrink this
structure by changing the code that uses it.

- z
diff mbox

Patch

diff --git a/fs/btrfs/delayed-inode.h b/fs/btrfs/delayed-inode.h
index a4b38f9..e511982 100644
--- a/fs/btrfs/delayed-inode.h
+++ b/fs/btrfs/delayed-inode.h
@@ -34,18 +34,18 @@ 
 #define BTRFS_DELAYED_DELETION_ITEM	2
 
 struct btrfs_delayed_root {
-	spinlock_t lock;
 	struct list_head node_list;
 	/*
 	 * Used for delayed nodes which is waiting to be dealt with by the
 	 * worker. If the delayed node is inserted into the work queue, we
 	 * drop it from this list.
 	 */
+	wait_queue_head_t wait;
 	struct list_head prepare_list;
 	atomic_t items;		/* for delayed items */
 	atomic_t items_seq;	/* for delayed items */
 	int nodes;		/* for delayed nodes */
-	wait_queue_head_t wait;
+	spinlock_t lock;
 };
 
 struct btrfs_delayed_node {