* free_pages_check
@ 2008-01-08 2:43 Yinghai Lu
2008-01-08 3:08 ` free_pages_check Andrew Morton
2008-01-08 3:34 ` free_pages_check Nick Piggin
0 siblings, 2 replies; 5+ messages in thread
From: Yinghai Lu @ 2008-01-08 2:43 UTC (permalink / raw)
To: Andrew Morton, Christoph Lameter, Eric W. Biederman, Adrian Bunk; +Cc: LKML
wonder why free_pages_check mm/page_alloc.c is using bit OR than logical OR
@@ -450,9 +450,9 @@ static inline void __free_one_page(struc
static inline int free_pages_check(struct page *page)
{
- if (unlikely(page_mapcount(page) |
- (page->mapping != NULL) |
- (page_count(page) != 0) |
+ if (unlikely(page_mapcount(page) ||
+ (page->mapping != NULL) ||
+ (page_count(page) != 0) ||
(page->flags & (
1 << PG_lru |
1 << PG_private |
YH
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: free_pages_check
2008-01-08 2:43 free_pages_check Yinghai Lu
@ 2008-01-08 3:08 ` Andrew Morton
2008-01-08 3:34 ` free_pages_check Nick Piggin
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2008-01-08 3:08 UTC (permalink / raw)
To: Yinghai Lu; +Cc: Christoph Lameter, Eric W. Biederman, Adrian Bunk, LKML
On Mon, 7 Jan 2008 18:43:46 -0800 "Yinghai Lu" <yhlu.kernel@gmail.com> wrote:
> wonder why free_pages_check mm/page_alloc.c is using bit OR than logical OR
>
> @@ -450,9 +450,9 @@ static inline void __free_one_page(struc
>
> static inline int free_pages_check(struct page *page)
> {
> - if (unlikely(page_mapcount(page) |
> - (page->mapping != NULL) |
> - (page_count(page) != 0) |
> + if (unlikely(page_mapcount(page) ||
> + (page->mapping != NULL) ||
> + (page_count(page) != 0) ||
> (page->flags & (
> 1 << PG_lru |
> 1 << PG_private |
It's a hack^Wtrick to avoid the generation of a series of not-taken
branches. I don't recall anyone verifying that it is actually of any
benefit, but the theory is good.
The lack of a comment, however, is not.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: free_pages_check
2008-01-08 2:43 free_pages_check Yinghai Lu
2008-01-08 3:08 ` free_pages_check Andrew Morton
@ 2008-01-08 3:34 ` Nick Piggin
2008-01-08 5:44 ` free_pages_check H. Peter Anvin
1 sibling, 1 reply; 5+ messages in thread
From: Nick Piggin @ 2008-01-08 3:34 UTC (permalink / raw)
To: Yinghai Lu
Cc: Andrew Morton, Christoph Lameter, Eric W. Biederman, Adrian Bunk,
LKML
On Tuesday 08 January 2008 13:43, Yinghai Lu wrote:
> wonder why free_pages_check mm/page_alloc.c is using bit OR than logical OR
>
> @@ -450,9 +450,9 @@ static inline void __free_one_page(struc
>
> static inline int free_pages_check(struct page *page)
> {
> - if (unlikely(page_mapcount(page) |
> - (page->mapping != NULL) |
> - (page_count(page) != 0) |
> + if (unlikely(page_mapcount(page) ||
> + (page->mapping != NULL) ||
> + (page_count(page) != 0) ||
> (page->flags & (
> 1 << PG_lru |
> 1 << PG_private |
Because the positive case is extremely rare, so there is no benefit (nor
any correctness requirement) for short-circuit evaluation, and we don't
want to have all the branches that it involves. I think it is 3 more
conditional jumps.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: free_pages_check
2008-01-08 3:34 ` free_pages_check Nick Piggin
@ 2008-01-08 5:44 ` H. Peter Anvin
2008-01-08 7:44 ` free_pages_check Nick Piggin
0 siblings, 1 reply; 5+ messages in thread
From: H. Peter Anvin @ 2008-01-08 5:44 UTC (permalink / raw)
To: Nick Piggin
Cc: Yinghai Lu, Andrew Morton, Christoph Lameter, Eric W. Biederman,
Adrian Bunk, LKML
Nick Piggin wrote:
> On Tuesday 08 January 2008 13:43, Yinghai Lu wrote:
>> wonder why free_pages_check mm/page_alloc.c is using bit OR than logical OR
>>
>> @@ -450,9 +450,9 @@ static inline void __free_one_page(struc
>>
>> static inline int free_pages_check(struct page *page)
>> {
>> - if (unlikely(page_mapcount(page) |
>> - (page->mapping != NULL) |
>> - (page_count(page) != 0) |
>> + if (unlikely(page_mapcount(page) ||
>> + (page->mapping != NULL) ||
>> + (page_count(page) != 0) ||
>> (page->flags & (
>> 1 << PG_lru |
>> 1 << PG_private |
>
> Because the positive case is extremely rare, so there is no benefit (nor
> any correctness requirement) for short-circuit evaluation, and we don't
> want to have all the branches that it involves. I think it is 3 more
> conditional jumps.
Depends on how smart the compiler is. If the page_() functions are
inlines or macros, there is only one pointer reference involved and it
should be able to do that transformation. Whether or not gcc is that
smart is another matter.
-hpa
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: free_pages_check
2008-01-08 5:44 ` free_pages_check H. Peter Anvin
@ 2008-01-08 7:44 ` Nick Piggin
0 siblings, 0 replies; 5+ messages in thread
From: Nick Piggin @ 2008-01-08 7:44 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Yinghai Lu, Andrew Morton, Christoph Lameter, Eric W. Biederman,
Adrian Bunk, LKML
On Tuesday 08 January 2008 16:44, H. Peter Anvin wrote:
> Nick Piggin wrote:
> > On Tuesday 08 January 2008 13:43, Yinghai Lu wrote:
> >> wonder why free_pages_check mm/page_alloc.c is using bit OR than logical
> >> OR
> >>
> >> @@ -450,9 +450,9 @@ static inline void __free_one_page(struc
> >>
> >> static inline int free_pages_check(struct page *page)
> >> {
> >> - if (unlikely(page_mapcount(page) |
> >> - (page->mapping != NULL) |
> >> - (page_count(page) != 0) |
> >> + if (unlikely(page_mapcount(page) ||
> >> + (page->mapping != NULL) ||
> >> + (page_count(page) != 0) ||
> >> (page->flags & (
> >> 1 << PG_lru |
> >> 1 << PG_private |
> >
> > Because the positive case is extremely rare, so there is no benefit (nor
> > any correctness requirement) for short-circuit evaluation, and we don't
> > want to have all the branches that it involves. I think it is 3 more
> > conditional jumps.
>
> Depends on how smart the compiler is. If the page_() functions are
> inlines or macros, there is only one pointer reference involved and it
> should be able to do that transformation. Whether or not gcc is that
> smart is another matter.
Mine is not.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-01-08 7:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-08 2:43 free_pages_check Yinghai Lu
2008-01-08 3:08 ` free_pages_check Andrew Morton
2008-01-08 3:34 ` free_pages_check Nick Piggin
2008-01-08 5:44 ` free_pages_check H. Peter Anvin
2008-01-08 7:44 ` free_pages_check Nick Piggin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox