Date: Wed, 26 May 2010 11:52:51 -0700 From: Garrett Cooper <yanefbsd@gmail.com> To: jhell <jhell@dataix.net> Cc: freebsd-hackers <freebsd-hackers@freebsd.org>, sbruno@freebsd.org, Sean Bruno <seanbru@yahoo-inc.com> Subject: Re: Exposing Zone Sleeps Message-ID: <AANLkTikfzFFNSg0ZqYmsmobpkCxcRsS0rx9HYJ_bCvWK@mail.gmail.com> In-Reply-To: <AANLkTimkvxtzMWuvfOLWy5SKQH9kTUow0F2I5nJtGL48@mail.gmail.com> References: <1274739973.31299.23.camel@localhost.localdomain> <4BFBD838.40208@dataix.net> <4BFC1660.1000405@dataix.net> <AANLkTimkvxtzMWuvfOLWy5SKQH9kTUow0F2I5nJtGL48@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, May 26, 2010 at 11:51 AM, Garrett Cooper <yanefbsd@gmail.com> wrote=
:
> On Tue, May 25, 2010 at 11:26 AM, jhell <jhell@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. =A0In other wor=
ds,
>>>> the number of times we BLOCK and wait, which is bad.
>>>
>>>> This allows differentiation between times when we failed to allocate a=
nd
>>>> it was ok and times where we were forced to sleep. =A0The 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 woul=
d
>>>> 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=3DYOUR_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:
>
> + =A0 =A0 =A0 printf("%-20s %6s %6s %8s %8s %8s %4s %4s\n\n", "ITEM", "SI=
ZE",
> + =A0 =A0 =A0 =A0 =A0 "LIMIT", "USED", "FREE", "REQ", "FAIL", "SLEEP");
>
> to
>
> + =A0 =A0 =A0 printf("%-20s %6s %6s %8s %8s %8s %4s %4s\n\n", "ITEM", "SI=
ZE",
> + =A0 =A0 =A0 =A0 =A0 "LIMIT", "USED", "FREE", "REQS", "FAIL", "SLEEP");
>
> that way the plural nature of requests is more straightforward and unders=
tood.
>
> Also, do all of the fields _really_ need to have a field width? Seems
> like overkill to me...
Oh, and the field width for the last item is wrong; SLEEP will be
truncated to SLEE.
Thanks,
-Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTikfzFFNSg0ZqYmsmobpkCxcRsS0rx9HYJ_bCvWK>