Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization

Joonsoo Kim
2013-08-05T08:10:01Z

Hello, Michal.

On Fri, Aug 02, 2013 at 11:36:07PM +0200, Michal Hocko wrote:
> On Fri 02-08-13 16:47:10, Johannes Weiner wrote:
> > On Fri, Aug 02, 2013 at 06:27:22PM +0200, Michal Hocko wrote:
> > > On Fri 02-08-13 11:07:56, Joonsoo Kim wrote:
> > > > We rarely allocate a page with ALLOC_NO_WATERMARKS and it is used
> > > > in slow path. For making fast path more faster, add likely macro to
> > > > help compiler optimization.
> > >
> > > The code is different in mmotm tree (see mm: page_alloc: rearrange
> > > watermark checking in get_page_from_freelist)
> >
> > Yes, please rebase this on top.
> >
> > > Besides that, make sure you provide numbers which prove your claims
> > > about performance optimizations.
> >
> > Isn't that a bit overkill? We know it's a likely path (we would
> > deadlock constantly if a sizable portion of allocations were to ignore
> > the watermarks). Does he have to justify that likely in general makes
> > sense?
>
> That was more a generic comment. If there is a claim that something
> would be faster it would be nice to back that claim by some numbers
> (e.g. smaller hot path).
>
> In this particular case, unlikely(alloc_flags & ALLOC_NO_WATERMARKS)
> doesn't make any change to the generated code with gcc 4.8.1 resp.
> 4.3.4 I have here.
> Maybe other versions of gcc would benefit from the hint but changelog
> didn't tell us. I wouldn't add the anotation if it doesn't make any
> difference for the resulting code.

Hmm, Is there no change with gcc 4.8.1 and 4.3.4?

I found a change with gcc 4.6.3 and v3.10 kernel.

text data bss dec hex filename
35683 1461 644 37788 939c page_alloc_base.o
35715 1461 644 37820 93bc page_alloc_patch.o

Slightly larger (32 bytes) than before.
And assembly code looks different as I expected.

* Original code

17126 .LVL1518:
17127 .loc 2 1904 0 is_stmt 1
17128 testb $4, -116(%rbp) #, %sfp
17129 je .L866 #,

(snip)

17974 .L866:
17975 .LBE6053:
17976 .LBE6052:
17977 .LBE6051:
17978 .LBE6073:
17979 .LBE6093:
17980 .LBB6094:
17981 .loc 2 1908 0
17982 movl -116(%rbp), %r14d # %sfp, D.42080
17983 .loc 2 1909 0
17984 movl -116(%rbp), %r8d # %sfp,
17985 movq %rbx, %rdi # prephitmp.1723,
17986 movl -212(%rbp), %ecx # %sfp,
17987 movl -80(%rbp), %esi # %sfp,
17988 .loc 2 1908 0
17989 andl $3, %r14d #, D.42080
17990 movslq %r14d, %rax # D.42080, D.42080
17991 movq (%rbx,%rax,8), %r13 # prephitmp.1723_268->watermark, mark
17992 .LVL1591:
17993 .loc 2 1909 0
17994 movq %r13, %rdx # mark,
17995 call zone_watermark_ok #

On 17129 line, we check ALLOC_NO_WATERMARKS and if not matched, then jump to L866.
L866 is on 17981 line.

* Patched code

17122 .L807:
17123 .LVL1513:
17124 .loc 2 1904 0 is_stmt 1
17125 testb $4, -88(%rbp) #, %sfp
17126 jne .L811 #,
17127 .LBB6092:
17128 .loc 2 1908 0
17129 movl -88(%rbp), %r13d # %sfp, D.42082
17130 .loc 2 1909 0
17131 movl -88(%rbp), %r8d # %sfp,
17132 movq %rbx, %rdi # prephitmp.1723,
17133 movl -160(%rbp), %ecx # %sfp,
17134 movl -80(%rbp), %esi # %sfp,
17135 .loc 2 1908 0
17136 andl $3, %r13d #, D.42082
17137 movslq %r13d, %rax # D.42082, D.42082
17138 movq (%rbx,%rax,8), %r12 # prephitmp.1723_270->watermark, mark
17139 .LVL1514:
17140 .loc 2 1909 0
17141 movq %r12, %rdx # mark,
17142 call zone_watermark_ok #

On 17124 line, we check ALLOC_NO_WATERMARKS (0x4) and if not matched,
execute following code without jumping. This is effect of 'likely' macro.
Isn't it reasonable?

Thanks.