Use sysctlbyname() to get kernel page size.

Use sysctlbyname() to get kernel page size.

This allows enabling of COUNT_RESIDENT_BYTES_SUPPORTED tests.

It is also possible to change iOS' implementation of
base::GetPageSize() implementation to use sysctlbyname()
instead of getpagesize(), but since it is only the use of
mincore() that requires the use of kernel page size, it is
better to have a smaller change than a more invasive
change to base::GetPageSize().

I'm concerned that changing GetPageSize() to return 4KB
(what sysctlbyname returns) instead of 16KB (what getpagesize
returns) may have some unknown side effects.

BUG=542671
Committed: https://crrev.com/0b19e32e90d7affe4fcee8b2613392a1c20fdf0d
Cr-Commit-Position: refs/heads/master@{#381475}

[pkl (ping after 24h if needed), 2016-03-13 00:59:47]

Description was changed from

==========
Use sysctlbyname() to get kernel page size.

This allows re-enabling of COUNT_RESIDENT_BYTES_SUPPORTED tests.

BUG=542671
==========

to

==========
Use sysctlbyname() to get kernel page size.

This allows enabling of COUNT_RESIDENT_BYTES_SUPPORTED tests.

It is also possible to change iOS' implementation of
base::GetPageSize() implementation to use sysctlbyname()
instead of getpagesize(), but since it is only the use of
mincore() that requires the use of kernel page size, it is
better to have a smaller change than a more invasive 
change to base::GetPageSize().

I'm concerned that changing GetPageSize() to return 4KB
(what sysctlbyname returns) instead of 16KB (what getpagesize
returns) may have some unknown side effects.

BUG=542671
==========

[pkl (ping after 24h if needed), 2016-03-13 03:26:42]

pkl@chromium.org changed reviewers:
+ primiano@chromium.org

[pkl (ping after 24h if needed), 2016-03-13 03:26:42]

Hi, Primiano,

This fixes the test for iOS. The breakage was because iOS platform returns
different values for getpagesize() and sysctlbyname("vm.pagesize"). Based on a
Radar I filed w/ Apple, they said that sysctl result should be used for
calculating array size for mincore().

Peter.

[Primiano Tucci (use gerrit), 2016-03-15 15:50:49]

Many thanks for the investigation. All this is ... fun :)
I agree it's safer to keep this change local to this file.
Maybe you want to add a note to GetPageSize in base to warn about this bug ?

LGTM with minor comments. Sorry for the delay.

https://codereview.chromium.org/1793943002/diff/40001/base/trace_event/proces...
File base/trace_event/process_memory_dump.cc (right):

https://codereview.chromium.org/1793943002/diff/40001/base/trace_event/proces...
base/trace_event/process_memory_dump.cc:56: int status =
sysctlbyname("vm.pagesize", NULL, &pagesize_len, NULL, 0);
nit: s/NULL/nullptr
same below

https://codereview.chromium.org/1793943002/diff/40001/base/trace_event/proces...
base/trace_event/process_memory_dump.cc:58: if (sysctlbyname("vm.pagesize",
&pagesize, &pagesize_len, NULL, 0) == 0)
nit: consistency, either you check the syscall ret value with ! or == 0, both
work for me as long as you are consistent within 3 lines :)

[pkl (ping after 24h if needed), 2016-03-16 14:24:21]

Thank you for reviewing. Submitting.

https://codereview.chromium.org/1793943002/diff/40001/base/trace_event/proces...
File base/trace_event/process_memory_dump.cc (right):

https://codereview.chromium.org/1793943002/diff/40001/base/trace_event/proces...
base/trace_event/process_memory_dump.cc:56: int status =
sysctlbyname("vm.pagesize", NULL, &pagesize_len, NULL, 0);
On 2016/03/15 15:50:48, Primiano wrote:
> nit: s/NULL/nullptr
> same below

Done.

https://codereview.chromium.org/1793943002/diff/40001/base/trace_event/proces...
base/trace_event/process_memory_dump.cc:58: if (sysctlbyname("vm.pagesize",
&pagesize, &pagesize_len, NULL, 0) == 0)
On 2016/03/15 15:50:48, Primiano wrote:
> nit: consistency, either you check the syscall ret value with ! or == 0, both
> work for me as long as you are consistent within 3 lines :)

Yep, you're right. Thank you!

[pkl (ping after 24h if needed), 2016-03-16 14:24:28]

The CQ bit was checked by pkl@chromium.org

[pkl (ping after 24h if needed), 2016-03-16 14:24:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
primiano@chromium.org
Link to the patchset: https://codereview.chromium.org/1793943002/#ps60001
(title: "addressed primiano's comments")

[commit-bot: I haz the power, 2016-03-16 14:24:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1793943002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1793943002/60001

[commit-bot: I haz the power, 2016-03-16 15:29:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-16 15:29:02]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Primiano Tucci (use gerrit), 2016-03-16 16:08:51]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2016-03-16 16:09:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1793943002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1793943002/60001

[commit-bot: I haz the power, 2016-03-16 16:57:39]

Description was changed from

==========
Use sysctlbyname() to get kernel page size.

This allows enabling of COUNT_RESIDENT_BYTES_SUPPORTED tests.

It is also possible to change iOS' implementation of
base::GetPageSize() implementation to use sysctlbyname()
instead of getpagesize(), but since it is only the use of
mincore() that requires the use of kernel page size, it is
better to have a smaller change than a more invasive 
change to base::GetPageSize().

I'm concerned that changing GetPageSize() to return 4KB
(what sysctlbyname returns) instead of 16KB (what getpagesize
returns) may have some unknown side effects.

BUG=542671
==========

to

==========
Use sysctlbyname() to get kernel page size.

This allows enabling of COUNT_RESIDENT_BYTES_SUPPORTED tests.

It is also possible to change iOS' implementation of
base::GetPageSize() implementation to use sysctlbyname()
instead of getpagesize(), but since it is only the use of
mincore() that requires the use of kernel page size, it is
better to have a smaller change than a more invasive 
change to base::GetPageSize().

I'm concerned that changing GetPageSize() to return 4KB
(what sysctlbyname returns) instead of 16KB (what getpagesize
returns) may have some unknown side effects.

BUG=542671
==========

[commit-bot: I haz the power, 2016-03-16 16:57:40]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-16 16:59:48]

Description was changed from

==========
Use sysctlbyname() to get kernel page size.

This allows enabling of COUNT_RESIDENT_BYTES_SUPPORTED tests.

It is also possible to change iOS' implementation of
base::GetPageSize() implementation to use sysctlbyname()
instead of getpagesize(), but since it is only the use of
mincore() that requires the use of kernel page size, it is
better to have a smaller change than a more invasive 
change to base::GetPageSize().

I'm concerned that changing GetPageSize() to return 4KB
(what sysctlbyname returns) instead of 16KB (what getpagesize
returns) may have some unknown side effects.

BUG=542671
==========

to

==========
Use sysctlbyname() to get kernel page size.

This allows enabling of COUNT_RESIDENT_BYTES_SUPPORTED tests.

It is also possible to change iOS' implementation of
base::GetPageSize() implementation to use sysctlbyname()
instead of getpagesize(), but since it is only the use of
mincore() that requires the use of kernel page size, it is
better to have a smaller change than a more invasive
change to base::GetPageSize().

I'm concerned that changing GetPageSize() to return 4KB
(what sysctlbyname returns) instead of 16KB (what getpagesize
returns) may have some unknown side effects.

BUG=542671

Committed: https://crrev.com/0b19e32e90d7affe4fcee8b2613392a1c20fdf0d
Cr-Commit-Position: refs/heads/master@{#381475}
==========

[commit-bot: I haz the power, 2016-03-16 16:59:49]

Patchset 4 (id:??) landed as
https://crrev.com/0b19e32e90d7affe4fcee8b2613392a1c20fdf0d
Cr-Commit-Position: refs/heads/master@{#381475}