From owner-freebsd-bugs@FreeBSD.ORG Sun Jan 15 01:53:07 2012 Return-Path: Delivered-To: freebsd-bugs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BB9A91065675; Sun, 15 Jan 2012 01:53:07 +0000 (UTC) (envelope-from kes-kes@yandex.ru) Received: from forward9.mail.yandex.net (forward9.mail.yandex.net [77.88.61.48]) by mx1.freebsd.org (Postfix) with ESMTP id 4D0968FC0A; Sun, 15 Jan 2012 01:53:07 +0000 (UTC) Received: from smtp6.mail.yandex.net (smtp6.mail.yandex.net [77.88.61.56]) by forward9.mail.yandex.net (Yandex) with ESMTP id 9ACBCCE1960; Sun, 15 Jan 2012 05:37:51 +0400 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1326591471; bh=dt4ytMvQ0sz0auaGY3164RN4QxF2NzxEydigznNtLC8=; h=Date:From:Reply-To:Message-ID:To:CC:Subject:In-Reply-To: References:MIME-Version:Content-Type:Content-Transfer-Encoding; b=uHzpM3Q00SkXrw4GGLFDN8U1yEsjRL2OZqgXAvXMOSC27W955jJwBKC3fYVMcmJ8D vVRKywwG3GzxCTtHtA/XKFbZklO/0cZ3zELEFGg+PoHehwoilahnihKVqQ9g7Olidc 2ax+glgn9q+REb4IzRKmil9fW66pic5rvWRpRgoU= Received: from smtp6.mail.yandex.net (localhost [127.0.0.1]) by smtp6.mail.yandex.net (Yandex) with ESMTP id 3D0D916404BD; Sun, 15 Jan 2012 05:37:51 +0400 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1326591471; bh=dt4ytMvQ0sz0auaGY3164RN4QxF2NzxEydigznNtLC8=; h=Date:From:Reply-To:Message-ID:To:CC:Subject:In-Reply-To: References:MIME-Version:Content-Type:Content-Transfer-Encoding; b=uHzpM3Q00SkXrw4GGLFDN8U1yEsjRL2OZqgXAvXMOSC27W955jJwBKC3fYVMcmJ8D vVRKywwG3GzxCTtHtA/XKFbZklO/0cZ3zELEFGg+PoHehwoilahnihKVqQ9g7Olidc 2ax+glgn9q+REb4IzRKmil9fW66pic5rvWRpRgoU= Received: from unknown (unknown [176.8.25.138]) by smtp6.mail.yandex.net (nwsmtp/Yandex) with ESMTP id bopOjP7Y-bopSOwai; Sun, 15 Jan 2012 05:37:50 +0400 X-Yandex-Spam: 1 Date: Sun, 15 Jan 2012 03:37:47 +0200 From: =?windows-1251?B?yu7t/Oru4iDF4uPl7ejp?= X-Mailer: The Bat! (v4.0.24) Professional Organization: =?windows-1251?B?188gyu7t/Oru4iwgRnJlZUxpbmU=?= X-Priority: 3 (Normal) Message-ID: <1254956114.20120115033747@yandex.ru> To: =?windows-1251?B?yu7t/Oru4iDF4uPl7ejp?= In-Reply-To: <68477246.20120115000025@yandex.ru> References: <201201142126.q0ELQVbZ087496@freefall.freebsd.org> <68477246.20120115000025@yandex.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=windows-1251 Content-Transfer-Encoding: 8bit Cc: freebsd-bugs@FreeBSD.org, bz@FreeBSD.org, rwatson@FreeBSD.org Subject: Re: misc/164130: broken netisr initialization X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: =?windows-1251?B?yu7t/Oru4iDF4uPl7ejp?= List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Jan 2012 01:53:07 -0000 Hi, rwatson. This is unsafe to do same things in different places: 368: sysctl_netisr_dispatch_policy(SYSCTL_HANDLER_ARGS) { char tmp[NETISR_DISPATCH_POLICY_MAXSTR]; u_int dispatch_policy; int error; netisr_dispatch_policy_to_str(netisr_dispatch_policy, tmp, sizeof(tmp)); error = sysctl_handle_string(oidp, tmp, sizeof(tmp), req); if (error == 0 && req->newptr != NULL) { ! error = netisr_dispatch_policy_from_str(tmp, ! &dispatch_policy); ! if (error == 0 && dispatch_policy == NETISR_DISPATCH_DEFAULT) ! error = EINVAL; ! if (error == 0) { ! netisr_dispatch_policy = dispatch_policy; ! netisr_dispatch_policy_compat(); } } return (error); } 1197: if (TUNABLE_STR_FETCH("net.isr.dispatch", tmp, sizeof(tmp))) { ! error = netisr_dispatch_policy_from_str(tmp, ! &dispatch_policy); ! if (error == 0 && dispatch_policy == NETISR_DISPATCH_DEFAULT) ! error = EINVAL; ! if (error == 0) { ! netisr_dispatch_policy = dispatch_policy; ! netisr_dispatch_policy_compat(); } else printf( "%s: invalid dispatch policy %s, using default\n", __func__, tmp); } Create procedure for such things! Index: netisr.c =================================================================== --- netisr.c (revision 230107) +++ netisr.c (working copy) @@ -158,11 +158,11 @@ * dispatch policy state. Now, we provide read-only export via them so that * older netstat binaries work. At some point they can be garbage collected. */ -static int netisr_direct_force; +static int netisr_direct_force = 1; SYSCTL_INT(_net_isr, OID_AUTO, direct_force, CTLFLAG_RD, &netisr_direct_force, 0, "compat: force direct dispatch"); -static int netisr_direct; +static int netisr_direct = 1; SYSCTL_INT(_net_isr, OID_AUTO, direct, CTLFLAG_RD, &netisr_direct, 0, "compat: enable direct dispatch"); @@ -716,6 +716,8 @@ * current CPU ID, which will force an immediate direct * dispatch. In the queued case, fall back on the SOURCE * policy. + * XXX: Make comment to hetisr.h to NETISR_POLICY_CPU + * Describing that it fallback to POLICY_SOURCE */ if (*cpuidp != NETISR_CPUID_NONE) return (m); @@ -727,6 +729,18 @@ } if (policy == NETISR_POLICY_FLOW) { + /* + * Do notice about: mbuf.h(202) + * #define>M_FLOWID<------>0x00400000 /* deprecated: flowid is valid */ + * + * and also notice some strange code: + * if( !a && b ) ...C1 + * if( a ) ...C2 + * it is same as next: + * if( a ) ...C1 + * else if( b ) ...C2 + * Second one is more clearer, is not? Or here maybe mistake... + */ if (!(m->m_flags & M_FLOWID) && npp->np_m2flow != NULL) { m = npp->np_m2flow(m, source); if (m == NULL) @@ -995,11 +1009,10 @@ proto)); dispatch_policy = netisr_get_dispatch(npp); - if (dispatch_policy == NETISR_DISPATCH_DEFERRED) - return (netisr_queue_src(proto, source, m)); /* - * If direct dispatch is forced, then unconditionally dispatch + * direct dispatch has more priority, so check on it first + * If there is direct dispatch, then unconditionally dispatch * without a formal CPU selection. Borrow the current CPU's stats, * even if there's no worker on it. In this case we don't update * nws_flags because all netisr processing will be source ordered due @@ -1010,11 +1023,16 @@ npwp = &nwsp->nws_work[proto]; npwp->nw_dispatched++; npwp->nw_handled++; - netisr_proto[proto].np_handler(m); + npp->np_handler(m); error = 0; goto out_unlock; } + /* DEFFERED DISPATCH */ + if (dispatch_policy == NETISR_DISPATCH_DEFERRED) + return (netisr_queue_src(proto, source, m)); + + KASSERT(dispatch_policy == NETISR_DISPATCH_HYBRID, ("%s: unknown dispatch policy (%u)", __func__, dispatch_policy)); @@ -1023,12 +1041,12 @@ * dispatch if we're on the right CPU and the netisr worker isn't * already running. */ - sched_pin(); - m = netisr_select_cpuid(&netisr_proto[proto], NETISR_DISPATCH_HYBRID, + sched_pin(); //Why you are trying to shced_pin() .... + m = netisr_select_cpuid(npp, NETISR_DISPATCH_HYBRID, source, m, &cpuid); if (m == NULL) { error = ENOBUFS; - goto out_unpin; + goto out_unpin; //...to immediately free it? (revert changes to r222249) } KASSERT(!CPU_ABSENT(cpuid), ("%s: CPU %u absent", __func__, cpuid)); if (cpuid != curcpu) @@ -1061,7 +1079,7 @@ */ nwsp->nws_flags |= NWS_DISPATCHING; NWS_UNLOCK(nwsp); - netisr_proto[proto].np_handler(m); + npp->np_handler(m); NWS_LOCK(nwsp); nwsp->nws_flags &= ~NWS_DISPATCHING; npwp->nw_handled++; @@ -1201,13 +1219,14 @@ error = EINVAL; if (error == 0) { netisr_dispatch_policy = dispatch_policy; - netisr_dispatch_policy_compat(); } else printf( "%s: invalid dispatch policy %s, using default\n", __func__, tmp); } + netisr_dispatch_policy_compat(); + netisr_start_swi(curcpu, pcpu_find(curcpu)); } SYSINIT(netisr_init, SI_SUB_SOFTINTR, SI_ORDER_FIRST, netisr_init, NULL);