From owner-svn-src-head@freebsd.org Tue Mar 14 21:57:08 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A402ED0CF86; Tue, 14 Mar 2017 21:57:08 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (glebi.us [96.95.210.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebi.us", Issuer "cell.glebi.us" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 6DD1E1087; Tue, 14 Mar 2017 21:57:07 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (localhost [127.0.0.1]) by cell.glebi.us (8.15.2/8.15.2) with ESMTPS id v2ELv6HX001718 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 14 Mar 2017 14:57:07 -0700 (PDT) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebi.us (8.15.2/8.15.2/Submit) id v2ELv6SI001717; Tue, 14 Mar 2017 14:57:06 -0700 (PDT) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebi.us: glebius set sender to glebius@FreeBSD.org using -f Date: Tue, 14 Mar 2017 14:57:06 -0700 From: Gleb Smirnoff To: Kristof Provost Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r315136 - head/sys/netpfil/pf Message-ID: <20170314215706.GB1072@FreeBSD.org> References: <201703120542.v2C5gvM4075391@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201703120542.v2C5gvM4075391@repo.freebsd.org> User-Agent: Mutt/1.7.2 (2016-11-26) X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Mar 2017 21:57:08 -0000 Kristof, On Sun, Mar 12, 2017 at 05:42:57AM +0000, Kristof Provost wrote: K> Log: K> pf: Fix incorrect rw_sleep() in pf_unload() K> K> When we unload we don't hold the pf_rules_lock, so we cannot call rw_sleep() K> with it, because it would release a lock we do not hold. There's no need for the K> lock either, so we can just tsleep(). K> K> While here also make the same change in pf_purge_thread(), because it explicitly K> takes the lock before rw_sleep() and then immediately releases it afterwards. The correct change would to be grab lock in pf_unload(), exactly as pf_purge_thread() does. With your change you introduces a possible infinite sleep due to race, since there is no timeout and no lock. No... Actually both cases should PF_RULES_WLOCK(), and read/write the pf_end_threads variable under this lock. And use rw_sleep. K> Modified: K> head/sys/netpfil/pf/pf.c K> head/sys/netpfil/pf/pf_ioctl.c K> K> Modified: head/sys/netpfil/pf/pf.c K> ============================================================================== K> --- head/sys/netpfil/pf/pf.c Sun Mar 12 05:36:31 2017 (r315135) K> +++ head/sys/netpfil/pf/pf.c Sun Mar 12 05:42:57 2017 (r315136) K> @@ -1429,9 +1429,7 @@ pf_purge_thread(void *unused __unused) K> u_int idx = 0; K> K> for (;;) { K> - PF_RULES_RLOCK(); K> - rw_sleep(pf_purge_thread, &pf_rules_lock, 0, "pftm", hz / 10); K> - PF_RULES_RUNLOCK(); K> + tsleep(pf_purge_thread, 0, "pftm", hz / 10); K> K> VNET_LIST_RLOCK(); K> VNET_FOREACH(vnet_iter) { K> K> Modified: head/sys/netpfil/pf/pf_ioctl.c K> ============================================================================== K> --- head/sys/netpfil/pf/pf_ioctl.c Sun Mar 12 05:36:31 2017 (r315135) K> +++ head/sys/netpfil/pf/pf_ioctl.c Sun Mar 12 05:42:57 2017 (r315136) K> @@ -3791,7 +3791,7 @@ pf_unload(void) K> pf_end_threads = 1; K> while (pf_end_threads < 2) { K> wakeup_one(pf_purge_thread); K> - rw_sleep(pf_purge_thread, &pf_rules_lock, 0, "pftmo", 0); K> + tsleep(pf_purge_thread, 0, "pftmo", 0); K> } K> K> if (pf_dev != NULL) K> _______________________________________________ K> svn-src-all@freebsd.org mailing list K> https://lists.freebsd.org/mailman/listinfo/svn-src-all K> To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" -- Totus tuus, Glebius.