Threads are dying, I cannot find the issue :(

[russian]

Yes, absolutely

Code: Select all

#define UTILITY_THREAD_STACK_SIZE 400

static THD_WORKING_AREA(waThreadStack, UTILITY_THREAD_STACK_SIZE);
   chThdCreateStatic(waThreadStack, sizeof(waThreadStack), NORMALPRIO, waThread, (void*) NULL);

static THD_WORKING_AREA(blinkingStack, 128);
   chThdCreateStatic(blinkingStack, sizeof(blinkingStack), NORMALPRIO, (tfunc_t) blinkingThread, NULL);

static THD_WORKING_AREA(consoleThreadStack, 2 * UTILITY_THREAD_STACK_SIZE);
   chThdCreateStatic(consoleThreadStack, sizeof(consoleThreadStack), NORMALPRIO, consoleThreadThreadEntryPoint, NULL);

[Giovanni]

Do you have just 3 threads?

Giovanni

[russian]

I have 11 or 13 threads, each has it's own working area.

[Giovanni]

I have 11 or 13 threads, each has it's own working area.

Copy paste errors there ?

[russian]

Most threads are declared in different files and all working areas are defined as 'static' so there is no cross visibility, but still - full grep is in the bottom of this message.

I can assume that all threads are really stuck in the same method - onUnlockHook method is relatively special and getTimeNowNt method is the one where I have volatile/atomic magic. I will try without the magic later today just to see if it affects anything.

efitick_t getTimeNowNt(void) {
return halTime.get();
}

uint64_t localH;
uint32_t localLow;
int c = 0;
while (true) {
localH = state.highBits;
localLow = state.lowBits;
uint64_t localH2 = state.highBits;
if(c++ == 10000)
chDbgFatal("stuck");
if (localH == localH2)
break;
}

the "stuck" fatal is not happening to my surprise.

Anyway,

$ grep -r chThdCreateStatic *
chibios/os/kernel/include/chthreads.h: Thread *chThdCreateStatic(void *wsp, size_t size,
chibios/os/kernel/src/chsys.c: chThdCreateStatic(_idle_thread_wa, sizeof(_idle_thread_wa), IDLEPRIO,
chibios/os/kernel/src/chthreads.c:Thread *chThdCreateStatic(void *wsp, size_t size,
chibios/os/kernel/src/chthreads.c: * - If the thread was spawned by @p chThdCreateStatic() or by
chibios/os/various/cpp_wrappers/ch.hpp: * - If the thread was spawned by @p chThdCreateStatic() or by
chibios/os/various/cpp_wrappers/ch.hpp: thread_ref = chThdCreateStatic(wa, sizeof(wa), prio, _thd_start, this);
console/console_io.c: chThdCreateStatic(consoleThreadStack, sizeof(consoleThreadStack), NORMALPRIO, consoleThreadThreadEntryPoint, NULL);
console/status_loop.cpp: chThdCreateStatic(lcdThreadStack, sizeof(lcdThreadStack), NORMALPRIO, (tfunc_t) lcdThread, engine);
console/status_loop.cpp: chThdCreateStatic(tsThreadStack, sizeof(tsThreadStack), NORMALPRIO, (tfunc_t) tsStatusThread, engine);
console/status_loop.cpp: chThdCreateStatic(blinkingStack, sizeof(blinkingStack), NORMALPRIO, (tfunc_t) blinkingThread, NULL);
console/tunerstudio/tunerstudio.cpp: chThdCreateStatic(tsThreadStack, sizeof(tsThreadStack), NORMALPRIO, tsThreadEntryPoint, NULL);
controllers/algo/accel_enrichment.cpp:// chThdCreateStatic(aeThreadStack, sizeof(aeThreadStack), LOWPRIO, (tfunc_t) DiffEnrichmentThread, NULL);
controllers/alternatorController.cpp: chThdCreateStatic(ivThreadStack, sizeof(ivThreadStack), LOWPRIO, (tfunc_t)AltCtrlThread, NULL);
controllers/electronic_throttle.cpp: chThdCreateStatic(etbTreadStack, sizeof(etbTreadStack), NORMALPRIO, (tfunc_t) etbThread, NULL);
controllers/engine_controller.cpp: chThdCreateStatic(csThreadStack, sizeof(csThreadStack), LOWPRIO, (tfunc_t) csThread, NULL);
controllers/idle_thread.cpp: chThdCreateStatic(ivThreadStack, sizeof(ivThreadStack), NORMALPRIO, (tfunc_t) ivThread, NULL);
controllers/injector_central.cpp: chThdCreateStatic(benchThreadStack, sizeof(benchThreadStack), NORMALPRIO, (tfunc_t) benchThread, NULL);
controllers/malfunction_indicator.cpp: chThdCreateStatic(mfiThreadStack, sizeof(mfiThreadStack), LOWPRIO, (tfunc_t) mfiThread, NULL);
development/engine_emulator.cpp: chThdCreateStatic(eeThreadStack, sizeof(eeThreadStack), NORMALPRIO, (tfunc_t) eeThread, engine);
development/test/testbmk.c: threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()-1, thread1, NULL);
development/test/testbmk.c: threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()+1, thread1, NULL);
development/test/testbmk.c: threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()+1, thread1, NULL);
development/test/testbmk.c: threads[1] = chThdCreateStatic(wa[1], WA_SIZE, chThdGetPriority()-2, thread2, NULL);
development/test/testbmk.c: threads[2] = chThdCreateStatic(wa[2], WA_SIZE, chThdGetPriority()-3, thread2, NULL);
development/test/testbmk.c: threads[3] = chThdCreateStatic(wa[3], WA_SIZE, chThdGetPriority()-4, thread2, NULL);
development/test/testbmk.c: threads[4] = chThdCreateStatic(wa[4], WA_SIZE, chThdGetPriority()-5, thread2, NULL);
development/test/testbmk.c: tp = threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()+1, thread4, NULL);
development/test/testbmk.c: * @p chThdCreateStatic() / @p chThdExit() / @p chThdWait() cycle is performed
development/test/testbmk.c: chThdWait(chThdCreateStatic(wap, WA_SIZE, prio, thread2, NULL));
development/test/testbmk.c: * @p chThdCreateStatic() / @p chThdExit() cycle is performed in each
development/test/testbmk.c: chThdCreateStatic(wap, WA_SIZE, prio, thread2, NULL);
development/test/testbmk.c: threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()+5, thread3, NULL);
development/test/testbmk.c: threads[1] = chThdCreateStatic(wa[1], WA_SIZE, chThdGetPriority()+4, thread3, NULL);
development/test/testbmk.c: threads[2] = chThdCreateStatic(wa[2], WA_SIZE, chThdGetPriority()+3, thread3, NULL);
development/test/testbmk.c: threads[3] = chThdCreateStatic(wa[3], WA_SIZE, chThdGetPriority()+2, thread3, NULL);
development/test/testbmk.c: threads[4] = chThdCreateStatic(wa[4], WA_SIZE, chThdGetPriority()+1, thread3, NULL);
development/test/testbmk.c: threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()-1, thread8, (void *)&n);
development/test/testbmk.c: threads[1] = chThdCreateStatic(wa[1], WA_SIZE, chThdGetPriority()-1, thread8, (void *)&n);
development/test/testbmk.c: threads[2] = chThdCreateStatic(wa[2], WA_SIZE, chThdGetPriority()-1, thread8, (void *)&n);
development/test/testbmk.c: threads[3] = chThdCreateStatic(wa[3], WA_SIZE, chThdGetPriority()-1, thread8, (void *)&n);
development/test/testbmk.c: threads[4] = chThdCreateStatic(wa[4], WA_SIZE, chThdGetPriority()-1, thread8, (void *)&n);
development/wave_analyzer.cpp: chThdCreateStatic(waThreadStack, sizeof(waThreadStack), NORMALPRIO, waThread, (void*) NULL);
egt2can.cpp: chThdCreateStatic(waThread1, sizeof(waThread1), NORMALPRIO, Thread1, NULL);
hw_layer/board_test.cpp: chThdCreateStatic(btThreadStack, sizeof(btThreadStack), NORMALPRIO, (tfunc_t) ivThread, NULL);
hw_layer/can_hw.cpp: chThdCreateStatic(canTreadStack, sizeof(canTreadStack), NORMALPRIO, (tfunc_t) canThread, NULL);
hw_layer/microsecond_timer.c: chThdCreateStatic(mwThreadStack, sizeof(mwThreadStack), NORMALPRIO, (tfunc_t) mwThread, NULL);
hw_layer/mmc_card.cpp: chThdCreateStatic(mmcThreadStack, sizeof(mmcThreadStack), LOWPRIO, (tfunc_t) MMCmonThread, NULL);
hw_layer/neo6m.cpp: chThdCreateStatic(gpsThreadStack, sizeof(gpsThreadStack), LOWPRIO, GpsThreadEntryPoint, NULL);
hw_layer/stepper.cpp: chThdCreateStatic(stThreadStack, sizeof(stThreadStack), NORMALPRIO, (tfunc_t) stThread, this);

[Giovanni]

This:

Code: Select all

controllers/alternatorController.cpp: chThdCreateStatic(ivThreadStack, sizeof(ivThreadStack), LOWPRIO, (tfunc_t)AltCtrlThread, NULL);

Then this:

Code: Select all

controllers/idle_thread.cpp: chThdCreateStatic(ivThreadStack, sizeof(ivThreadStack), NORMALPRIO, (tfunc_t) ivThread, NULL);

Giovanni

[russian]

This:

Code: Select all

controllers/alternatorController.cpp: chThdCreateStatic(ivThreadStack, sizeof(ivThreadStack), LOWPRIO, (tfunc_t)AltCtrlThread, NULL);

Until yesterday this was dead code - see line 18,

#if 0

https://svn.code.sf.net/p/rusefi/code/tags/tag20150301_7212/firmware/controllers/alternatorController.cpp

Yesterday I have un-commented this code, both files have their own static fields
alternatorController.cpp

static THD_WORKING_AREA(ivThreadStack, UTILITY_THREAD_STACK_SIZE);

idle_thread.cpp

static THD_WORKING_AREA(ivThreadStack, UTILITY_THREAD_STACK_SIZE);

But this is bad anyway. Let me check if I have actually transitioned from one bug into another by un-commenting this file yesterday

[russian]

Nevermind. initAlternatorCtrl is never invoked - Alternator control thread is never created.

[colin]

I think I've found the issue, it has nothing to do with Chibi, sorry for bothering.

I actually believe it has something to do with the compiler, but it's midnight here so I am not at full capacity. I have what I believe should be a lock-free atomic read from a 64 bit timer counter and that's where my code hangs up:

Code: Select all

typedef struct {
   uint64_t highBits;
   uint32_t lowBits;
} State64;

   /**
    * this method is lock-free and thread-safe, that's because the 'update' method
    * is atomic with a critical zone requirement.
    *
    * http://stackoverflow.com/questions/5162673/how-to-read-two-32bit-counters-as-a-64bit-integer-without-race-condition
    */
   uint64_t localH;
   uint32_t localLow;
   int counter = 0;
   while (true) {
      localH = state.highBits;
      localLow = state.lowBits;
      uint64_t localH2 = state.highBits;
      if (localH == localH2)
         break;
   }

the idea was that I am reading from highBits twice and if there is a mismatch I am re-reading both again until they match. Looks like the compiler has taken localH = state.highBits out of the look and I am hanging up here. Probably my fault that I have not declared the fields volatile?

Maybe this is not your issue for the lockup, if you aren't seeing it get "stuck" here. But it seems there might be a race condition in the lock-free counter code you wrote.

The lock-free technique as mentioned in the StackOverflow answers is only valid when the individual high/low word load operations are each atomic with respect to itself. But will the code be correct when the highBits field is a 64-bit integer? Does ARMv7-M have atomic 64-bit load? If not, the read in localH = state.highBits or localH2 = state.highBits will be potentially corrupted.

Maybe there's a reason it is impossible, but perhaps it's worth taking to heart Giovanni's suggestion to have a small critical section around the accessing of these timer fields instead of trying to make this short sequence of instructions lock-free.

[russian]

Does ARMv7-M have atomic 64-bit load? If not, the read in localH = state.highBits or localH2 = state.highBits will be potentially corrupted.

If there was a 64-bit atomic load the whole trick would not be needed I guess. Only the higher 32 bits of the 64 bit variable are used while the lower 32 bits are always zero, I assume that reading the 32 bit parts would be atomic, but I agree that this is iffy.

Anyway, I have replaced the questionable lock-free with a plain critical section and I am still experiencing the same issue: after some random duration of time, I get all my threads in rlist instead of vtlist, I see that thread switch is occurring while none of the threads seem to be really running.