From owner-svn-src-all@FreeBSD.ORG Wed Mar 5 23:29:57 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 81DB37FB; Wed, 5 Mar 2014 23:29:57 +0000 (UTC) Received: from mail-qg0-x233.google.com (mail-qg0-x233.google.com [IPv6:2607:f8b0:400d:c04::233]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 078E7C91; Wed, 5 Mar 2014 23:29:56 +0000 (UTC) Received: by mail-qg0-f51.google.com with SMTP id q108so4939468qgd.10 for ; Wed, 05 Mar 2014 15:29:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=69o/hqxwIzE7CsJzt9nYyffz2xx5wfAEa0OiGMQ48lo=; b=I9m7p/l54kHjTTsrapI9EWnVhaxdCuOpNCo+rC6HTqMHbasc8BTtMi7lUVsznBlzT/ BzB/wpke6I1a2uRHvcWif4MqR0WZLTAw+/0R+HY7fVKA2bCgAzqN93dZgKEqLfRDfdsl 1d94Zd5ds/8hHO2SJXhypeFa0syttDiD/doaXIW7x8Pzi5w/rPq1uMGS2i8VAQhahv2U iCjmXdUZmx4f5VKbJIs2RH1zt3myYvVvuj0iO4DSvd2yBmZHcGKvYFzWBwuxVoYzkEeA zmG5p97PsZYQSoUoGFcPY/50jSSxPtkCQewtswmxkeUkgRrEDcrDJUWTwUms6kEEMF/y bUqw== MIME-Version: 1.0 X-Received: by 10.224.11.136 with SMTP id t8mr10469327qat.26.1394062196018; Wed, 05 Mar 2014 15:29:56 -0800 (PST) Sender: adrian.chadd@gmail.com Received: by 10.224.8.137 with HTTP; Wed, 5 Mar 2014 15:29:55 -0800 (PST) In-Reply-To: <201403052116.s25LGkEq007924@svn.freebsd.org> References: <201403052116.s25LGkEq007924@svn.freebsd.org> Date: Wed, 5 Mar 2014 15:29:55 -0800 X-Google-Sender-Auth: F0rQRVBEPVIxuFXmnfEit1CNi6o Message-ID: Subject: Re: svn commit: r262806 - head/sys/net From: Adrian Chadd To: Gleb Smirnoff Content-Type: text/plain; charset=ISO-8859-1 Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Mar 2014 23:29:57 -0000 ... why's the code returning locked mutexes to UMA? Why not fix the places that are doing this and doing a lock assertion in the destructor? What's this buy us? I'm very wary of design patterns like this that do conditional unlocking in free/destructor routines; it allows for the caller to not necessarily get all the lock/unlock stuff to line up in the main code and it can make debugging more of a pain. Thanks, -a On 5 March 2014 13:16, Gleb Smirnoff wrote: > Author: glebius > Date: Wed Mar 5 21:16:46 2014 > New Revision: 262806 > URL: http://svnweb.freebsd.org/changeset/base/262806 > > Log: > The route code used to mtx_destroy() a locked mutex before rtentry free. Now, > after r262763 it started to return locked mutexes to UMA. To fix that, > conditionally unlock the mutex in the destructor. > > Tested by: "Sergey V. Dyatko" > > Modified: > head/sys/net/route.c > head/sys/net/route.h > > Modified: head/sys/net/route.c > ============================================================================== > --- head/sys/net/route.c Wed Mar 5 20:01:04 2014 (r262805) > +++ head/sys/net/route.c Wed Mar 5 21:16:46 2014 (r262806) > @@ -237,6 +237,14 @@ rtentry_ctor(void *mem, int size, void * > } > > static void > +rtentry_dtor(void *mem, int size, void *arg) > +{ > + struct rtentry *rt = mem; > + > + RT_UNLOCK_COND(rt); > +} > + > +static void > vnet_route_init(const void *unused __unused) > { > struct domain *dom; > @@ -248,7 +256,7 @@ vnet_route_init(const void *unused __unu > sizeof(struct radix_node_head *), M_RTABLE, M_WAITOK|M_ZERO); > > V_rtzone = uma_zcreate("rtentry", sizeof(struct rtentry), > - rtentry_ctor, NULL, > + rtentry_ctor, rtentry_dtor, > rtentry_zinit, rtentry_zfini, UMA_ALIGN_PTR, 0); > for (dom = domains; dom; dom = dom->dom_next) { > if (dom->dom_rtattach == NULL) > > Modified: head/sys/net/route.h > ============================================================================== > --- head/sys/net/route.h Wed Mar 5 20:01:04 2014 (r262805) > +++ head/sys/net/route.h Wed Mar 5 21:16:46 2014 (r262806) > @@ -309,6 +309,10 @@ struct rt_addrinfo { > #define RT_UNLOCK(_rt) mtx_unlock(&(_rt)->rt_mtx) > #define RT_LOCK_DESTROY(_rt) mtx_destroy(&(_rt)->rt_mtx) > #define RT_LOCK_ASSERT(_rt) mtx_assert(&(_rt)->rt_mtx, MA_OWNED) > +#define RT_UNLOCK_COND(_rt) do { \ > + if (mtx_owned(&(_rt)->rt_mtx)) \ > + mtx_unlock(&(_rt)->rt_mtx); \ > +} while (0) > > #define RT_ADDREF(_rt) do { \ > RT_LOCK_ASSERT(_rt); \ >