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

[Michal Hocko <mhocko@kernel.org>]

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@linux.intel.com> 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>