Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
xiaoxiang781216 merged PR #16333: URL: https://github.com/apache/nuttx/pull/16333 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
jlaitine commented on code in PR #16333:
URL: https://github.com/apache/nuttx/pull/16333#discussion_r2079263741
##
sched/signal/sig_dispatch.c:
##
@@ -374,6 +339,76 @@ static FAR sigpendq_t *nxsig_add_pendingsignal(FAR struct
tcb_s *stcb,
return sigpend;
}
+/
+ * Name: nxsig_alloc_dyn_pending
+ *
+ * Description:
+ * Dynamically allocate more pending signal and pending sigaction
+ * structures, if there are no more left. Note that this leaves the
+ * the critical section for the allocation. During that time it is
+ * it is possible that structures are freed, or another signalling thread
+ * allocates more structures. This is not an issue, any extra pending
+ * structures are freed after they get used.
+ *
+ * Assumptions:
+ * Called with g_sigpendingsignal and g_sigpendingaction locked by
+ * critical section.
+ *
+ /
+
+static irqstate_t nxsig_alloc_dyn_pending(irqstate_t flags)
+{
+ if (!up_interrupt_context())
Review Comment:
I believe debugassert is proper here; *if* this was called from idle thread
(which should never happen) and there were no available pending signals, it
would just lead to a potential crash at the point a pending signal would need
to be added...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
jlaitine commented on code in PR #16333:
URL: https://github.com/apache/nuttx/pull/16333#discussion_r2079183313
##
sched/signal/sig_dispatch.c:
##
@@ -374,6 +339,76 @@ static FAR sigpendq_t *nxsig_add_pendingsignal(FAR struct
tcb_s *stcb,
return sigpend;
}
+/
+ * Name: nxsig_alloc_dyn_pending
+ *
+ * Description:
+ * Dynamically allocate more pending signal and pending sigaction
+ * structures, if there are no more left. Note that this leaves the
+ * the critical section for the allocation. During that time it is
+ * it is possible that structures are freed, or another signalling thread
+ * allocates more structures. This is not an issue, any extra pending
+ * structures are freed after they get used.
+ *
+ * Assumptions:
+ * Called with g_sigpendingsignal and g_sigpendingaction locked by
+ * critical section.
+ *
+ /
+
+static irqstate_t nxsig_alloc_dyn_pending(irqstate_t flags)
+{
+ if (!up_interrupt_context())
Review Comment:
Yes, but this function cannot be called from idle thread; this was also not
checked before. Maybe add DEBUGASSERT instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
xiaoxiang781216 commented on code in PR #16333:
URL: https://github.com/apache/nuttx/pull/16333#discussion_r2079100828
##
sched/signal/sig_dispatch.c:
##
@@ -374,6 +339,76 @@ static FAR sigpendq_t *nxsig_add_pendingsignal(FAR struct
tcb_s *stcb,
return sigpend;
}
+/
+ * Name: nxsig_alloc_dyn_pending
+ *
+ * Description:
+ * Dynamically allocate more pending signal and pending sigaction
+ * structures, if there are no more left. Note that this leaves the
+ * the critical section for the allocation. During that time it is
+ * it is possible that structures are freed, or another signalling thread
+ * allocates more structures. This is not an issue, any extra pending
+ * structures are freed after they get used.
+ *
+ * Assumptions:
+ * Called with g_sigpendingsignal and g_sigpendingaction locked by
+ * critical section.
+ *
+ /
+
+static irqstate_t nxsig_alloc_dyn_pending(irqstate_t flags)
+{
+ if (!up_interrupt_context())
Review Comment:
idle task can't call kmm_malloc since kmm_malloc will try to hold the mutex.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
jlaitine commented on PR #16333: URL: https://github.com/apache/nuttx/pull/16333#issuecomment-2861984535 I dropped the patches on an older commit to see if it now passes the CI; it looks to me that there is something wrong with the current master. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
jlaitine commented on code in PR #16333:
URL: https://github.com/apache/nuttx/pull/16333#discussion_r2078911248
##
sched/signal/sig_dispatch.c:
##
@@ -374,6 +339,76 @@ static FAR sigpendq_t *nxsig_add_pendingsignal(FAR struct
tcb_s *stcb,
return sigpend;
}
+/
+ * Name: nxsig_alloc_dyn_pending
+ *
+ * Description:
+ * Dynamically allocate more pending signal and pending sigaction
+ * structures, if there are no more left. Note that this leaves the
+ * the critical section for the allocation. During that time it is
+ * it is possible that structures are freed, or another signalling thread
+ * allocates more structures. This is not an issue, any extra pending
+ * structures are freed after they get used.
+ *
+ * Assumptions:
+ * Called with g_sigpendingsignal and g_sigpendingaction locked by
+ * critical section.
+ *
+ /
+
+static irqstate_t nxsig_alloc_dyn_pending(irqstate_t flags)
+{
+ if (!up_interrupt_context())
Review Comment:
hm, why does idle task dispatch signals?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
xiaoxiang781216 commented on code in PR #16333:
URL: https://github.com/apache/nuttx/pull/16333#discussion_r2078092232
##
sched/signal/sig_dispatch.c:
##
@@ -374,6 +339,76 @@ static FAR sigpendq_t *nxsig_add_pendingsignal(FAR struct
tcb_s *stcb,
return sigpend;
}
+/
+ * Name: nxsig_alloc_dyn_pending
+ *
+ * Description:
+ * Dynamically allocate more pending signal and pending sigaction
+ * structures, if there are no more left. Note that this leaves the
+ * the critical section for the allocation. During that time it is
+ * it is possible that structures are freed, or another signalling thread
+ * allocates more structures. This is not an issue, any extra pending
+ * structures are freed after they get used.
+ *
+ * Assumptions:
+ * Called with g_sigpendingsignal and g_sigpendingaction locked by
+ * critical section.
+ *
+ /
+
+static irqstate_t nxsig_alloc_dyn_pending(irqstate_t flags)
+{
+ if (!up_interrupt_context())
Review Comment:
need check idle_task too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] sched/signal: signal dispatch cleanups [nuttx]
jlaitine commented on PR #16333: URL: https://github.com/apache/nuttx/pull/16333#issuecomment-2858444015 No changes, just rebase to master to trigger CI again, it failed to something not related to this PR -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
