Date: Sun, 7 Jul 2013 08:25:53 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Benno Rice <benno@FreeBSD.org> Cc: freebsd-arch@freebsd.org Subject: Re: Reworking vmmeter Message-ID: <20130707052553.GC91021@kib.kiev.ua> In-Reply-To: <A4007ED2-FFD8-4003-8196-4E986CD96EC5@FreeBSD.org> References: <A4007ED2-FFD8-4003-8196-4E986CD96EC5@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--PN720ooTGWi/DZaq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jul 07, 2013 at 09:53:37AM +1000, Benno Rice wrote: > So I've put together this patch: >=20 > http://people.freebsd.org/~benno/vmmeter.diff >=20 > This patch does a few things: >=20 > - Renames the singleton "cnt" to "vmmeter". > - Replaces all the per-cpu counters with counter_u64_t. > - Removes the vmmeter instance from struct pcpu, due to the above mention= ed change. > - Adds includes for vmmeter.h to a few files that were only getting it vi= a pollution in pcpu.h > - Removes some entries from assym that weren't being used. >=20 > This has been tested on amd64 and nothing else right now, I'm more postin= g this to get general comments on whether people think this is a good idea.= My motivation for this was twofold, firstly to rename cnt and secondly to = move the counters to the common counter framework. More testing will be don= e prior to commit. >=20 Why did you removed the CTASSERT from sys/pcpu.h ? Your edits for the inlines in the sys/vmmeter.h are notoriously violating style. You probably could add some macro like COUNTER_U64_INITIALIZED(), which would check for the counter containing non-NULL pointer. At least this would allow to remove vmmeter_use_early_counters. Still the hack of early_*_faults cannot be avoided this way. Or, since BSP is guaranteed to have id 0, you could temporary put a pointer to the early_*_faults into the counter_u64_t, which is to be overwritten after the real counter is initialized. Then the if()s in the vm_fault() can be removed. Note that the parts of counters(9) KPI used in your patch has known issue on some 32 bit arches, namely powerpc32, mips32 and arm. The fetch could loose the carry bit in a cell, transiently. This is a bug in the platform implementation, and not the inherent counters(9) limitation. Fixing requires some asm magic (basically, the counter cells updates and fetches must be 64bit atomics). This is done on i386 already, and the problem does not exist on 64bit arches. --PN720ooTGWi/DZaq Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.20 (FreeBSD) iQIcBAEBAgAGBQJR2PvgAAoJEJDCuSvBvK1BhZAP/RUWBuFbDdpGbOzGTIx9i9Kf oUBLy+lG+xzK2ZKWhid/D/VP01FCeZ//ANvtD0o/Y/qGUBiu+IVH/VkfLt6bj3Wz 4f82z77SuhD7iuqvkHfjm4Lc2ASFq+K8aOpZoD5xLWGk+awamzMe8xf8t4OoX3hX S0e5zrEU/qHSmB/Z4X3/0VXdC/xPtmyNESYU3Thrzy4NMdKjzBGNm7NeyMUTT8Ha XIOxIrXmvVl7maG0oH3Re1QXlT5eKRY6tr3Y0/AJBVuTg96HJu5ghF9UgmLCQavF T4dT3hhvdD96XcXN3ASXE7A2lJhHjB9+RS6WL2A6y9vy7YOIdv8YZ5VU5POaQUUx nPa/KPU6gaCJfZnZv2qwroSl16b6w/uwTRrH/oEEKMiPc8GZS0wvvDlKwqy8hq3P dVU6N2knAr3UCd085grKuszH5V8aJ9J1m0oAgh4EI53J0ctM1ViZH9gXrTWVGf3X nshngembQKboX6SsO8QsVBLOBnnVfvMF1Q9LqjvQdDN3O4+ZWuqU2DUW2Ql10XfO qiTafbgdtWUVxVgxOqXqq1yy3ieZJ9EG+8wI133R4yH1Aej57Ifjb0ZCMUA69zSr FO+taweaJPTZq6MWeH0x3D8BZgj83/PftcDZuS7EJKBjC4u8oAekYCio3YRNqant EQ4hyWfLdV3oUxLW3YEQ =lWmE -----END PGP SIGNATURE----- --PN720ooTGWi/DZaq--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130707052553.GC91021>