#293

yeah, agreed these aren’t particularly bad. and there are still a few avenues to explore, which is good.

doing a crash course on memory barriers / thread safety in avr world and so far it seems that anything that needs thread safety is relying on atomic implementations which are relying on doing proper interrupt masking. so sounds like maybe there is no point in replacing the event queue code as it’ll come down to doing masking correctly anyway.

interesting bit here:

To sum it up:
memory barriers ensure proper ordering of volatile accesses
memory barriers don’t ensure statements with no volatile accesses to be reordered across the barrier

and:

Unfortunately, at the moment, in avr-gcc (nor in the C standard), there is no mechanism to enforce complete match of written and executed code ordering - except maybe of switching the optimization completely off (-O0), or writing all the critical code in assembly.

#294

Oh boy, that reads like gibberish to me… I’m a Haskell coder dirtying myself with you embedded people. Really I’m here for PLT. I want my garbage collector :smiley:

I can (just about) cope with suspending interrupts. The memory barrier stuff is just a bit much. I know the CPU needs to support them for lock free data structures but I have no idea how.

One point, those docs are for AVR chips, does it still apply to AVR32?

If you git grep barrier in libavr32 it shows up in a few places, they might be instructive to look at (if you care…).

#295

this is my question as well… i’m a bit lost when it comes to different versions of processors / libraries.

i’m familiar a bit with what they are talking about (as it happens, from reading java double locked checking discussions back in the day!) - basically, if you rely on some variables to make your code thread safe (which is the case with any code that involves ISRs) you have to be aware of some traps, such as the fact that the compiler can optimize things in a way that makes your code not thread safe in a completely non obvious manner. say, you create a flag to signal whether the queue is being edited. except that instead of checking the actual value it might check some cached value instead, or the order of operations is rearranged by the compiler, so your flag doesn’t get set until after you disable interrupts. etc etc. fun stuff!

1 Like
#296

So the memory barrier acts like a line in the sand. Don’t move operations from before to after (and vice-versa)?

:smiley:

#297

yeah, for memory operations specifically. basically, it guarantees that any operations on memory before the barrier are done before any memory operations after the barrier. so for the event queue if you have a variable that signals that an event is being added (so you can’t add another one at the moment) and you set it as the first operation before you modify the queue it’s not actually guaranteed that it will not reorder it and set it after the operations that modify the queue.

from what i can tell volatile keyword tells the compiler you want to make sure any read/write operations on that variable are not optimized or re-ordered. but apparently that does not prevent caching issues (really, the more i read about the topic the more confusing it is…)

1 Like
#298

bad news first - it still freezes. but it takes a while (5-10 min), and this is under super heavy load that would lock it pretty much immediately before.

what i have: ansible, 2x TXi, 4x TXo on i2c bus. 10ms metro reading from ansible and updating 24 TXo outputs. 6 audio rate triggers from just friends, 4 of them each updating 2 teletype outputs and 2 TXo outputs.

what i did:

  • changed UI_IRQ_PRIORITY to 1 instead of 3
  • updated the timer methods to use cpu_irq_disable_level(APP_TC_IRQ_PRIORITY) / cpu_irq_enable_level(APP_TC_IRQ_PRIORITY) instead of timers_pause / timers_resume
  • in irq_port0_line0 and irq_port0_line1 moved gpio_clear_pin_interrupt_flag before (not after, as previously written) event_post - was getting freezing without doing this part after changing UI_IRQ_PRIORITY level
  • made the queue variables volatile
  • in the events methods changed cpu_irq_save to cpu_irq_disable_level(APP_TC_IRQ_PRIORITY) and cpu_irq_disable_level(UI_IRQ_PRIORITY) and moved saveIndex = putIdx; behind it (and changed cpu_irq_restore to cpu_irq_enable_level)

i also tried giving the trigger interrupts level 2 and the timer interrupts level 1 - this seemed to work okay but at some point CV outputs weren’t getting updated. perhaps it’s because CV outputs get updated by a timer, but all the other timer reliant stuff worked fine. the same script was updating TXo outputs just fine.

going to try a couple more things, but in any case it’s encouraging that it takes longer to freeze it now with these changes. the UI seems to be responsive enough, no issues with screen refresh or keyboard (i did notice slowness that would go away if i turned down the trigger rate, sometimes it took a couple of tries to register a keypress - i think that’s totally acceptable under such a heavy load).

1 Like
Does your teletype stop responding after an hour or two?
#299

Great investigations!!

What metro rate are you running?

#300

What’s the before and after here? 1.4 to 2.0b9, or from 2.0b9 to your newer changes?[quote=“scanner_darkly, post:298, topic:6939”]
what i have: ansible, 2x TXi, 4x TXo on i2c bus. 10ms metro reading from ansible and updating 24 TXo outputs. 6 audio rate triggers from just friends, 4 of them each updating 2 teletype outputs and 2 TXo outputs.
[/quote]

:sunglasses:

You’re sure there is no danger from nested interrupts? Even so, keep the functions for encapsulation. It will make it easier to change the behaviour in the future.

Don’t forget to test on Ansible.

Is there a typo in there? And does this change improve performance or stability?

The event change (cpu_irq_save / cpu_irq_restore) was the one that brought the biggest stability win when I was playing with it. Part of me taking the extreme approach was to keep that code safe no matter what else changes, in any module. None of us are full time AVR32 coders. Some of us (me) aren’t even that experienced with embedded systems. I think it’s best to take an approach that whoever contributes changes, that code is always secure.

Are these changes online anywhere? I’ve only got today (Friday) left for playing with this low level stuff. Tomorrow my modules all go back in the rack and I can’t easily gain access to the serial port. (It’s half term soon, and a few weeks after that the new baby arrives…)

edit: one more thing.

I’ve been wondering if we should limit the metro to a more conservative speed, perhaps 20 or 25ms?

The idea I had was to add an extra M! op that was allowed to set the speed faster (back down to 10ms).

e.g.

M 10      # only sets the metro to 25ms
M! 10     # does set the metro to 10ms

It’s really just to indicate that you’ve turned it up to 11, and you’re into uncharted and dangerous territory.

3 Likes
#301

10ms :slight_smile:

before - https://github.com/samdoshi/teletype/commit/d840fb4d0a0c8c5f0fdf10dad9ddcb527cc4182a

after - with the changes described above (you’re right, there is a couple of typos, thanks for pointing it out, i edited the post!)

i wasn’t sure if cpu_irq_save blocked all interrupts - if it did it would interfere with i2c which i thought was the main reason for freezing (since it has the highest interrupt level). so this should increase stability (assuming cpu_irq_save does block all). i think there is a higher chance of i2c hanging if it misses some of its interrupts. but i want to also try giving it the lowest priority and see how it behaves.

in this case i’d also like to see

M!! 5 :slight_smile:

uncharted and dangerous is good! i love running M at 10… seeing teletype churn in real time is fascinating.

Are these changes online anywhere?

i’ll do a proper separate branch on my fork tomorrow. these changes require proper testing, and there is a couple more things i’d like to try, so what makes sense is to release your version as it’s more stable and properly tested.

i’m trying a version right now that gives i2c interrupt level of 1 (instead of 3), and so far (18 min) it’s fine, with audio rate triggers hitting 6 inputs and 10ms metro reading 16 times from ansible and writing 32 times to TXo!

1 Like
#302

I’ll make you a deal. I’ll try and get the Bash on Windows stuff to work today. And I’ll leave the lower level stuff in your capable hands. I’m going to move the discussion back over to the previous thread though.

If we do decide to have an M! op, I have no problem with setting the minimum for that as low as possible, 5ms or lower.

Thoughts everyone else?

4 Likes
#303

First the bad news, I’ve limited the M op to setting the fastest metro to 25ms (equiv. to 16th notes at 600bpm…)

But… I’ve added a M! op to allow setting the metro at unsupported speeds down to 2ms!

:sunglasses:

(sorry for the terrible video quality)

I’ve just glanced over at my Teletype and Ansible, they’ve been sitting there for the last 50 mins still running the same script at M 25. I’d forgotten about it…

3 Likes
#304

Following up on this… I’m going to remove XOR. It has an identical truth table to NE when used for boolean logic.

1 Like
#305

agreed.

furthermore i’d say we should simply leave out bitwise ops for now.

1 Like
#306

Still running… 5.5 hours. Need to flash a firmware though.

Ansible was directly connected to the Teletype, no other devices on the i2c bus.

#307

getting back to 2.0 discussion (will reply in the other thread shortly) - didn’t see any other bugs. i did get the screen only partially refreshing in some cases, but this could be related to when running it under heavy stress. in any case doesn’t seem like a major thing.

@sam - you said you’re planning on releasing 2.0b10 later today? will it include all the IRQ related changes introduced since d059926?

1 Like
#308

Beta 10 released. Download in first post.

Changes since beta 9:

  • breaking: remove unused Meadowphysics ops: MP.SYNC, MP.MUTE, MP.UNMUTE, MP.FREEZE, MP.UNFREEZE (these no longer work with MP 2.0)
  • breaking: rename Ansible Meadowphysics ops to start with ME, and make the names consistent with those used by Meadowphysics (names are now: ME.PRESET, ME.RESET, ME.STOP, ME.SCALE, ME.PERIOD
  • breaking: remove op XOR
  • new: M limited to setting the metronome speed to 25ms, added M! to allow setting the metronome at unsupported speeds as low as 2ms (save first, though I haven’t crashed it yet…)
  • improved: script recursion goes far and wide now
  • improved: AND and OR use boolean logic now, and are not bitwise

This version uses the latest version of libavr32 (with all the changes @scanner_darkly), please abuse the I2C.

PDF docs included in the zip, thanks to @GoneCaving and @tambouri for their contributions. Still a lot of OPs need documenting though.

Developers, given the focus is now documentation, I have merged all the changes from my 2.0 branch into the docs branch. Please use that instead.

Unless something major comes up soon, this will be the last beta for a week or 2 (school holidays are about to start).

Assuming all goes well, expect to see “release candidate 1” at the end of May.

Two more questions to leave you with. Do we want to increase the number of delayed commands from 8? And likewise stacked commands (also 8). 16 for each?

4 Likes
#309

I’ve seen that too. Screen rendering is done in the event loop now, instead of directly from a timer (and given that things inside a timer callback run with a masked timer IRQ, that’s a good thing, screen rendering is sloooow). The downside is that it can glitch out under heavy load.

It’s possible (probably even) that the SPI code is not re-entrant. Given that CV updates and slewing occur in a timer callback, it’s possible that they pre-empt the screen rendering while it’s halfway though the process. Basically, does it occur only when doing CV? Something to look at in 2.1 I guess.

#310

perhaps CV and screen updates should also use app_pause and app_resume… i’ll give this a try. i wonder if this explains CV outputs not getting updated when i gave the timer interrupts lower priority.

#311

Alternatively CV updates could go into the event queue too? Given how well the UI holds it’s responsiveness (it’s purely event queue now) it might be worth a go.

#312

yeah, could give this a try, although i’d be hesitant to make changes to that part of code - this might introduce some latency to CV updates, which would be undesirable if you have slewing enabled, for instance.

so far my thinking is that ideally i’d want to get to the point where timers have higher priority than triggers, and timers are split into 2 different priority groups, system timers (responsible for processing grid/arc/keyboard/module controls/screen refresh/output updates) that would just do their work in their callbacks and wouldn’t need to be masked from the event code, and regular timers that would generate events.