Exposing Zone Sleeps

[Garrett Cooper]

On Tue, May 25, 2010 at 11:26 AM, jhell <jhell at dataix.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 05/25/2010 10:01, jhell wrote:
>> On 05/24/2010 18:26, Sean Bruno wrote:
>>> Find attached a patch against -CURRENT.
>>
>>> This update exposes a counter that indicates the number of times that we
>>> sleep when attempting to allocate a slab from the keg.  In other words,
>>> the number of times we BLOCK and wait, which is bad.
>>
>>> This allows differentiation between times when we failed to allocate and
>>> it was ok and times where we were forced to sleep.  The current FAIL
>>> counter does not make this distinction.
>>
>>> Exposes this information via uma_zone_t->uz_sleeps.
>>
>>> Add a new sysctl to retrieve this information.
>>> Enhance vmstat -z to retrieve this information.
>>
>>> We've found this *extremely* useful here at Yahoo in the past and would
>>> like to commit this if it is acceptable.
>>
>>> Tested on 32bit and 64bit architectures on 6/7/CURRENT.
>>
>>
>> Hi Sean,
>>
>> Nice work on this. I applied this to stable/8 r208530 and I am in the
>> process of compiling the kernel right now. Everything else has built &
>> runs as expected "i386". Attached is the adjusted patch which was one
>> modification to the line number for uz_sleeps in sys/vm/uma_int.h.
>>
>> 8 files changed, 106 insertions(+), 7 deletions(-)
>>
>> For those wishing to apply this patch and test for them self:
>>
>> cd /usr/src
>> patch </path/to/sleep_stat_stable8_r208530.diff
>> cd /usr/src/include
>> make obj && make depend && make includes && make install
>> cd /usr/src/lib/libmemstat
>> make obj && make depend && make includes && make install
>> cd /usr/src/usr.bin/vmstat
>> make obj && make depend && make install
>> cd /usr/src
>> make kernel KERNCONF=YOUR_KERN_CONF
>> reboot
>>
>> Can't wait to see some results from this & I will report back with
>> either negative results of the build & run or positive results from the
>> stats collected.
>>
>> If there is anything needed feel free to let me know and I will do what
>> is possible ASAP.
>>
>> Thanks again,
>>
>
> This patch instead pardon the early.post but there was a problem with
> the last patch that I attached for stable/8 r208530 with arguments 10 &
> 11 to function sysctl_vm_zone where it wanted a long unsigned integer
> rather than u_int64_t.
>
> This patch satisfies that. Whether its correct is left to the reader but
> compiles cleanly & runs smoothly.

I know this seems trivial, but could you change:

+	printf("%-20s %6s %6s %8s %8s %8s %4s %4s\n\n", "ITEM", "SIZE",
+	    "LIMIT", "USED", "FREE", "REQ", "FAIL", "SLEEP");

to

+	printf("%-20s %6s %6s %8s %8s %8s %4s %4s\n\n", "ITEM", "SIZE",
+	    "LIMIT", "USED", "FREE", "REQS", "FAIL", "SLEEP");

that way the plural nature of requests is more straightforward and understood.

Also, do all of the fields _really_ need to have a field width? Seems
like overkill to me...

Thanks,
-Garrett