Fixes: #4488

The code looks sane to me. I guess you have not yet looked at code sizes, right? Especially the memory increase for the tcb would be interesting to know...

Though in general I would say that we don't enable this feature per default and try to get the newlib 'thread-safe-enough' with other means (e.g. actively blocking or buffering UART for puts/printf) to safe memory and task switching overheads, but this is also something we would need to benchmark to make some informed decisions...

Yep, please update tests/sizeof_tcb/main.c.

Fixed compile problems and included it in tests\sizeof_tcb

Please note that it would also be possible to make the reent implementation thread specific by using a pointer to struct _reent where the struct itself will be placed on the stack when enabled and the tcb would only hold the pointer to the location on the stack or use the _global_impure_ptr. This way a simple LED toggle task wouldn't have a thread-safe newlib context. But the consol handler does for example.

@haukepetersen the code in newlib is already called, the only difference now is that newlib doesn't use a global struct but a thread specific struct. So no penalties are to be expected.

So no penalties are to be expected

I am not so sure about that. We have this newlib struct the as part of every tcb, so the tcp size increases, right? Did you do some initial measurements on the changes in code/ram usage?

Any news?

The reentry struct is 96 bytes / 24 words total :)

@DipSwitch So, that is the cost per thread to implement thread safe errno etc?

Yup :)
On 29 Jan 2016 14:37, "Joakim Nohlgård" [email protected] wrote:

@DipSwitch https://github.com/DipSwitch So, that is the cost per thread
to implement thread safe errno etc?

—
Reply to this email directly or view it on GitHub
#4529 (comment).

I tried this PR very briefly and I get hard faults on mulle during startup in the xtimer_drift test application and a plain lockup on stm32f4discovery. The stm32 is stuck inside sched_run with an empty runqueue. The program runs up to the first xtimer_usleep_until call before hanging, but there is no output seen at all on the UART.

Below is my quick debug dump from the stm32f4discovery board:

^C
Program received signal SIGINT, Interrupt.
0x08000398 in bitarithm_lsb (v=0) at /home/jgn/work/src/riot/core/bitarithm.c:53
53          r++;
(gdb) bt
#0  bitarithm_lsb (v=0) at /home/jgn/work/src/riot/core/bitarithm.c:53
#1  0x08000220 in sched_run () at /home/jgn/work/src/riot/core/sched.c:72
#2  0x0800047e in isr_svc () at /home/jgn/work/src/riot/cpu/cortexm_common/thread_arch.c:275
(gdb) list
48  {
49      register unsigned r = 0;
50  
51      while ((v & 0x01) == 0) {
52          v >>= 1;
53          r++;
54      };
55  
56      return r;
57  }
(gdb) print v
$1 = 0
(gdb) print r
$2 = 77772978
(gdb) frame 1
#1  0x08000220 in sched_run () at /home/jgn/work/src/riot/core/sched.c:72
72      int nextrq = bitarithm_lsb(runqueue_bitcache);
(gdb) list
67      tcb_t *active_thread = (tcb_t *)sched_active_thread;
68  
69      /* The bitmask in runqueue_bitcache is never empty,
70       * since the threading should not be started before at least the idle thread was started.
71       */
72      int nextrq = bitarithm_lsb(runqueue_bitcache);
73      tcb_t *next_thread = clist_get_container(sched_runqueues[nextrq], tcb_t, rq_entry);
74  
75      DEBUG("sched_run: active thread: %" PRIkernel_pid ", next thread: %" PRIkernel_pid "\n",
76            (active_thread == NULL) ? KERNEL_PID_UNDEF : active_thread->pid,
(gdb) print runqueue_bitcache 
$3 = 0

Any news?

ping @DipSwitch

I looked yesterday and have a fix the crash. But then I noticed the stack
sizes where bigger without optimization. IDLE went from 168 to 1590
somewhere. While with debug enabled and no thread-safe newlib the size
didn't increase so dramatically. Will state findings and numbers in a bit.
First some food :p

Maybe instead of using -O0 this can be solved using -Og although I don't
know if clang supports this?
On 13 Mar 2016 08:40, "Joakim Nohlgård" [email protected] wrote:

ping @DipSwitch https://github.com/DipSwitch

—
Reply to this email directly or view it on GitHub
#4529 (comment).

@DipSwitch no rush, could you push your updated branch to Github?

Hmm false positive...

newlib-thread-safe: -Og
> ps
        pid | name                 | state    Q | pri | stack ( used) | location
          1 | idle                 | pending  Q |  15 |  2048 ( 1200) | 0x200001cc
          2 | main                 | running  Q |   7 |  2560 ( 1784) | 0x200009cc
            | SUM                  |            |     |  4608 ( 2984)

newlib-thread-safe: -Os
> ps
        pid | name                 | state    Q | pri | stack ( used) | location
          1 | idle                 | pending  Q |  15 |  2048 ( 1200) | 0x200001cc
          2 | main                 | running  Q |   7 |  2560 ( 1792) | 0x200009cc
            | SUM                  |            |     |  4608 ( 2992)

newlib: -Os
> ps
        pid | name                 | state    Q | pri | stack ( used) | location
          1 | idle                 | pending  Q |  15 |   256 (  136) | 0x200001cc
          2 | main                 | running  Q |   7 |  1536 (  728) | 0x200002cc
            | SUM                  |            |     |  1792 (  864)


newlib: -Og
> ps
        pid | name                 | state    Q | pri | stack ( used) | location
          1 | idle                 | pending  Q |  15 |   256 (  136) | 0x200001cc
          2 | main                 | running  Q |   7 |  1536 (  720) | 0x200002cc
            | SUM                  |            |     |  1792 (  856)

main(): This is RIOT! (Version: 2016.03-devel-1104-g5e548-dipswitch-K55VD-pr/add_newlib_threadsafe)
        member, sizeof, offsetof
sizeof(thread_t): 1104
        sp            4   0
        status        1   4
        priority      1   5
        pid           2   6
        rq_entry      8   8
        wait_data     4  16
        msg_waiters   4  20
        msg_queue    12  24
        msg_array     4  36
        newlib_reent1064  40
Done.

And sorry about the stray commits, will fix later.

I don't know where the previous statement of 96 bytes is comming from. Maybe it got something to do with an optimalization which came with using -fdata-sections -ffunction-sections -Wl,--gc-sections enabled and having the struct _reent being located in bss...

I don't see why this is marked as a bug (except if you see newlib's non-threadsafeness as a bug), so postponed due to feature freeze

Any progress here?

postponed 4 times in the past, removing milestone label.

No significant progress for over a year. Closing as memo for now.