Fix usr hrt #866
Fix usr hrt #866
Conversation
…g mavlink Signed-off-by: Jukka Laitinen <[email protected]>
| /* Remove potential inflight entry as well */ | ||
| sq_rem(&e->list_item, &callout_inflight); | ||
| kmm_free(e); |
There was a problem hiding this comment.
Is there some reason this was removed? It's not immediately obvious.
There was a problem hiding this comment.
I was thinking that it is not safe to do that inside of the loop, where the next iteration pointer is taken from the e which was just deleted (even though inside spinlock and assuming that "free" doesn't alter the contents of that memory).
The deletion should be unnecessary anyways, since it should not grow the maximum used kernel heap; the next allocation is anyhow taken from the freelist
There was a problem hiding this comment.
I can, of course, do the deletion as well in safe manner if you like!
There was a problem hiding this comment.
It is fine to me to not free the entries at all, was just wondering why the line was removed
| struct usr_hrt_call *e; | ||
| irqstate_t flags; | ||
|
|
||
| flags = spin_lock_irqsave_wo_note(&g_hrt_ioctl_lock); | ||
|
|
||
| while ((e = (void *)sq_remfirst(&callout_queue))) { | ||
| sq_for_every(&callout_queue, queued) { | ||
| kmm_free(deleted); |
There was a problem hiding this comment.
Should this be:
if (deleted) {
kmm_free(deleted);
deleted = NULL;
}
There was a problem hiding this comment.
could be... freeing a NULLptr should be ok with every free function though, so it is redundant to add if
There was a problem hiding this comment.
I think kmm_free has some kind of debugassert i.e. not posix compliant. Fine by me either way.
1. When moving the list item to "inflight" queue, it must be also removed from "callout" queue. Otherwise the "callout" queue is corrupt 2. In HRT_CANCEL, the items need to be removed also from the inflight queue, in case the HRT has been triggered already, but the client is not waiting for the event 3. HRT_UNREGISTER can't just clear the whole callout_queue, since it is a kernel list shared between processes Signed-off-by: Jukka Laitinen <[email protected]>
Unsubscribe & unadvertise topics which were subscribed/advertised in the constructor Signed-off-by: Jukka Laitinen <[email protected]>
The interrupt handler needs to take the spinlock as well, as another CPU can be futzing around with the lists when another CPU takes the HRT trap.
|
added checking of "deleted" ptr acc. to the suggestion and for clarity, and squashed the commits. should be okayish now? |
No description provided.