From owner-p4-projects@FreeBSD.ORG Wed Sep 3 21:23:44 2003 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 605F316A4C1; Wed, 3 Sep 2003 21:23:44 -0700 (PDT) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0DA4816A4BF for ; Wed, 3 Sep 2003 21:23:44 -0700 (PDT) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9094C43FE0 for ; Wed, 3 Sep 2003 21:23:43 -0700 (PDT) (envelope-from sam@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.6/8.12.6) with ESMTP id h844Nh0U058402 for ; Wed, 3 Sep 2003 21:23:43 -0700 (PDT) (envelope-from sam@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.6/8.12.6/Submit) id h844Nhec058399 for perforce@freebsd.org; Wed, 3 Sep 2003 21:23:43 -0700 (PDT) Date: Wed, 3 Sep 2003 21:23:43 -0700 (PDT) Message-Id: <200309040423.h844Nhec058399@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to sam@freebsd.org using -f From: Sam Leffler To: Perforce Change Reviews Subject: PERFORCE change 37475 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Sep 2003 04:23:44 -0000 http://perforce.freebsd.org/chv.cgi?CH=37475 Change 37475 by sam@sam_ebb on 2003/09/03 21:23:33 Don't hold a reference in rt_parent with a reference count, it is unnecessary and we cannot safely remove it. Specifically when deleting an rtentry from which cloned entries were created rtrequest1 recurses down the tree deleting cloned entries. But this is done while holding the parent rtentry lock. This means that when the backpointer in rt_parent is reclaimed we must recursively lock the parent (which fails because the mutex is non-recursive). It's not safe to do the clone purging first or to allow recursion on the lock. The better solution appears to be to depend on the ordered relationship between the parent rtentry and all cloned items to insure rt_parent is valid. This was (briefly) discussed with Garrett who initially added the refcnt bump in rev 1.19. FWIW BSD/OS does not hold use the refcnt. Affected files ... .. //depot/projects/netperf/sys/net/route.c#7 edit Differences ... ==== //depot/projects/netperf/sys/net/route.c#7 (text+ko) ==== @@ -262,8 +262,7 @@ */ if (rt->rt_ifa) IFAFREE(rt->rt_ifa); - if (rt->rt_parent) - RTFREE(rt->rt_parent); + rt->rt_parent = NULL; /* NB: no refcnt on parent */ /* * The key is separatly alloc'd so free it (see rt_setgate()). @@ -729,9 +728,17 @@ rt->rt_rmx = (*ret_nrt)->rt_rmx; /* copy metrics */ rt->rt_rmx.rmx_pksent = 0; /* reset packet counter */ if ((*ret_nrt)->rt_flags & (RTF_CLONING | RTF_PRCLONING)) { + /* + * NB: We do not bump the refcnt on the parent + * entry under the assumption that it will + * remain so long as we do. This is + * important when deleting the parent route + * as this operation requires traversing + * the tree to delete all clones and futzing + * with refcnts requires us to double-lock + * parent through this back reference. + */ rt->rt_parent = *ret_nrt; - /* XXX locking */ - (*ret_nrt)->rt_refcnt++; } }