Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Mon, Aug 05, 2013 at 05:50:41PM +0900, Joonsoo Kim wrote: > On Mon, Aug 05, 2013 at 05:10:08PM +0900, Joonsoo Kim wrote: > > 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. > > Ah... I did a test on mmotm (Johannes's git) and found that this patch > doesn't make any effect. I guess, a change from Johannes ('rearrange > watermark checking in get_page_from_freelist') already makes better code > for !ALLOC_NO_WATERMARKS case. IMHO, although there is no effect, it is > better to add likely macro, because arrangement can be changed from time > to time without any consideration of assembly code generation. How about > your opinion, Johannes and Michal? I'm not against it. It does not really matter if gcc gets it right by accident right now and the annotation does not have an immediate effect. It's a compiler hint and we want gcc to know it so it doesn't ever assume otherwise in the future. Yes, nobody will re-evaluate if the conditional still generates the jumps correctly after shifting the code around. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Mon 05-08-13 17:50:41, Joonsoo Kim wrote: [...] > IMHO, although there is no effect, it is better to add likely macro, > because arrangement can be changed from time to time without any > consideration of assembly code generation. How about your opinion, > Johannes and Michal? This is a matter of taste. I do not like new *likely annotations if they do not make difference. But no strong objections if others like it. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Mon, Aug 05, 2013 at 05:10:08PM +0900, Joonsoo Kim wrote: > 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. Ah... I did a test on mmotm (Johannes's git) and found that this patch doesn't make any effect. I guess, a change from Johannes ('rearrange watermark checking in get_page_from_freelist') already makes better code for !ALLOC_NO_WATERMARKS case. IMHO, although there is no effect, it is better to add likely macro, because arrangement can be changed from time to time without any consideration of assembly code generation. How about your opinion, Johannes and Michal? Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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. textdata bss dec hex filename 35683 1461 644 37788939c page_alloc_base.o 35715 1461 644 3782093bc 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 callzone_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 callzone_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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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. textdata bss dec hex filename 35683 1461 644 37788939c page_alloc_base.o 35715 1461 644 3782093bc 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 callzone_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 callzone_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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Mon, Aug 05, 2013 at 05:10:08PM +0900, Joonsoo Kim wrote: 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. Ah... I did a test on mmotm (Johannes's git) and found that this patch doesn't make any effect. I guess, a change from Johannes ('rearrange watermark checking in get_page_from_freelist') already makes better code for !ALLOC_NO_WATERMARKS case. IMHO, although there is no effect, it is better to add likely macro, because arrangement can be changed from time to time without any consideration of assembly code generation. How about your opinion, Johannes and Michal? Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Mon 05-08-13 17:50:41, Joonsoo Kim wrote: [...] IMHO, although there is no effect, it is better to add likely macro, because arrangement can be changed from time to time without any consideration of assembly code generation. How about your opinion, Johannes and Michal? This is a matter of taste. I do not like new *likely annotations if they do not make difference. But no strong objections if others like it. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Mon, Aug 05, 2013 at 05:50:41PM +0900, Joonsoo Kim wrote: On Mon, Aug 05, 2013 at 05:10:08PM +0900, Joonsoo Kim wrote: 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. Ah... I did a test on mmotm (Johannes's git) and found that this patch doesn't make any effect. I guess, a change from Johannes ('rearrange watermark checking in get_page_from_freelist') already makes better code for !ALLOC_NO_WATERMARKS case. IMHO, although there is no effect, it is better to add likely macro, because arrangement can be changed from time to time without any consideration of assembly code generation. How about your opinion, Johannes and Michal? I'm not against it. It does not really matter if gcc gets it right by accident right now and the annotation does not have an immediate effect. It's a compiler hint and we want gcc to know it so it doesn't ever assume otherwise in the future. Yes, nobody will re-evaluate if the conditional still generates the jumps correctly after shifting the code around. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Fri, Aug 02, 2013 at 11:07:56AM +0900, 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. > > Signed-off-by: Joonsoo Kim Acked-by: Johannes Weiner -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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) Besides that, make sure you provide numbers which prove your claims about performance optimizations. > Signed-off-by: Joonsoo Kim > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index b100255..86ad44b 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1901,7 +1901,7 @@ zonelist_scan: > goto this_zone_full; > > BUILD_BUG_ON(ALLOC_NO_WATERMARKS < NR_WMARK); > - if (!(alloc_flags & ALLOC_NO_WATERMARKS)) { > + if (likely(!(alloc_flags & ALLOC_NO_WATERMARKS))) { > unsigned long mark; > int ret; > > -- > 1.7.9.5 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majord...@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: mailto:"d...@kvack.org;> em...@kvack.org -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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) Besides that, make sure you provide numbers which prove your claims about performance optimizations. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com diff --git a/mm/page_alloc.c b/mm/page_alloc.c index b100255..86ad44b 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1901,7 +1901,7 @@ zonelist_scan: goto this_zone_full; BUILD_BUG_ON(ALLOC_NO_WATERMARKS NR_WMARK); - if (!(alloc_flags ALLOC_NO_WATERMARKS)) { + if (likely(!(alloc_flags ALLOC_NO_WATERMARKS))) { unsigned long mark; int ret; -- 1.7.9.5 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majord...@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
On Fri, Aug 02, 2013 at 11:07:56AM +0900, 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. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com Acked-by: Johannes Weiner han...@cmpxchg.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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. Signed-off-by: Joonsoo Kim diff --git a/mm/page_alloc.c b/mm/page_alloc.c index b100255..86ad44b 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1901,7 +1901,7 @@ zonelist_scan: goto this_zone_full; BUILD_BUG_ON(ALLOC_NO_WATERMARKS < NR_WMARK); - if (!(alloc_flags & ALLOC_NO_WATERMARKS)) { + if (likely(!(alloc_flags & ALLOC_NO_WATERMARKS))) { unsigned long mark; int ret; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] mm, page_alloc: add likely macro to help compiler optimization
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. Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com diff --git a/mm/page_alloc.c b/mm/page_alloc.c index b100255..86ad44b 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1901,7 +1901,7 @@ zonelist_scan: goto this_zone_full; BUILD_BUG_ON(ALLOC_NO_WATERMARKS NR_WMARK); - if (!(alloc_flags ALLOC_NO_WATERMARKS)) { + if (likely(!(alloc_flags ALLOC_NO_WATERMARKS))) { unsigned long mark; int ret; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/