Date: Mon, 29 Sep 2014 11:27:16 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-hackers@freebsd.org Cc: jeff@freebsd.org, Bryan Venteicher <bryanv@daemoninthecloset.org> Subject: Re: Change uma_mtx to rwlock Message-ID: <1458140.gGPpU3NGiG@ralph.baldwin.cx> In-Reply-To: <CAMo0n6Q=P5H3%2BCqr8KjFRVLvWHZfJYnROVe4xF3DmKw95D%2B5zQ@mail.gmail.com> References: <CAMo0n6Q=P5H3%2BCqr8KjFRVLvWHZfJYnROVe4xF3DmKw95D%2B5zQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday, September 27, 2014 07:59:47 PM Bryan Venteicher wrote:
> Hi,
>
> I'd appreciate some comments attached patch that changes the uma_mtx to a
> rwlock.
>
> At $JOB, we have machines with ~400GB RAM, with much of that being
> allocated through UMA zones. We've observed that timeouts were sometimes
> unexpectedly delayed by a half second or more. We tracked one of the
> reasons for this down to when the page daemon was running, calling
> uma_reclaim() -> zone_foreach(). zone_foreach() holds the uma_mtx while
> zone_drain()'ing each zone. If uma_timeout() fires, it will block on the
> uma_mtx when it tries to zone_timeout() each zone.
The only nit I see is in zone_drain_wait(). It would be nice to not need the
hack of checking for a read or write lock and just require the one it actually
needs depending on the callers.
However, checking the code in HEAD, this appears to just be broken.
Specifically, zone_drain_wait() is called in two places:
void
zone_drain(uma_zone_t zone)
{
zone_drain_wait(zone, M_NOWAIT);
}
...
static void
zone_dtor(void *arg, int size, void *udata)
{
...
mtx_lock(&uma_mtx);
LIST_REMOVE(zone, uz_link);
mtx_unlock(&uma_mtx);
/*
* XXX there are some races here where
* the zone can be drained but zone lock
* released and then refilled before we
* remove it... we dont care for now
*/
zone_drain_wait(zone, M_WAITOK);
...
}
Neither one calls it with the uma_mtx locked! This appears to have been
broken since that function was introduced in r187681.
I think it might be best to first remove the unlock/lock of uma_mtx from
zone_drain_wait() (so it can be MFC'd). That then simplifies that one part of
your patch (which I think is otherwise fine).
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1458140.gGPpU3NGiG>