linux-kernel - Re: [PATCHv3 3/5] mm: pack compound_dtor and compound

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 21 Aug 2015 09:13:42 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Hugh Dickins <hughd@...gle.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Dave Hansen <dave.hansen@...el.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	Johannes Weiner <hannes@...xchg.org>,
	David Rientjes <rientjes@...gle.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCHv3 3/5] mm: pack compound_dtor and compound_order into one
 word in struct page

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@...ux.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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/