[Devel] [PATCH rh7] x86: arch_free_page() introduced

[Stanislav Kinsburskiy]

17.11.2015 16:08, Vladimir Davydov пишет:
> On Tue, Nov 17, 2015 at 03:42:46PM +0100, Stanislav Kinsburskiy wrote:
>>
>> 17.11.2015 15:20, Vladimir Davydov пишет:
>>> On Thu, Nov 12, 2015 at 10:01:14PM +0400, Stanislav Kinsburskiy wrote:
>>>
>>>> @@ -0,0 +1,23 @@
>>>> +#ifndef _ASM_X86_FREE_PAGE_H
>>>> +#define _ASM_X86_FREE_PAGE_H
>>>> +
>>>> +#ifdef __KERNEL__
>>>> +
>>>> +#ifndef __ASSEMBLY__
>>>> +
>>>> +#include <linux/jump_label.h>
>>>> +
>>>> +#define HAVE_ARCH_FREE_PAGE
>>>> +
>>>> +extern struct static_key zero_free_pages;
>>>> +extern void do_zero_pages(struct page *page, int order);
>>>> +
>>>> +static __always_inline void arch_free_page(struct page *page, int order)
>>>> +{
>>>> +	if (static_key_false(&zero_free_pages))
>>>> +		do_zero_pages(page, order);
>>>> +}
>>> There is no point in making this feature arch-dependant now, as you use
>>> jump labels, which are arch-independent. The only reason why I had to
>>> overwrite arch_free_page in PCS6 is that I had to write some asm code to
>>> implement functionality close to that provided by jump labels, which are
>>> absent in RH6-based kernels. Please move it to mm/page_alloc.c.
>>>
>>> Thanks.
>> If we will make this function generic, then we have to declare
>> "zero_free_pages" as generic (which is more or less ok)  and define
>> do_zero_pages for the case, when HAVE_ARCH_FREE_PAGE is not defined. Does
>> the goal worth it?
>> If yes, then why?
>>
> I don't think I get your point. All I want you to do is something like
> this:
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f70c5f4da2a2..12126f212f3b 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -730,8 +730,11 @@ static bool free_pages_prepare(struct page *page, unsigned int order)
>   
>   	if (PageAnon(page))
>   		page->mapping = NULL;
> -	for (i = 0; i < (1 << order); i++)
> +	for (i = 0; i < (1 << order); i++) {
>   		bad += free_pages_check(page + i);
> +		if (static_key_false(&zero_free_pages))
> +			clear_highpage(page + i);
> +	}
>   	if (bad)
>   		return false;
>
> IMO this is better than messing around arch code.

Ok, I got your point.
I'm trying to follow a little bit different strategy, which is aimed to 
impact generic code as less, as possible. And from this point of view 
using already defined arch_free_page callback looks better to me. But I 
don't insist and can make it your way.
Should I?