svn commit: r310023 - in head/sys/cddl: compat/opensolaris/sys contrib/opensolaris/uts/common/fs/zfs

[Andriy Gapon]

On 13/12/2016 18:20, Alexander Motin wrote:
> Author: mav
> Date: Tue Dec 13 16:20:10 2016
> New Revision: 310023
> URL: https://svnweb.freebsd.org/changeset/base/310023
> 
> Log:
>   Reduce diff from Illumos by better variables mapping.
> 
> Modified:
>   head/sys/cddl/compat/opensolaris/sys/kmem.h
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
> 
> Modified: head/sys/cddl/compat/opensolaris/sys/kmem.h
> ==============================================================================
> --- head/sys/cddl/compat/opensolaris/sys/kmem.h	Tue Dec 13 13:46:09 2016	(r310022)
> +++ head/sys/cddl/compat/opensolaris/sys/kmem.h	Tue Dec 13 16:20:10 2016	(r310023)
> @@ -37,6 +37,7 @@
>  #include <vm/uma.h>
>  #include <vm/vm.h>
>  #include <vm/vm_extern.h>
> +#include <vm/vm_pageout.h>
>  
>  MALLOC_DECLARE(M_SOLARIS);
>  
> @@ -77,8 +78,10 @@ void kmem_reap(void);
>  int kmem_debugging(void);
>  void *calloc(size_t n, size_t s);
>  
> -#define	freemem				vm_cnt.v_free_count
> -#define	minfree				vm_cnt.v_free_min
> +#define	freemem				(long)vm_cnt.v_free_count
> +#define	desfree				(long)vm_cnt.v_free_target
> +#define	minfree				(long)vm_cnt.v_free_min
> +#define	needfree			(long)vm_pageout_deficit
>  #define	heap_arena			kmem_arena
>  #define	kmem_alloc(size, kmflags)	zfs_kmem_alloc((size), (kmflags))
>  #define	kmem_zalloc(size, kmflags)	zfs_kmem_alloc((size), (kmflags) | M_ZERO)
> 
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
> ==============================================================================
> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c	Tue Dec 13 13:46:09 2016	(r310022)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c	Tue Dec 13 16:20:10 2016	(r310023)
> @@ -357,6 +357,7 @@ int zfs_arc_shrink_shift = 0;
>  int zfs_arc_p_min_shift = 0;
>  uint64_t zfs_arc_average_blocksize = 8 * 1024; /* 8KB */
>  u_int zfs_arc_free_target = 0;
> +#define	lotsfree	zfs_arc_free_target
>  
>  /* Absolute min for arc min / max is 16MB. */
>  static uint64_t arc_abs_min = 16 << 20;
> @@ -3827,8 +3828,6 @@ arc_shrink(int64_t to_free)
>  	}
>  }
>  
> -static long needfree = 0;
> -
>  typedef enum free_memory_reason_t {
>  	FMR_UNKNOWN,
>  	FMR_NEEDFREE,
> @@ -3875,17 +3874,6 @@ arc_available_memory(void)
>  	}
>  
>  	/*
> -	 * Cooperate with pagedaemon when it's time for it to scan
> -	 * and reclaim some pages.
> -	 */
> -	n = PAGESIZE * ((int64_t)freemem - zfs_arc_free_target);
> -	if (n < lowest) {
> -		lowest = n;
> -		r = FMR_LOTSFREE;
> -	}
> -
> -#ifdef illumos
> -	/*
>  	 * check that we're out of range of the pageout scanner.  It starts to
>  	 * schedule paging if freemem is less than lotsfree and needfree.
>  	 * lotsfree is the high-water mark for pageout, and needfree is the
> @@ -3898,6 +3886,7 @@ arc_available_memory(void)
>  		r = FMR_LOTSFREE;
>  	}
>  
> +#ifdef illumos
>  	/*
>  	 * check to make sure that swapfs has enough space so that anon
>  	 * reservations can still succeed. anon_resvmem() checks that the
> @@ -4154,9 +4143,6 @@ arc_reclaim_thread(void *dummy __unused)
>  		 * infinite loop.
>  		 */
>  		if (arc_size <= arc_c || evicted == 0) {
> -#

Alexander,

I tried to see how the code looks after expanding illumos variables to FreeBSD
variables before and after your change.

Before:
n = PAGESIZE * ((int64_t)freemem - zfs_arc_free_target);
==>
n = PAGESIZE * ((int64_t)vm_cnt.v_free_count - zfs_arc_free_target);

After:
n = PAGESIZE * (freemem - lotsfree - needfree - desfree);
==>
n = PAGESIZE * ((long)vm_cnt.v_free_count - zfs_arc_free_target -
(long)vm_pageout_deficit - (long)vm_cnt.v_free_target);

Default value of zfs_arc_free_target is vm_pageout_wakeup_thresh and its default
value is (vm_cnt.v_free_min / 10) * 11.

vm_pageout_deficit is probably just a noise most of the time.

But v_free_target is a substantial value (even greater than the default
zfs_arc_free_target).

It seems that now we subtract much more than we did before.
So, this change does not merely reduce diff, it also changes ARC sizing behavior.

I wonder how much testing have you done for this change and if you can qualify
ARC size behavior in various scenarios.  I expect that with your change the ARC
would more easily give in to the memory pressure.  That may delight some, but it
could be an issue for others.  Especially if it forces ARC to its minimum size
for no good reason.

P.S.
My impression is that the page daemon in illumos has a different algorithm from
our page daemon, so some similarities could be misleading rather than helpful.

-- 
Andriy Gapon