diff mbox

[4/4,RESEND] Btrfs: reduce size of struct btrfs_inode

Message ID 20180424233717.31283-5-nefelim4ag@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Timofey Titovets April 24, 2018, 11:37 p.m. UTC
Currently btrfs_inode have size equal 1136 bytes. (On x86_64).

struct btrfs_inode store several vars releated to compression code,
all states use 1 or 2 bits.

Lets declare bitfields for compression releated vars, to reduce
sizeof btrfs_inode to 1128 bytes.

Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
---
 fs/btrfs/btrfs_inode.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

David Sterba April 26, 2018, 1:42 p.m. UTC | #1
On Wed, Apr 25, 2018 at 02:37:17AM +0300, Timofey Titovets wrote:
> Currently btrfs_inode have size equal 1136 bytes. (On x86_64).
> 
> struct btrfs_inode store several vars releated to compression code,
> all states use 1 or 2 bits.
> 
> Lets declare bitfields for compression releated vars, to reduce
> sizeof btrfs_inode to 1128 bytes.

Unfortunatelly, this has no big effect. The inodes are allocated from a
slab page, that's 4k and there are at most 3 inodes there. Snippet from
/proc/slabinfo:

# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab>
btrfs_inode       256043 278943   1096    3    1

The size on my box is 1096 as it's 4.14, but this should not matter to
demonstrate the idea.

objperslab is 3 here, ie. there are 3 btrfs_inode in the page, and
there's 4096 - 3 * 1096 = 808 of slack space. In order to pack 4 inodes
per page, we'd have to squeeze the inode size to 1024 bytes. I've looked
into that and did not see enough members to remove or substitute. IIRC
there were like 24-32 bytes possible to shave, but that was it.

Once we'd get to 1024, adding anything new to btrfs_inode would be quite
difficult and as it goes, there's always something to add to the inode.

So I'd take a different approach, to regroup items and decide by
cacheline access patterns what to put together and what to separate.

The maximum size of inode before going to 2 objects per page is 1365, so
there's enough space for cacheline alignments.
--
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
Timofey Titovets April 28, 2018, 10:01 a.m. UTC | #2
чт, 26 апр. 2018 г. в 16:44, David Sterba <dsterba@suse.cz>:

> On Wed, Apr 25, 2018 at 02:37:17AM +0300, Timofey Titovets wrote:
> > Currently btrfs_inode have size equal 1136 bytes. (On x86_64).
> >
> > struct btrfs_inode store several vars releated to compression code,
> > all states use 1 or 2 bits.
> >
> > Lets declare bitfields for compression releated vars, to reduce
> > sizeof btrfs_inode to 1128 bytes.

> Unfortunatelly, this has no big effect. The inodes are allocated from a
> slab page, that's 4k and there are at most 3 inodes there. Snippet from
> /proc/slabinfo:

> # name            <active_objs> <num_objs> <objsize> <objperslab>
<pagesperslab>
> btrfs_inode       256043 278943   1096    3    1

> The size on my box is 1096 as it's 4.14, but this should not matter to
> demonstrate the idea.

> objperslab is 3 here, ie. there are 3 btrfs_inode in the page, and
> there's 4096 - 3 * 1096 = 808 of slack space. In order to pack 4 inodes
> per page, we'd have to squeeze the inode size to 1024 bytes. I've looked
> into that and did not see enough members to remove or substitute. IIRC
> there were like 24-32 bytes possible to shave, but that was it.

> Once we'd get to 1024, adding anything new to btrfs_inode would be quite
> difficult and as it goes, there's always something to add to the inode.

> So I'd take a different approach, to regroup items and decide by
> cacheline access patterns what to put together and what to separate.

> The maximum size of inode before going to 2 objects per page is 1365, so
> there's enough space for cacheline alignments.

May be i misunderstood something, but i was think that slab combine several
pages
in continuous range, so object in slab can cross page boundary.
So, all calculation will be very depends on scale of slab size.

i.e. on my machine that looks quite different:
name                    <active_objs>  <num_objs>  <objsize>  <objperslab>
  <pagesperslab>
btrfs_inode             142475         146272      1136       28
  8

So,
PAGE_SIZE * pagesperslab / objperslab
4096 * 8 / 28 = 1170.28

4096*8 - 1136*28 = 960

That's looks like object can cross page boundary in slab.
So, if size reduced to 1128,
4096 * 8 / 29 = 1129.93
4096*8 - 1128*29 = 56

Did i miss something?

Thanks.
David Sterba May 1, 2018, 3:39 p.m. UTC | #3
On Sat, Apr 28, 2018 at 10:01:57AM +0000, Timofey Titovets wrote:
> May be i misunderstood something, but i was think that slab combine
> several pages in continuous range, so object in slab can cross page
> boundary.  So, all calculation will be very depends on scale of slab
> size.
> 
> i.e. on my machine that looks quite different:
> name                    <active_objs>  <num_objs>  <objsize>  <objperslab>   <pagesperslab>
> btrfs_inode             142475         146272      1136       28             8
> 
> So,
> PAGE_SIZE * pagesperslab / objperslab
> 4096 * 8 / 28 = 1170.28
> 
> 4096*8 - 1136*28 = 960
> 
> That's looks like object can cross page boundary in slab.
> So, if size reduced to 1128,
> 4096 * 8 / 29 = 1129.93
> 4096*8 - 1128*29 = 56
> 
> Did i miss something?

Then this depends on the allocator, SLAB does 4k pages and SLUB 32k, so
there's a value in reducing the inode size.
--
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
diff mbox

Patch

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 9eb0c92ee4b4..9d29d7e68757 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -181,13 +181,13 @@  struct btrfs_inode {
 	/*
 	 * Cached values of inode properties
 	 */
-	unsigned prop_compress;		/* per-file compression algorithm */
+	unsigned prop_compress : 2;	/* per-file compression algorithm */
 	/*
 	 * Force compression on the file using the defrag ioctl, could be
 	 * different from prop_compress and takes precedence if set
 	 */
-	unsigned defrag_compress;
-	unsigned change_compress;
+	unsigned defrag_compress : 2;
+	unsigned change_compress : 1;
 
 	struct btrfs_delayed_node *delayed_node;