Date: Sun, 15 Jan 2012 03:37:47 +0200 From: =?windows-1251?B?yu7t/Oru4iDF4uPl7ejp?= <kes-kes@yandex.ru> To: =?windows-1251?B?yu7t/Oru4iDF4uPl7ejp?= <kes-kes@yandex.ru> Cc: freebsd-bugs@FreeBSD.org, bz@FreeBSD.org, rwatson@FreeBSD.org Subject: Re: misc/164130: broken netisr initialization Message-ID: <1254956114.20120115033747@yandex.ru> In-Reply-To: <68477246.20120115000025@yandex.ru> References: <201201142126.q0ELQVbZ087496@freefall.freebsd.org> <68477246.20120115000025@yandex.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1254956114.20120115033747>