svn commit: r264321 - head/sys/netinet
Julien Charbon
jcharbon at verisign.com
Fri Apr 11 07:15:09 UTC 2014
Hi Gleb,
On 10/04/14 21:29, Gleb Smirnoff wrote:
> one comment:
>
> On Thu, Apr 10, 2014 at 06:15:35PM +0000, John Baldwin wrote:
> J> +/*
> J> + * Drop a refcount on an tw elevated using tw_pcbref(). Return
> J> + * the tw lock released.
> J> + */
> J> +static int
> J> +tw_pcbrele(struct tcptw *tw)
> J> +{
> J> +
> J> + TW_WLOCK_ASSERT(V_tw_lock);
> J> + KASSERT(tw->tw_refcount > 0, ("%s: refcount 0", __func__));
> J> +
> J> + if (!refcount_release(&tw->tw_refcount)) {
> J> + TW_WUNLOCK(V_tw_lock);
> J> + return (0);
> J> + }
> J> +
> J> + uma_zfree(V_tcptw_zone, tw);
> J> + TW_WUNLOCK(V_tw_lock);
> J> + return (1);
> J> +}
>
> We can unlock before uma_zfree(), that would reduce contention.
>
> Why do we need the lock in this function at all? We don't manipulate
> the TAILQ.
That's indeed a good point. When I started prototyping this change
having TW_WLOCK in tw_pcbrele() made sense but not anymore.
Below a patch that removes TW_WLOCK from tw_pcbrele(), I will do
another review pass with this change.
Thanks.
diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c
index c455be8..1aff6b4 100644
--- a/sys/netinet/tcp_timewait.c
+++ b/sys/netinet/tcp_timewait.c
@@ -131,22 +131,17 @@ tw_pcbref(struct tcptw *tw)
}
/*
- * Drop a refcount on an tw elevated using tw_pcbref(). Return
- * the tw lock released.
+ * Drop a refcount on an tw elevated using tw_pcbref().
*/
static int
tw_pcbrele(struct tcptw *tw)
{
- TW_WLOCK_ASSERT(V_tw_lock);
KASSERT(tw->tw_refcount > 0, ("%s: refcount 0", __func__));
- if (!refcount_release(&tw->tw_refcount)) {
- TW_WUNLOCK(V_tw_lock);
+ if (!refcount_release(&tw->tw_refcount))
return (0);
- }
uma_zfree(V_tcptw_zone, tw);
- TW_WUNLOCK(V_tw_lock);
return (1);
}
@@ -680,10 +675,8 @@ tcp_tw_2msl_stop(struct tcptw *tw, int reuse)
crfree(tw->tw_cred);
tw->tw_cred = NULL;
- if (!reuse) {
+ if (!reuse)
tw_pcbrele(tw);
- return;
- }
TW_WUNLOCK(V_tw_lock);
}
@@ -728,7 +721,6 @@ tcp_tw_2msl_scan(void)
/* Close timewait state */
if (INP_INFO_TRY_WLOCK(&V_tcbinfo)) {
- TW_WLOCK(V_tw_lock);
if (tw_pcbrele(tw))
continue;
@@ -740,7 +732,6 @@ tcp_tw_2msl_scan(void)
INP_INFO_WUNLOCK(&V_tcbinfo);
} else {
/* INP_INFO lock is busy, continue later */
- TW_WLOCK(V_tw_lock);
tw_pcbrele(tw);
break;
}
--
Julien
More information about the svn-src-head
mailing list