svn commit: r312724 - in head/sys: sys vm

[Bruce Evans]

On Tue, 24 Jan 2017, Mateusz Guzik wrote:

> Log:
>  hwpmc: partially depessimize munmap handling if the module is not loaded
>
>  HWPMC_HOOKS is enabled in GENERIC and triggers some work avoidable in the
>  common (module not loaded) case.
> ...
> Modified: head/sys/sys/pmckern.h
> ==============================================================================
> --- head/sys/sys/pmckern.h	Tue Jan 24 21:48:57 2017	(r312723)
> +++ head/sys/sys/pmckern.h	Tue Jan 24 22:00:16 2017	(r312724)
> @@ -174,6 +174,9 @@ extern const int pmc_kernel_version;
> /* PMC soft per cpu trapframe */
> extern struct trapframe pmc_tf[MAXCPU];
>
> +/* Quick check if preparatory work is necessary */
> +#define	PMC_HOOK_INSTALLED(cmd)	__predict_false(pmc_hook != NULL)

I'm still waiting for other __predict_ugly() macro invocations to be
removed.

The 2 new ones here even less effect than most.  I couldn't measure the
effect on makeworld of removing the PMC_HOOKS completely.  Removing KTRACE
long ago also seemed to have no effect.  Unfortunately, it is impossible
to remove procctl and other bloat that has grown in the syscall path, and
is easy to measure slowdowns from this.

The above one is an even better obfuscation than most.  It is only invoked
once, and it is context-dependent whether to false branch is the unusual
case.

> Modified: head/sys/vm/vm_mmap.c
> ==============================================================================
> --- head/sys/vm/vm_mmap.c	Tue Jan 24 21:48:57 2017	(r312723)
> +++ head/sys/vm/vm_mmap.c	Tue Jan 24 22:00:16 2017	(r312724)
> @@ -526,6 +526,7 @@ sys_munmap(td, uap)
> #ifdef HWPMC_HOOKS
> 	struct pmckern_map_out pkm;
> 	vm_map_entry_t entry;
> +	bool pmc_handled;
> #endif
> 	vm_offset_t addr;
> 	vm_size_t size, pageoff;
> @@ -551,20 +552,24 @@ sys_munmap(td, uap)
> 		return (EINVAL);
> 	vm_map_lock(map);
> #ifdef HWPMC_HOOKS
> -	/*
> -	 * Inform hwpmc if the address range being unmapped contains
> -	 * an executable region.
> -	 */
> -	pkm.pm_address = (uintptr_t) NULL;
> -	if (vm_map_lookup_entry(map, addr, &entry)) {
> -		for (;
> -		     entry != &map->header && entry->start < addr + size;
> -		     entry = entry->next) {
> -			if (vm_map_check_protection(map, entry->start,
> -				entry->end, VM_PROT_EXECUTE) == TRUE) {
> -				pkm.pm_address = (uintptr_t) addr;
> -				pkm.pm_size = (size_t) size;
> -				break;
> +	pmc_handled = false;
> +	if (PMC_HOOK_INSTALLED(PMC_FN_MUNMAP)) {
> +		pmc_handled = true;
> +		/*
> +		 * Inform hwpmc if the address range being unmapped contains
> +		 * an executable region.
> +		 */
> +		pkm.pm_address = (uintptr_t) NULL;
> +		if (vm_map_lookup_entry(map, addr, &entry)) {
> +			for (;
> +			    entry != &map->header && entry->start < addr + size;
> +			    entry = entry->next) {
> +				if (vm_map_check_protection(map, entry->start,
> +					entry->end, VM_PROT_EXECUTE) == TRUE) {
> +					pkm.pm_address = (uintptr_t) addr;
> +					pkm.pm_size = (size_t) size;
> +					break;
> +				}
> 			}
> 		}
> 	}

Predictions could also be implemented in a more hard-coded way using
'goto slowcase' and moving the slow case out of the way (assuming that
that the hardware predicts forward branches as not taken and the
compiler doesn't reorder the code anyway).  This would be uglier, but
not much more invasive than re-indenting the code after adding a test
for the slow case.

Bruce