From 6bdde37049ba9fae31ccf83620468f77a6b9f396 Mon Sep 17 00:00:00 2001 From: Giampaolo Rodola Date: Thu, 17 Dec 2020 18:59:13 +0100 Subject: [PATCH] [macOS, UNIX] prefer _SC_PAGESIZE over (partially) deprecated getpagesize() (#1891) Add a reusable `psutil_getpagesize()` utility common to all UNIXes. Related to #1885 (`getpagesize()` is deprecated on recent macOS, POSIX.1-2001 and possibly other UNIXes). The problem emerged on macOS but `getpagesize()` is also used in FreeBSD, NetBSD, OpenBSD and AIX, so it makes sense to do this in one place only, similarly to Windows which also provide a `psutil_getpagesize()` utility. Follow cPython's `mmapmodule.c` and `resourcemodule.c` lead and rely on `sysconf(_SC_PAGESIZE)` instead, but leave `getpagesize()` in place as last resort/attempt for systems where it's not deprecated and/or they still legitimately rely on it. Also provide a python wrapper so we can test the return value of this C function against Python's stdlib modules. Signed-off-by: Giampaolo Rodola --- psutil/_psaix.py | 2 +- psutil/_psbsd.py | 5 +--- psutil/_pslinux.py | 2 +- psutil/_psosx.py | 2 +- psutil/_pssunos.py | 2 +- psutil/_psutil_aix.c | 4 +-- psutil/_psutil_bsd.c | 2 +- psutil/_psutil_osx.c | 12 +++------ psutil/_psutil_posix.c | 47 ++++++++++++++++++++++++++++++++++ psutil/_psutil_posix.h | 1 + psutil/arch/freebsd/specific.c | 10 ++------ psutil/arch/netbsd/specific.c | 4 +-- psutil/arch/openbsd/specific.c | 2 +- psutil/tests/test_bsd.py | 11 ++++---- psutil/tests/test_osx.py | 10 +++----- psutil/tests/test_posix.py | 16 ++++++++++++ psutil/tests/test_system.py | 8 ------ 17 files changed, 91 insertions(+), 49 deletions(-) diff --git a/psutil/_psaix.py b/psutil/_psaix.py index 57d5378ab..7160ecd63 100644 --- a/psutil/_psaix.py +++ b/psutil/_psaix.py @@ -46,7 +46,7 @@ HAS_NET_IO_COUNTERS = hasattr(cext, "net_io_counters") HAS_PROC_IO_COUNTERS = hasattr(cext, "proc_io_counters") -PAGE_SIZE = os.sysconf('SC_PAGE_SIZE') +PAGE_SIZE = cext_posix.getpagesize() AF_LINK = cext_posix.AF_LINK PROC_STATUSES = { diff --git a/psutil/_psbsd.py b/psutil/_psbsd.py index f0a0c1444..764463e98 100644 --- a/psutil/_psbsd.py +++ b/psutil/_psbsd.py @@ -98,10 +98,7 @@ cext.PSUTIL_CONN_NONE: _common.CONN_NONE, } -if NETBSD: - PAGESIZE = os.sysconf("SC_PAGESIZE") -else: - PAGESIZE = os.sysconf("SC_PAGE_SIZE") +PAGESIZE = cext_posix.getpagesize() AF_LINK = cext_posix.AF_LINK HAS_PER_CPU_TIMES = hasattr(cext, "per_cpu_times") diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py index ff13b5eb2..068c3f03b 100644 --- a/psutil/_pslinux.py +++ b/psutil/_pslinux.py @@ -81,7 +81,7 @@ # Number of clock ticks per second CLOCK_TICKS = os.sysconf("SC_CLK_TCK") -PAGESIZE = os.sysconf("SC_PAGE_SIZE") +PAGESIZE = cext_posix.getpagesize() BOOT_TIME = None # set later # Used when reading "big" files, namely /proc/{pid}/smaps and /proc/net/*. # On Python 2, using a buffer with open() for such files may result in a diff --git a/psutil/_psosx.py b/psutil/_psosx.py index be22b48d5..c7770d659 100644 --- a/psutil/_psosx.py +++ b/psutil/_psosx.py @@ -35,7 +35,7 @@ # ===================================================================== -PAGESIZE = os.sysconf("SC_PAGE_SIZE") +PAGESIZE = cext_posix.getpagesize() AF_LINK = cext_posix.AF_LINK TCP_STATUSES = { diff --git a/psutil/_pssunos.py b/psutil/_pssunos.py index 2af15dfa4..5618bd446 100644 --- a/psutil/_pssunos.py +++ b/psutil/_pssunos.py @@ -43,7 +43,7 @@ # ===================================================================== -PAGE_SIZE = os.sysconf('SC_PAGE_SIZE') +PAGE_SIZE = cext_posix.getpagesize() AF_LINK = cext_posix.AF_LINK IS_64_BIT = sys.maxsize > 2**32 diff --git a/psutil/_psutil_aix.c b/psutil/_psutil_aix.c index 791e831e5..a80bed70d 100644 --- a/psutil/_psutil_aix.c +++ b/psutil/_psutil_aix.c @@ -876,7 +876,7 @@ psutil_disk_io_counters(PyObject *self, PyObject *args) { static PyObject * psutil_virtual_mem(PyObject *self, PyObject *args) { int rc; - int pagesize = getpagesize(); + long pagesize = psutil_getpagesize(); perfstat_memory_total_t memory; rc = perfstat_memory_total( @@ -902,7 +902,7 @@ psutil_virtual_mem(PyObject *self, PyObject *args) { static PyObject * psutil_swap_mem(PyObject *self, PyObject *args) { int rc; - int pagesize = getpagesize(); + long pagesize = psutil_getpagesize(); perfstat_memory_total_t memory; rc = perfstat_memory_total( diff --git a/psutil/_psutil_bsd.c b/psutil/_psutil_bsd.c index 1956ff27f..15b646e33 100644 --- a/psutil/_psutil_bsd.c +++ b/psutil/_psutil_bsd.c @@ -189,7 +189,7 @@ psutil_proc_oneshot_info(PyObject *self, PyObject *args) { long memstack; int oncpu; kinfo_proc kp; - long pagesize = sysconf(_SC_PAGESIZE); + long pagesize = psutil_getpagesize(); char str[1000]; PyObject *py_name; PyObject *py_ppid; diff --git a/psutil/_psutil_osx.c b/psutil/_psutil_osx.c index 33d466357..13d0bb685 100644 --- a/psutil/_psutil_osx.c +++ b/psutil/_psutil_osx.c @@ -441,7 +441,7 @@ psutil_proc_memory_uss(PyObject *self, PyObject *args) { mach_vm_size_t size = 0; mach_msg_type_number_t info_count = VM_REGION_TOP_INFO_COUNT; kern_return_t kr; - vm_size_t page_size; + long pagesize = psutil_getpagesize(); mach_vm_address_t addr = MACH_VM_MIN_ADDRESS; mach_port_t task = MACH_PORT_NULL; vm_region_top_info_data_t info; @@ -505,11 +505,7 @@ psutil_proc_memory_uss(PyObject *self, PyObject *args) { } mach_port_deallocate(mach_task_self(), task); - - if (host_page_size(mach_host_self(), &page_size) != KERN_SUCCESS) - page_size = PAGE_SIZE; - - return Py_BuildValue("K", private_pages * page_size); + return Py_BuildValue("K", private_pages * pagesize); } @@ -525,7 +521,7 @@ psutil_virtual_mem(PyObject *self, PyObject *args) { uint64_t total; size_t len = sizeof(total); vm_statistics_data_t vm; - int pagesize = getpagesize(); + long pagesize = psutil_getpagesize(); // physical mem mib[0] = CTL_HW; mib[1] = HW_MEMSIZE; @@ -565,7 +561,7 @@ psutil_swap_mem(PyObject *self, PyObject *args) { size_t size; struct xsw_usage totals; vm_statistics_data_t vmstat; - int pagesize = getpagesize(); + long pagesize = psutil_getpagesize(); mib[0] = CTL_VM; mib[1] = VM_SWAPUSAGE; diff --git a/psutil/_psutil_posix.c b/psutil/_psutil_posix.c index b41c6dec9..305cec76d 100644 --- a/psutil/_psutil_posix.c +++ b/psutil/_psutil_posix.c @@ -15,6 +15,7 @@ #include #include #include +#include #ifdef PSUTIL_SUNOS10 #include "arch/solaris/v10/ifaddrs.h" @@ -50,6 +51,38 @@ #include "_psutil_common.h" + +// ==================================================================== +// --- Utils +// ==================================================================== + + +/* + * From "man getpagesize" on Linux, https://linux.die.net/man/2/getpagesize: + * + * > In SUSv2 the getpagesize() call is labeled LEGACY, and in POSIX.1-2001 + * > it has been dropped. + * > Portable applications should employ sysconf(_SC_PAGESIZE) instead + * > of getpagesize(). + * > Most systems allow the synonym _SC_PAGE_SIZE for _SC_PAGESIZE. + * > Whether getpagesize() is present as a Linux system call depends on the + * > architecture. + */ +long +psutil_getpagesize(void) { +#ifdef _SC_PAGESIZE + // recommended POSIX + return sysconf(_SC_PAGESIZE); +#elif _SC_PAGE_SIZE + // alias + return sysconf(_SC_PAGE_SIZE); +#else + // legacy + return (long) getpagesize(); +#endif +} + + /* * Check if PID exists. Return values: * 1: exists @@ -123,6 +156,18 @@ psutil_raise_for_pid(long pid, char *syscall) { } +// ==================================================================== +// --- Python wrappers +// ==================================================================== + + +// Exposed so we can test it against Python's stdlib. +static PyObject * +psutil_getpagesize_pywrapper(PyObject *self, PyObject *args) { + return Py_BuildValue("l", psutil_getpagesize()); +} + + /* * Given a PID return process priority as a Python integer. */ @@ -629,6 +674,8 @@ static PyMethodDef mod_methods[] = { "Retrieve NIC MTU"}, {"net_if_is_running", psutil_net_if_is_running, METH_VARARGS, "Return True if the NIC is running."}, + {"getpagesize", psutil_getpagesize_pywrapper, METH_VARARGS, + "Return memory page size."}, #if defined(PSUTIL_BSD) || defined(PSUTIL_OSX) {"net_if_duplex_speed", psutil_net_if_duplex_speed, METH_VARARGS, "Return NIC stats."}, diff --git a/psutil/_psutil_posix.h b/psutil/_psutil_posix.h index 59b9e5323..5a37e48b1 100644 --- a/psutil/_psutil_posix.h +++ b/psutil/_psutil_posix.h @@ -4,5 +4,6 @@ * found in the LICENSE file. */ +long psutil_getpagesize(void); int psutil_pid_exists(pid_t pid); void psutil_raise_for_pid(pid_t pid, char *msg); diff --git a/psutil/arch/freebsd/specific.c b/psutil/arch/freebsd/specific.c index e498a7b33..2776de8ce 100644 --- a/psutil/arch/freebsd/specific.c +++ b/psutil/arch/freebsd/specific.c @@ -404,7 +404,7 @@ psutil_virtual_mem(PyObject *self, PyObject *args) { size_t size = sizeof(total); struct vmtotal vm; int mib[] = {CTL_VM, VM_METER}; - long pagesize = getpagesize(); + long pagesize = psutil_getpagesize(); #if __FreeBSD_version > 702101 long buffers; #else @@ -465,7 +465,7 @@ psutil_swap_mem(PyObject *self, PyObject *args) { struct kvm_swap kvmsw[1]; unsigned int swapin, swapout, nodein, nodeout; size_t size = sizeof(unsigned int); - int pagesize; + long pagesize = psutil_getpagesize(); kd = kvm_open(NULL, _PATH_DEVNULL, NULL, O_RDONLY, "kvm_open failed"); if (kd == NULL) { @@ -499,12 +499,6 @@ psutil_swap_mem(PyObject *self, PyObject *args) { "sysctlbyname('vm.stats.vm.v_vnodeout)'"); } - pagesize = getpagesize(); - if (pagesize <= 0) { - PyErr_SetString(PyExc_ValueError, "invalid getpagesize()"); - return NULL; - } - return Py_BuildValue( "(KKKII)", (unsigned long long)kvmsw[0].ksw_total * pagesize, // total diff --git a/psutil/arch/netbsd/specific.c b/psutil/arch/netbsd/specific.c index 2fbc24187..4e286e5ee 100644 --- a/psutil/arch/netbsd/specific.c +++ b/psutil/arch/netbsd/specific.c @@ -445,7 +445,7 @@ psutil_virtual_mem(PyObject *self, PyObject *args) { size_t size; struct uvmexp_sysctl uv; int mib[] = {CTL_VM, VM_UVMEXP2}; - long pagesize = getpagesize(); + long pagesize = psutil_getpagesize(); size = sizeof(uv); if (sysctl(mib, 2, &uv, &size, NULL, 0) < 0) { @@ -472,6 +472,7 @@ psutil_swap_mem(PyObject *self, PyObject *args) { uint64_t swap_total, swap_free; struct swapent *swdev; int nswap, i; + long pagesize = psutil_getpagesize(); nswap = swapctl(SWAP_NSWAP, 0, 0); if (nswap == 0) { @@ -505,7 +506,6 @@ psutil_swap_mem(PyObject *self, PyObject *args) { size_t size = sizeof(total); struct uvmexp_sysctl uv; int mib[] = {CTL_VM, VM_UVMEXP2}; - long pagesize = getpagesize(); size = sizeof(uv); if (sysctl(mib, 2, &uv, &size, NULL, 0) < 0) { PyErr_SetFromErrno(PyExc_OSError); diff --git a/psutil/arch/openbsd/specific.c b/psutil/arch/openbsd/specific.c index aa4568f59..cdd4f6231 100644 --- a/psutil/arch/openbsd/specific.c +++ b/psutil/arch/openbsd/specific.c @@ -297,7 +297,7 @@ psutil_virtual_mem(PyObject *self, PyObject *args) { struct uvmexp uvmexp; struct bcachestats bcstats; struct vmtotal vmdata; - long pagesize = getpagesize(); + long pagesize = psutil_getpagesize(); size = sizeof(total_physmem); if (sysctl(physmem_mib, 2, &total_physmem, &size, NULL, 0) < 0) { diff --git a/psutil/tests/test_bsd.py b/psutil/tests/test_bsd.py index 06a65087a..b0bff87f6 100755 --- a/psutil/tests/test_bsd.py +++ b/psutil/tests/test_bsd.py @@ -32,12 +32,13 @@ if BSD: - PAGESIZE = os.sysconf("SC_PAGE_SIZE") - if os.getuid() == 0: # muse requires root privileges - MUSE_AVAILABLE = which('muse') - else: - MUSE_AVAILABLE = False + from psutil._psutil_posix import getpagesize + + PAGESIZE = getpagesize() + # muse requires root privileges + MUSE_AVAILABLE = True if os.getuid() == 0 and which('muse') else False else: + PAGESIZE = None MUSE_AVAILABLE = False diff --git a/psutil/tests/test_osx.py b/psutil/tests/test_osx.py index 097bff108..348976f88 100755 --- a/psutil/tests/test_osx.py +++ b/psutil/tests/test_osx.py @@ -4,9 +4,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -"""MACOS specific tests.""" +"""macOS specific tests.""" -import os import re import time @@ -23,9 +22,6 @@ from psutil.tests import unittest -PAGESIZE = os.sysconf("SC_PAGE_SIZE") if MACOS else None - - def sysctl(cmdline): """Expects a sysctl command with an argument and parse the result returning only the value of interest. @@ -40,13 +36,15 @@ def sysctl(cmdline): def vm_stat(field): """Wrapper around 'vm_stat' cmdline utility.""" + from psutil._psutil_posix import getpagesize + out = sh('vm_stat') for line in out.split('\n'): if field in line: break else: raise ValueError("line not found") - return int(re.search(r'\d+', line).group(0)) * PAGESIZE + return int(re.search(r'\d+', line).group(0)) * getpagesize() # http://code.activestate.com/recipes/578019/ diff --git a/psutil/tests/test_posix.py b/psutil/tests/test_posix.py index d9513eedb..acb6aa200 100755 --- a/psutil/tests/test_posix.py +++ b/psutil/tests/test_posix.py @@ -35,6 +35,12 @@ from psutil.tests import unittest from psutil.tests import which +if POSIX: + import mmap + import resource + + from psutil._psutil_posix import getpagesize + def ps(fmt, pid=None): """ @@ -404,6 +410,16 @@ def df(device): self.assertAlmostEqual(usage.percent, percent, delta=1) +@unittest.skipIf(not POSIX, "POSIX only") +class TestMisc(PsutilTestCase): + + def test_getpagesize(self): + pagesize = getpagesize() + self.assertGreater(pagesize, 0) + self.assertEqual(pagesize, resource.getpagesize()) + self.assertEqual(pagesize, mmap.PAGESIZE) + + if __name__ == '__main__': from psutil.tests.runner import run_from_name run_from_name(__file__) diff --git a/psutil/tests/test_system.py b/psutil/tests/test_system.py index 4da4e784d..90ecff945 100755 --- a/psutil/tests/test_system.py +++ b/psutil/tests/test_system.py @@ -218,14 +218,6 @@ def test_users(self): else: psutil.Process(user.pid) - @unittest.skipIf(not POSIX, 'POSIX only') - def test_PAGESIZE(self): - # pagesize is used internally to perform different calculations - # and it's determined by using SC_PAGE_SIZE; make sure - # getpagesize() returns the same value. - import resource - self.assertEqual(os.sysconf("SC_PAGE_SIZE"), resource.getpagesize()) - def test_test(self): # test for psutil.test() function stdout = sys.stdout