Hallo zusammen
Ich habe einen Ringpuffer der in der Hauptprogrammschleife gefüllt und
in einer ISR abgearbeitet wird.
Es ist nun ja möglich, dass AddCommand gerade ausgeführt und
unterbrochen wird
CmdBuffer.Count++;
CmdBuffer.EndIdx++; <--------------hier Interrupt
CmdBuffer.EndIdx &= (MAX_CMD_BUFFER-1);
return 0;
Mir ist nun aufgefallen, dass es vorkam, dass die Variable Count am Ende
der Ausführung um 1 zu gross war.
Die Variablen werden im gesamten Code an keiner anderen Stelle
verändert.
Wenn ich Count in AddCmd so codiere, dann trat der Fehler nicht auf:
CmdBuffer.EndIdx++; <--------------hier Interrupt
CmdBuffer.EndIdx &= (MAX_CMD_BUFFER-1);
CmdBuffer.Count++;
Auch wenn es allgemein "schöner" ist, die Variable Count erst am Ende zu
erhöhen habe ich keine Erklärung, wieso der Code zu einem Problem führen
könnte.
Controller LPC2148, ARM7, Compiler GCC
Habe den wesentlichen Teil mal isoliert. Er sieht so aus:
Geri schrieb:> CmdBuffer.Count++;> CmdBuffer.EndIdx++; <--------------hier Interrupt> CmdBuffer.EndIdx &= (MAX_CMD_BUFFER-1);> Die Variablen werden im gesamten Code an keiner anderen> Stelle verändert.
Nicht?
Und was ist damit?
CmdBuffer.Count--;
> Bin schon gespannt auf eure Meinung!
Dein Denkfehler besteht darin, dass du davon ausgehst, dass eine
C-Instruktion keine 'Feinstruktur' hat und als solches nicht durch einen
Interrupt unterbrechbar ist.
Je nachdem, wie der Compiler das genau implementiert, kann
CmdBuffer.Count++;
zb so übersetzt werden
1) lade CmdBuffer.Count in ein Arbeitsregister
2) inkrementiere das Arbeitsregister
3) speichere das Arbeitsregister nach CmdBuffer.Count zurück.
Wenn dir der Interrupt zwischen den Schritten 1) und 2) reinknallt, kann
es daher passieren, dass du zb mit einem Count von 5 startest. Die 5
werden in die CPU geladen, jetzt passiert ein Interrupt. Im Interrupt
wird 1 Zeichen ausgegeben und der Count auf 4 korrigiert. Der Interrupt
kommt zurück, der Code hier macht weiter. Die 5 sind immer noch im
Register, werden erhöht und zurückgeschrieben. Deine Variable hat jetzt
den Wert 6, obwohl sie eigentlich 5 sein sollte.
Ein Vorschlag :
In der AddCmd inkrementierst Du nur EndIdx
In der IRQ inkrementierst Du nur StartIdx
( Natürlich immer Modulo Buffergröße. )
Statt if (CmdBuffer->Count > 0) vergleichst Du StartIdx und EndIdx
if (CmdBuffer.StartIdx != CmdBuffer.EndIdx)
Dann brauchst Du das Count gar nicht mehr und es muss nicht aus zwei
Funktionen heraus geschrieben werden.
Die sichere Methode ist immer, solche Sequenzen atomar zu kapseln.
Gerade bei 32Bittern hat man ja eh nicht extreme Echtzeit-Anforderungen,
d.h. eine kurze Interruptsperre tut nicht weh.
Peter
NurEinGast schrieb:> Ein Vorschlag :>> In der AddCmd inkrementierst Du nur EndIdx> In der IRQ inkrementierst Du nur StartIdx
Stimmt, mache ich auch so.
Count, BufferSize sind vollkommen überflüssig.
Beitrag "AVR-GCC: UART mit FIFO"
Peter
Hallo Karl Heinz
Super, vielen Dank für deine schnelle Rückmeldung und die detaillierte
Problemanalyse!!!
Es ist genau so wie du sagtes: Ich habe mir gedacht, das Inkremenieren
der Variable wird atomar ausgeführt, du hast aber recht, dazwischen
werden noch Befehle ausgeführt.
F1: Interessanterweise hat Count nie mehr als um 1 nicht gestimmt. Wenn
das Programm unendlich lange läuft, dann müsste Count doch mit der Zeit
davonlaufen - oder? Ich habe es noch nicht geprüft, werde es aber tun.
F2: Wenn ich Count nun wie beschrieben am Ende ausführe, reicht es dann
aus, dass der Code Thread-Safe - oder wie immer man hierzu sagt - ist?
Theoretisch exisiert das von Karl Heinz beschriebene Problem ja immer
noch. Dann halt erst etwas später.
if (len > 2) memcpy((char*)&CmdBuffer.Cmd CmdBuffer.EndIdx++;
8
CmdBuffer.EndIdx &= (MAX_CMD_BUFFER-1);
9
CmdBuffer.Count++; // Count erst am Ende inkrementieren
10
return 0;
11
}
12
else
13
return -1;
14
}
F3: Ich habe aus Interesse nun auch mal folgendermassen codiert und der
Code wurde auc richtig ausgeführt. "Unschön" ist allerdings, dass die
Routine Einfluss auf das ISR-Timing hat weil der Timer zurückgesetzt
wird, was ich eigentlich nicht will.
if (len > 2) memcpy((char*)&CmdBuffer.Cmd[CmdBuffer.EndIdx].Data,&Data[2], len-2);
8
T0MCR = 0; // Timer0 Interrupts ausschalten
9
CmdBuffer.Count++;
10
CmdBuffer.EndIdx++;
11
CmdBuffer.EndIdx &= (MAX_CMD_BUFFER-1);
12
T0TCR = 0x02; // Timer 0 aus und Reset
13
T0MCR = 3; // Interrupts on MR0-Overflow einschalten
14
T0TCR = 0x01; // Zähler einschalten
15
return 0;
16
}
17
else
18
return -1;
19
}
Was meint ihr dazu? Ich würde Variante 1 bevorzugen, wenn sie "sicher"
ist.
..der müsste man gar in AddCmd mit Hilfe eines Flags prüfen, ob die ISR
aktiv ist oder geht es einfacher? Vielleicht anstatt
Hallo zusammen
Nun habe ich zu lagen am Text getippt und danach gelesen:) Trotzdem sind
mir noch ein paar offene Fragen:)
#NurEinGast und mio
Ja, das habe ich mir auch schon gedacht. Würde das aber ausreichen?
@Peter
Den Interrupt auszuschalten, dagegen hätte ich nichts.
Reicht es aus, den Interrupt vor dem Inkremenieren auszuschalten und
anschliessend wieder einzuschalten?
Ein Problem, betreffend den LPC2148 bzw. ARM7, ist mir dabei
aufgefallen:
Mit dem Code aus Variante 2 wird der Timer gestoppt und auf den Wert 0
zurückgesetzt. Nachdem der Timer
wieder gestartet wird legt er los.
Damit verändere ich also im für einen Interruptzyklus die Frequenz weil
der Reload-Wert verändert wurde.
Führe ich den Reset nicht aus, dann läuft Timer 0 über den Wert im
Match-Register MR0 und die ISR wird dann länger nicht mehr aufgerufen.
Wenn für diese Anwendung folgende Codezeile ausreicht, dann mache ich es
lieber so als die ISR zu beinflussen. Das ganze ist dann auch nicht so
Controllerspezifisch.
1
// neu
2
if (CmdBuffer.StartIdx != CmdBuffer.EndIdx)
3
{
4
// hole und verarbeite Kommando
5
}
Count und Bufferzize sind nicht notwendig, das stimmt.
Buffersize hatte ich anstatt des defines MAX_CMD_BUFFER zum Testen
verwendet und Count der besseren Übersicht:)
Vielen Dank nochmals
Geri
Geri schrieb:> Führe ich den Reset nicht aus, dann läuft Timer 0 über den Wert im> Match-Register MR0 und die ISR wird dann länger nicht mehr aufgerufen.
Das würde aber den Interrupt Mechanismus ad absurdum führen.
Normalerweise ist es so, dass das Auftreten einer Interrupt-Bedinung in
einem CPU Flag erst mal nur registriert wird. Sind die Interrupts
freigegeben, dann wird die zugehörige ISR ausgeführt. Das Flag wird aber
auf jeden Fall gesetzt, selbst wenn die Interrupts momentan gesperrt
sind. Mit dem freigeben der Interrupts wird die aufgetretene
Interrupt-Bedingung anhand des Flags erkannt und die ISR quasi 'im
Nachhinein' ausgeführt.
Durchforste mal dein Datenblatt, wie das auf deinem µC läuft.
Wahrscheinlich ziemlich genau gleich.
> Wenn für diese Anwendung folgende Codezeile ausreicht, dann mache ich es> lieber so als die ISR zu beinflussen. Das ganze ist dann auch nicht so> Controllerspezifisch.>
1
> // neu
2
> if (CmdBuffer.StartIdx != CmdBuffer.EndIdx)
3
> {
4
> // hole und verarbeite Kommando
5
> }
6
>
>> Count und Bufferzize sind nicht notwendig, das stimmt.
ist sowieso die bessere Lösung.
Altes Datenbankprinzip: Speichere dieselbe Information nie 2-mal. Im
besten Fall gewinnst du dadurch nichts, im schlechtesten Fall stimmen
die beiden Vorkommen nicht überein.
Aus CmdBuffer.StartIdx und CmdBuffer.EndIdx kannst du dir den Count
berechnen, falls du ihn irgendwann mal brauchen würdest.
Das Tupel (CmdBuffer.StartIdx; CmdBuffer.EndIdx) und CmdBuffer.Count
repräsentieren also die gleiche Information.
Geri schrieb:> "Unschön" ist allerdings, dass die> Routine Einfluss auf das ISR-Timing hat weil der Timer zurückgesetzt> wird, was ich eigentlich nicht will.
Daher macht man sowas nicht. Wenn Du im Interrupt einen Timer händisch
zurücksetzt, hast Du immer auch eine variable Interruptlatenz.
Und spätestens, wenn es noch andere Interrupts gibt, variiert diese noch
viel stärker.
Entweder Du benutzt einen Clear-on-Compare Modus, d.h. der Timer wird
exakt nach dem Comparewert von der Timerhardware zurückgesetzt.
Oder Du läßt den Timer durchlaufen und addierst das neue Compare auf das
alte Compare.
In beiden Fällen hat die Interruptlatenz keinerlei Einfuß.
Der Interrupt muß nur vor dem nächsten Compare ausgeführt worden sein.
Peter
Kann man auch mit Synchronisierungobjekten wie mutexe und semaphore
machen bzw. kann man auch sowas wie ne critical section selber basteln.
Brauchst z.B. nur kurz die Interupts zu deaktivieren. Auch ein Locked
Flag mit zugehöriger Funktion isLocked() ist einfach zu implementieren.
if (GetCmdBufCount() >= (VMAX_CMD_BUFFER-1)) return;
GetCmdBufCount liefert die Anzahl der Elmente im Puffer.
Aufgefallen ist mir dabei noch folgendes:
1
uint16_t Count;
2
Count = GetCmdBufCount();
3
4
if (Count != GetCmdBufCount)
5
{
6
// Werte ungleich
7
}
Dieser Fall tritt nach wie vor auf.
Nur thoretische betrachtet: Könnte es bei extrem hoher
Interruptbelastung vorkommen, dass die obige Bedingung für das Schreiben
des Puffers Problem macht?
Hast recht mit den redundaten Definitionen. Mir fällt hier auch kein
Grund ein wieso man es so machen soll.
@Peter
Meine ISR arbeitet momentan auch mit dem Timer-Match-Register. Dieser
Wert wird in der ISR laufend verändert.
Die Idee mit dem dazu-Addieren finde ich auch besser. Wollte am Anfang
den sicheren Weg gehen (Reentrance vermeiden) weil ich den Code zuerst
stabil haben wollte und dann optimieren:) Meine ISR wird mit max 150 kHz
aufgerufen. Nebenher laufen noch Interrrupts der seriellen Schnittstelle
und ein weiter Timer (1 kHz).
@Uwe
Danke für den Hinweis. ISR ausschalten möchte ich nicht wenn es nicht
notwendig ist. Schau ich mir aber trotzdem wieder mal an wie es in
embedded-Systemen OS gemacht wird.
mach da gleich noch eine Klammer rundherum, die die beiden Teilausdrücke
vom & aneinander kettet.
IM übrigen denke ich, dass der Ausdruck nicht stimmt. Wenn das ein
Ringbuffer ist, dann kann EndIdx kleiner als StartIdx sein. In dem Fall
liefert dir der Ausdruck IMHO das falsche Ergebnis.
Im Senden:
> if (GetCmdBufCount() >= (VMAX_CMD_BUFFER-1)) return;
Wozu? Ob der BUffer voll ist oder nicht, sagen dir EndIdx und StartIdx.
Den Count brauchst du hier nicht explizit.
Darf ich vorschlagen, dass du dir einen Ringbuffer auf dem Papier
aufmalst und einfach mal ein paar Einfüge und Ausleseoperationen
durchspielst wobei du darauf achtest, den Count nicht zu benutzen,
sondern du dich einfach überall fragst, wie du mittels der beiden Indexe
an die gewünschte Information kommst?
> Nur thoretische betrachtet: Könnte es bei extrem hoher> Interruptbelastung vorkommen, dass die obige Bedingung für das Schreiben> des Puffers Problem macht?
Natürlich.
IMMER wenn ein Wert in einer Berechnung vorkommt UND in einer Interrupt
Routine benutzt verändert wird (oder umgekehrt) kann es zu dem Effekt
kommen, dass du mit falschen Werten rechnest.
> ISR ausschalten möchte ich nicht wenn es nicht notwendig ist.
Du gewöhnst duch besser daran, dass es Fälle gibt, in denen du für ein
paar µs die Interrupts mal kurz ausschalten musst. Sagt ja keiner, dass
das ausarten muss. Aber für kurze Zeit ist das völlig normal.
Hallo Karlheinz
Vielen Dank für deine Rückmeldung!
Danke für den Hinweis mit der Klammer
>IM übrigen denke ich, dass der Ausdruck nicht stimmt. Wenn das ein>Ringbuffer ist, dann kann EndIdx kleiner als StartIdx sein. In dem Fall>liefert dir der Ausdruck IMHO das falsche Ergebnis.
An den Fall, dass EndIdx kleiner als StartIdx ist habe ich schon
gedacht.
Ich denke, die Definition deckt beide Fälle ab. Habe zumindest ein paar
Fälle durchgespielt. Habe es nun aber doch nochmals geprüft:) Ein
Nachtteil ergibt sich halt, dass ein Element weniger geschrieben werden
kann, wenn ich beim Auslesen nach StartIdx ! EndIdx abfrage.. ..oder
heute ist es bereits zu spät..
Bzgl. der Interrupts schau ich mir mal an, wie sich das Programm
verhält.
Count und Buffersize habe ich inzwischen eliminiert und es geht auch.
Beste Grüsse und vielen Dank nochmals für deine ausführliche Hilfe!
Geri
Geri schrieb:> @Uwe> Danke für den Hinweis. ISR ausschalten möchte ich nicht wenn es nicht> notwendig ist. Schau ich mir aber trotzdem wieder mal an wie es in> embedded-Systemen OS gemacht wird.
Um es kurz zu machen: Du hast ein Ressource (Ringpuffer) auf der Du von
2 Task/Threads, what ever, zugreifen willst. Das simpelste in der
Industrie hierfür verwendete Synchronisationsmittel dürfte wohl ein
Semaphor sein...
Vorsicht mit solchen Bedingungen im Ringpuffer:
> CmdBuffer.StartIdx != CmdBuffer.EndIdx
Wenn der Puffer ganz voll werden kann ist Start- und End-Index wieder
gleich!
Also: Bei der Prüfung, ob man noch eins einfügen kann, oder ob schon
voll ist, immer eins frei lassen.
Thread beobachten
Seitenaufteilung abschalten