On 01/24/18 08:54, Wojciech Macek wrote:
Author: wma
Date: Wed Jan 24 07:54:05 2018
New Revision: 328320
URL: https://svnweb.freebsd.org/changeset/base/328320

Log:
   ULE: provide defaults to ts_cpu
Fix a bug when the system has no CPU 0. When created, threads were implicitly assigned to CPU 0.
   This had no practical effect since a real CPU was chosen immediately by the 
scheduler. However,
   on systems without a CPU 0, sched_ule attempted to access the scheduler queue of the 
"old" CPU
   when assigned the initial choice of the old one. This caused an attempt to 
use illegal memory
   and a crash (or, more usually, a deadlock). Fix this by assigned new threads 
to the BSP
   explicitly and add some asserts to see that this problem does not recur.
Authored by: Nathan Whitehorn <nwhiteh...@freebsd.org>
   Submitted by:          Wojciech Macek <w...@semihalf.com>
   Obtained from:         Semihalf
   Differential revision: https://reviews.freebsd.org/D13932

Modified:
   head/sys/kern/sched_ule.c

Modified: head/sys/kern/sched_ule.c
==============================================================================
--- head/sys/kern/sched_ule.c   Wed Jan 24 07:01:44 2018        (r328319)
+++ head/sys/kern/sched_ule.c   Wed Jan 24 07:54:05 2018        (r328320)
@@ -1405,6 +1405,7 @@ sched_setup(void *dummy)
/* Add thread0's load since it's running. */
        TDQ_LOCK(tdq);
+       td_get_sched(&thread0)->ts_cpu = curcpu; /* Something valid to start */
        thread0.td_lock = TDQ_LOCKPTR(TDQ_SELF());
        tdq_load_add(tdq, &thread0);
        tdq->tdq_lowpri = thread0.td_priority;
@@ -2454,6 +2455,7 @@ sched_add(struct thread *td, int flags)
         * Pick the destination cpu and if it isn't ours transfer to the
         * target cpu.
         */
+       td_get_sched(td)->ts_cpu = curcpu; /* Pick something valid to start */
        cpu = sched_pickcpu(td, flags);
        tdq = sched_setcpu(td, cpu, flags);
        tdq_add(tdq, td, flags);



Hi Wojciech,

I believe this chunk is wrong and the same like in r326218. See the explanation for r326376 as pasted below. Can you please revert?

Further refer to existing discussions about "r326218" on the SVN commit list.

Maybe you missed the point of r326376 when integrating?

--HPS

Revision 326376 - (view) (download) (annotate) - [select for diffs]
Modified Wed Nov 29 23:28:40 2017 UTC (7 weeks, 6 days ago) by hselasky
File length: 80183 byte(s)
Diff to previous 326271

The sched_add() function is not only used when the thread is initially
started, but also by the turnstiles to mark a thread as runnable for
all locks, for instance sleepqueues do:
setrunnable()->sched_wakeup()->sched_add()

In r326218 code was added to allow booting from non-zero CPU numbers
by setting the ts_cpu field inside the ULE scheduler's sched_add()
function. This had an undesired side-effect that prior sched_pin() and
sched_bind() calls got disregarded. This patch fixes the
initialization of the ts_cpu field for the ULE scheduler to only
happen once when the initial thread is constructed during system
init. Forking will then later on ensure that a valid ts_cpu value gets
copied to all children.

Reviewed by:    jhb, kib
Discussed with: nwhitehorn
MFC after:      1 month
Differential revision:  https://reviews.freebsd.org/D13298
Sponsored by:   Mellanox Technologies



_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to