[CRIU] [PATCH 1/6] Replace PAGE_SIZE in vaddr_to_pfn

[Pavel Emelyanov]

On 10/08/2014 04:43 PM, Cyrill Gorcunov wrote:
> On Wed, Oct 08, 2014 at 08:13:36AM -0400, Christopher Covington wrote:
>> On 10/07/2014 05:15 AM, Cyrill Gorcunov wrote:
>>> On Tue, Oct 07, 2014 at 01:06:54PM +0400, Pavel Emelyanov wrote:
>>>> On 10/06/2014 10:50 PM, Christopher Covington wrote:
>>>>> On AArch64, pages may be 4K or 64K depending on kernel configuration.
>>>>> Instead of using the build time constant PAGE_SIZE, query the page
>>>>> size at run time using sysconf(_SC_PAGESIZE).
>>>>
>>>> AFAIK on x86 the PAGE_SIZE is always 4k (Cyrill?).
>>>
>>> yes (on hw level it is not, but from software view PAGE_SIZE is always 4k)
>>>
>>>> If this is true, can we have the vaddr_to_pfn() still operate on
>>>> a constant rather than the _sysconf() call for x86?
>>>
>>> This kind of things should be wrapped with #ifdef depending on
>>> arch we're working on and for x86 it should remains 4k constant.
>>
>> If the difference between this being a libc call and a macro significantly
>> impacts performance then maybe I should explore a templated approach. I'm
>> thinking it should be possible to essentially build the code, or parts of it,
>> once with -D PAGE_SIZE 4096 and again with -D PAGE_SIZE 65536 and then run the
>> appropriate version based on a single initial call to sysconf(). This might
>> also lay the foundation for a single binary, both for ARM and x86, that can
>> dump both 32-bit and 64-bit programs. Would this be worth exploring?
> 
> Yeah, looks good to me. Even more -- in our build engine we can investigate
> the page size and move PAGE_SIZE into config.h (I suppose arm doesn't run
> kernel with different page sizes simultaneously, right?)
> 
> IOW we can do something (in Makefile.config)
> 
> ifeq ($(ARCH),arm)
> ifeq ($(call try-cc,$(PAGE_SIZE_TEST_65536),),y)
> 	$(Q) @echo '#define CONFIG_PAGE_SIZE 65536' >> $@
> elseif ($(call try-cc,$(PAGE_SIZE_TEST_4096),),y)
> 	$(Q) @echo '#define CONFIG_PAGE_SIZE 4096' >> $@
> endif
> endif
> 
> or something like that.

But the intention was exactly to avoid compile-time PAGE_SIZE, thus
direct call to _sysconf on ARM looks more acceptable here.

As far as the performance is concerned. In the current set I see no
problems having one as a call, but we may someday have the PAGE_SIZE
get involved into some for() loop and making one call per iteration
would just produce unnecessary noise.

Can we "cache" the value in kerndat.c and always use the variable?

Thanks,
Pavel