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

[Christopher Covington]

On 10/08/2014 09:29 AM, Cyrill Gorcunov wrote:
> On Wed, Oct 08, 2014 at 09:11:46AM -0400, Christopher Covington wrote:
>>>>
>>>> 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.
> 
> Guys, guys, wait. Look, we have a generic definitions for PAGE_SIZE in
> include/asm-generic/page.h
> 
> #ifndef __CR_ASM_GENERIC_PAGE_H__
> #define __CR_ASM_GENERIC_PAGE_H__
> 
> #ifndef PAGE_SHIFT
> # define PAGE_SHIFT12
> #endif
> 
> #ifndef PAGE_SIZE
> # define PAGE_SIZE(1UL << PAGE_SHIFT)
> #endif
> 
> #ifndef PAGE_MASK
> # define PAGE_MASK(~(PAGE_SIZE - 1))
> #endif
> 
> #define PAGE_PFN(addr)addr((addr) / PAGE_SIZE)
> 
> #endif /* __CR_ASM_GENERIC_PAGE_H__ */
> 
> Why don't
> 
> #include "asm/page.h"
> 
> here and redefine PAGE_SIZE/SHIT and such there. IOW,
> for ARM it could be
> 
> arch/arm/include/asm/page.h
> 
> #define PAGE_SIZE (sysconf(_PAGE_SIZE)))
> 
> or something like that. SO we don't have to change PAGE_SIZE all over
> the criu code at all, but in one place only. Hmm? What do you think?

I did try that and I'd like to end up with something mostly equivalent.
However, some existing code assumes PAGE_SIZE is constant, using it for
compile-time buffer sizes. My instinct is to not turn all of these into
run-time allocations.

(As a brief aside, I it's a reasonable assumption to think PAGE_SIZE or
ANY_MACRO_REALLY is a compile-time constant. In my opinion, using a function
such as sysconf(_SC_PAGESIZE) or lowercase variable name such as page_size
best communicates to those reading and writing the common code that the value
cannot be relied upon to be a compile time constant across all systems.)

Here are the different possible uses of PAGE_SIZE and corresponding
replacements that I came up with:

PAGE_OR_LESS -- (local) compile-time constant minimum page size, guaranteed to
be less than or equal to one page but not necessarily page aligned

EXEC_PAGESIZE -- (libc) compile-time constant maximum page size, guaranteed to
be greater than or equal to one page and page aligned

sysconf(_SC_PAGESIZE) or page_size -- (libc or local) run-time determined
value of the current page size

(And maybe also something like BUFFER_SIZE, if I run into uses that really
have no functional or performance connection to the page size:
http://lists.busybox.net/pipermail/busybox/2011-June/075864.html)

Thanks,
Christopher

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