linux-kernel - Re: [PATCH] mm: page_alloc: remove branch operation in free_pages

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:	Sat, 09 Mar 2013 11:54:24 +0800
From:	Will Huck <will.huckk@...il.com>
To:	Hugh Dickins <hughd@...gle.com>
CC:	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH] mm: page_alloc: remove branch operation in free_pages_prepare()

Hi Hugh,
On 03/08/2013 10:01 AM, Hugh Dickins wrote:
> On Fri, 8 Mar 2013, Joonsoo Kim wrote:
>> On Thu, Mar 07, 2013 at 10:54:15AM -0800, Hugh Dickins wrote:
>>> On Thu, 7 Mar 2013, Joonsoo Kim wrote:
>>>
>>>> When we found that the flag has a bit of PAGE_FLAGS_CHECK_AT_PREP,
>>>> we reset the flag. If we always reset the flag, we can reduce one
>>>> branch operation. So remove it.
>>>>
>>>> Cc: Hugh Dickins <hughd@...gle.com>
>>>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@....com>
>>> I don't object to this patch.  But certainly I would have written it
>>> that way in order not to dirty a cacheline unnecessarily.  It may be
>>> obvious to you that the cacheline in question is almost always already
>>> dirty, and the branch almost always more expensive.  But I'll leave that
>>> to you, and to those who know more about these subtle costs than I do.
>> Yes. I already think about that. I thought that even if a cacheline is
>> not dirty at this time, we always touch the 'struct page' in
>> set_freepage_migratetype() a little later, so dirtying is not the problem.
> I expect that a very high proportion of user pages have
> PG_uptodate to be cleared here; and there's also the recently added

When PG_uptodate will be set?

> page_nid_reset_last(), which will dirty the flags or a nearby field
> when CONFIG_NUMA_BALANCING.  Those argue in favour of your patch.
>
>> But, now, I re-think this and decide to drop this patch.
>> The reason is that 'struct page' of 'compound pages' may not be dirty
>> at this time and will not be dirty at later time.
> Actual compound pages would have PG_head or PG_tail or PG_compound
> to be cleared there, I believe (check if I'm right on that).  The
> questionable case is the ordinary order>0 case without __GFP_COMP
> (and page_nid_reset_last() is applied to each subpage of those).
>
>> So this patch is bad idea.
> I'm not so sure.  I doubt your patch will make a giant improvement
> in kernel performance!  But it might make a little - maybe you just
> need to give some numbers from perf to justify it (but I'm easily
> dazzled by numbers - don't expect me to judge the result).
>
> Hugh
>
>> Is there any comments?
>>
>> Thanks.
>>
>>> Hugh
>>>
>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>> index 8fcced7..778f2a9 100644
>>>> --- a/mm/page_alloc.c
>>>> +++ b/mm/page_alloc.c
>>>> @@ -614,8 +614,7 @@ static inline int free_pages_check(struct page *page)
>>>>   		return 1;
>>>>   	}
>>>>   	page_nid_reset_last(page);
>>>> -	if (page->flags & PAGE_FLAGS_CHECK_AT_PREP)
>>>> -		page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>>> +	page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>>>   	return 0;
>>>>   }
>>>>   
>>> --
>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>> the body to majordomo@...ck.org.  For more info on Linux MM,
>>> see: http://www.linux-mm.org/ .
>>> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>

--
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/