Date: Wed, 3 Sep 2003 21:23:43 -0700 (PDT) From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 37475 for review Message-ID: <200309040423.h844Nhec058399@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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++; } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200309040423.h844Nhec058399>