[CRIU] [PATCH 2/2] include/common: Default to build host's page size

[cov at codeaurora.org]

Hi Dmitry,

On 2016-11-02 14:29, Dmitry Safonov wrote:
> 2016-11-02 20:15 GMT+03:00 Christopher Covington <cov at codeaurora.org>:
>> On PowerPC or arm64, different distributions may configure their
>> kernel to provide different page sizes (4k versus 64k for example).
>> CRIU must currently be built for a specific page size. Default to
>> using the build system's page size, to make native building work by
>> default. For cross-compiling or other situations, the default can
>> be overriden like so:
>> 
>>   rm criu/include/config.h
>>   make PAGE_SIZE=65536
>> 
>> Signed-off-by: Christopher Covington <cov at codeaurora.org>
> 
> Hi,
> 
> I have a dumb question :)
> Why do we have PAGE_SIZE constant *and* page_size() func?

Statically allocated buffers/arrays need a size and the usual choice
appears to be PAGE_SIZE. We could call it PAGE_SIZE_MIN or BUF_SIZE or
whatever. Or we could dynamically allocate all the arrays.

Buffers/arrays used in several system calls must have actual run time 
page
size alignment. Or, assuming the page size options are multiples of each
other, we could use a PAGE_SIZE_MAX #define.

I think there other cases like values in /proc where the actual run time
page size must be used with no hard-coded alternative available as far
as I can tell.

I've tried and failed in the past to categorize all the PAGE_SIZE usage
into the above three PAGE_SIZE_MIN, PAGE_SIZE_MAX, and page_size() 
cases.

Instead, for years I've just hobbled along building once for 4K and once
for 64K (with a local change removing the BUILD_BUG_ON).

This is an attempt to at least get upstream to the level of 
functionality
I'm at locally--the ability to at least build a 64K CRIU for arm64. I
unfortunately am not sure that I'll be able to accomplish the original
proper categorization approach any time soon.

> I mean, I don't completely get why in some places we use one over
> the other alternative. I see two cons for using page_size() everywhere:
> 1. perf cost for calling sysconf()
> 2. using PAGE_SIZE in BUILD_BUG_ON macros.
> 
> So, I guess, if we introduce some kdat variable, that will get 
> initialized
> with sysconf() and use it everywhere, we will solve (1) problem and
> even gain some free perf in places where page_size() is used now.

I've looked at how glibc caches the value, and it seems pretty optimal. 
I
don't have profile information handy to calculate whether the library 
call
overhead is significant or negligible.

> For (2), we can rename PAGE_SIZE to PAGE_SIZE_MIN or something,
> so we keep all BUILD_BUG_ONs in places and review, where we
> could use kdat var instead. Maybe even if there is a chance to rewrite
> those build-checks without arch page size, we could remove this
> macro completely.

What BUILD_BUG_ON macro would you like to keep? The one I've removed
prevents users from building CRIU in a manner that is compatible with
several perfectly valid kernel configurations (4K, 16K, 256K pages on
ppc64, 16K, 64K pages on arm64). And this isn't just theoretical, 
Fedora,
Red Hat, and CentOS for arm64 use 64k pages. Debian and Ubuntu for arm64
use 4k pages.

> With kdat var we also will fix page_size() on platforms where is just
> returns PAGE_SIZE -- AFAIK, x86 also has optional huge pages.

As far as I know, in terms of basic functionality, userspace doesn't
care whether huge pages are being used or not. I don't think you ever
have to align system call buffers to huge page size, for example. 
Perhaps
there are performance considerations, however.

Thanks,
Cov