diff --git a/sched/sched/sched_note.c b/sched/sched/sched_note.c index 0abd56ed6a..22d87b8264 100644 --- a/sched/sched/sched_note.c +++ b/sched/sched/sched_note.c @@ -281,9 +281,6 @@ static void note_add(FAR const uint8_t *note, uint8_t notelen) { unsigned int head; unsigned int next; -#if 0 /* Backed out for now. */ - unsigned int nxthd; -#endif #ifdef CONFIG_SMP /* Ignore notes that are not in the set of monitored CPUs */ @@ -296,48 +293,26 @@ static void note_add(FAR const uint8_t *note, uint8_t notelen) } #endif -#if 0 /* Backed out for now. This could advance the head index and wrap - * past the tail index. I am not sure how this would interact with - * note_remove(). Needs time to test and verify. - */ + /* REVISIT: In the single CPU case, the following should be safe because + * the logic is always called within a critical section, but in the SMP + * case we have protection. One option would be to precalculate and + * advancing the new head entry before writing the data into the buffer. + * That will eliminate fatal race conditions (although could result in + * single notes being corrupted harmlessly). + * + * But there is a complexity: Advancing the head pointer where the note + * buffer is almost full could advance the head to wrap beyond the tail + * leaving the buffer in a bad state. A solution to this would be to pre- + * remove entries at the tail of the buffer as necessary to make certain + * that there will be space for the new note at the beginning of the + * buffer. I am less certain that this can be done safely in the SMP + * case. + */ - DEBUGASSERT(note != NULL && notelen < CONFIG_SCHED_NOTE_BUFSIZE); - - do - { - /* Get the index to the head of the circular buffer */ - - head = g_note_info.ni_head; - - /* Pre-calculate the next head index. We do this in advance to - * protect note as it is written. In the single CPU case, this is not - * a problem because this logic is always called within a critical - * section, but in the SMP case we have less protection. pre- - * calculating the next head indexed does not eliminate all - * possibility of conflict; it is possible that one note could be lost - * or corrupted. But the integrity of the overal buffer should be - * retained. - */ - - nxthd = note_next(head, notelen); - - /* Write the next head. There are two possible race conditions: (1) - * some other CPU wrote the head after we fetched it above. That - * note from the other CPU will be lost (and this one may be - * corrupted), or (2) some other CPU will write the head after we - * write it and the write the same value. Again, one note will be - * lost or corrupted. - */ - - g_note_info.ni_head = nxthd; - } - while (nxthd != g_note_info.ni_head); -#else /* Get the index to the head of the circular buffer */ DEBUGASSERT(note != NULL && notelen < CONFIG_SCHED_NOTE_BUFSIZE); head = g_note_info.ni_head; -#endif /* Loop until all bytes have been transferred to the circular buffer */ @@ -363,11 +338,7 @@ static void note_add(FAR const uint8_t *note, uint8_t notelen) notelen--; } -#if 0 /* Backed out for now. */ - DEBUGASSERT(head == nxthd); -#else g_note_info.ni_head = head; -#endif } /****************************************************************************