svn commit: r348117 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Alexander Motin]

On 22.05.2019 16:34, Slawa Olhovchenkov wrote:
> On Wed, May 22, 2019 at 04:00:58PM -0400, Alexander Motin wrote:
> 
>> On 22.05.2019 15:49, Slawa Olhovchenkov wrote:
>>> On Wed, May 22, 2019 at 06:43:48PM +0000, Alexander Motin wrote:
>>>> Author: mav
>>>> Date: Wed May 22 18:43:48 2019
>>>> New Revision: 348117
>>>> URL: https://svnweb.freebsd.org/changeset/base/348117
>>>>
>>>> Log:
>>>>   Allocate buffers smaller then ABD chunk size as linear.
>>>>   
>>>>   This allows to reduce memory waste by letting UMA to put multiple small
>>>>   buffers into one memory page slab.  The page sharing means that UMA
>>>>   may not be able to free memory page when some of buffers are freed, but
>>>>   alternatively memory used by that buffer would just be wasted from the
>>>>   beginning.
>>>>   
>>>>   This change follows alike change in ZoL, but unlike Linux (according to
>>>>   my understanding of it from comments) FreeBSD never shares slabs bigger
>>>>   then one memory page, so this should be even less invasive then there.
>>>>   
>>>>   MFC after:	2 weeks
>>>>   Sponsored by:	iXsystems, Inc.
>>>>
>>>> Modified:
>>>>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/abd.c
>>>>
>>>> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/abd.c
>>>> ==============================================================================
>>>> --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/abd.c	Wed May 22 17:42:22 2019	(r348116)
>>>> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/abd.c	Wed May 22 18:43:48 2019	(r348117)
>>>> @@ -290,7 +290,7 @@ abd_free_struct(abd_t *abd)
>>>>  abd_t *
>>>>  abd_alloc(size_t size, boolean_t is_metadata)
>>>>  {
>>>> -	if (!zfs_abd_scatter_enabled)
>>>> +	if (!zfs_abd_scatter_enabled || size <= zfs_abd_chunk_size)
>>>>  		return (abd_alloc_linear(size, is_metadata));
>>>
>>> may be `size < zfs_abd_chunk_size`?
>>
>> Why?  It should be the same from memory usage, but I see theoretical
>> benefits from having linear buffer with one page rather then scatter
>> list with one page.  Plus ZoL also had there `size <= PAGESIZE`.
> 
> May be malloc() for page size more effictive? Not sure.
> Less overhead for kegs and etc.

malloc()'s up to 64KB are also backed by UMA.  May be it could be
optimized on platforms with direct map, and may be after 12 VM subsystem
would not be too shocked, but on platforms without direct map requests
going around UMA causes huge TLB shootdown on free().

Thinking again, I see one benefit of allocating 4KB buffers not as
linear but as ABD -- using same ABD UMA zone for both 4KB and above
should improve its caching efficiency.  On the other side I already saw
lock congestion on UMA locks there, so there may be trade-off.

-- 
Alexander Motin