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

[Christopher Covington]

On 10/08/2014 09:01 AM, Pavel Emelyanov wrote:
> 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?

Sure, for v2 I'll try out page_size and task_size variables in kerndat.c.

Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.