Re: [PATCHv3 3/5] mm: pack compound_dtor and compound_order into one word in struct page

From: Michal Hocko
Date: Fri Aug 21 2015 - 03:13:49 EST


On Thu 20-08-15 16:26:04, Andrew Morton wrote:
> On Wed, 19 Aug 2015 12:21:44 +0300 "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
>
> > The patch halves space occupied by compound_dtor and compound_order in
> > struct page.
> >
> > For compound_order, it's trivial long -> int/short conversion.
> >
> > For get_compound_page_dtor(), we now use hardcoded table for destructor
> > lookup and store its index in the struct page instead of direct pointer
> > to destructor. It shouldn't be a big trouble to maintain the table: we
> > have only two destructor and NULL currently.
> >
> > This patch free up one word in tail pages for reuse. This is preparation
> > for the next patch.
> >
> > ...
> >
> > @@ -145,8 +143,13 @@ struct page {
> > */
> > /* First tail page of compound page */
> > struct {
> > - compound_page_dtor *compound_dtor;
> > - unsigned long compound_order;
> > +#ifdef CONFIG_64BIT
> > + unsigned int compound_dtor;
> > + unsigned int compound_order;
> > +#else
> > + unsigned short int compound_dtor;
> > + unsigned short int compound_order;
> > +#endif
>
> Why not use ushort for 64-bit as well?

Yeah, I have asked the same in the previous round. So I've tried to
compile with ushort. The resulting code was slightly larger
text data bss dec hex filename
476370 90811 44632 611813 955e5 mm/built-in.o.prev
476418 90811 44632 611861 95615 mm/built-in.o.after

E.g. prep_compound_page
before:
4c6b: c7 47 68 01 00 00 00 movl $0x1,0x68(%rdi)
4c72: 89 77 6c mov %esi,0x6c(%rdi)
after:
4c6c: 66 c7 47 68 01 00 movw $0x1,0x68(%rdi)
4c72: 66 89 77 6a mov %si,0x6a(%rdi)

which looks very similar to me but I am not an expert here so it might
possible that movw is slower.

__free_pages_ok
before:
63af: 8b 77 6c mov 0x6c(%rdi),%esi
after:
63b1: 0f b7 77 6a movzwl 0x6a(%rdi),%esi

which looks like a worse code to me. Whether this all is measurable or
worth it I dunno. The ifdef is ugly but maybe the ugliness is a destiny
for struct page.
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/