You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/09/10 13:50:36 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #1747: sched: Rename note_add to sched_note_add
xiaoxiang781216 opened a new pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747
## Summary
to better match other sched_note_* function
## Impact
## Testing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
patacongo removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690861100
First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs). Very amateurish.Sent from Samsung tablet.
-------- Original message --------From: Xiang Xiao <no...@github.com> Date: 9/10/20 9:20 PM (GMT-06:00) To: apache/incubator-nuttx <in...@noreply.github.com> Cc: Subscribed <su...@noreply.github.com> Subject: Re: [apache/incubator-nuttx] sched: Rename note_add to sched_note_add (#1747)
I am referring to changing 4e45a66
This schedule_note_xxxx interface has been used for years. It is an API to board-specific logic it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
What it means is It is an API to board-specific logic that should NOT waist any time.
The board interface may change sometime if there is a good techinical reason, for example here is your change:
commit ed5d00edd877c0489b8281c476a5d3dd35628bca
Author: David Sidrane <Da...@NscDg.com>
Date: Tue Aug 4 10:14:54 2020 -0700
board_crashdump:use consistent type from outer function for file name
which also break other people's code. Should we forbid this change and revert it? I don't think so, because the change is good from the technology perspective.
The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
That is to note an event of a specific kind.
Feel free to extend the use case but not change the interface. That is a breaking change.
The break can't avoid to get the better design or fix the new challenge, here is the partial list the recent change which break the compatibility in some way:
797bf44
797bf44
a0ce81d
875828b
f92dba2
153eee6
c2244a2
What should we do with these patches?
—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or unsubscribe.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690865979
> First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs).
It isn't wrong, I am use the term from queue definition in C++ standard:
https://en.cppreference.com/w/cpp/container/queue
Also, it's a term issue with the context which doesn't affect you understand what I mean.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
patacongo removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690861100
First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs). Very amateurish.Sent from Samsung tablet.
-------- Original message --------From: Xiang Xiao <no...@github.com> Date: 9/10/20 9:20 PM (GMT-06:00) To: apache/incubator-nuttx <in...@noreply.github.com> Cc: Subscribed <su...@noreply.github.com> Subject: Re: [apache/incubator-nuttx] sched: Rename note_add to sched_note_add (#1747)
I am referring to changing 4e45a66
This schedule_note_xxxx interface has been used for years. It is an API to board-specific logic it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
What it means is It is an API to board-specific logic that should NOT waist any time.
The board interface may change sometime if there is a good techinical reason, for example here is your change:
commit ed5d00edd877c0489b8281c476a5d3dd35628bca
Author: David Sidrane <Da...@NscDg.com>
Date: Tue Aug 4 10:14:54 2020 -0700
board_crashdump:use consistent type from outer function for file name
which also break other people's code. Should we forbid this change and revert it? I don't think so, because the change is good from the technology perspective.
The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
That is to note an event of a specific kind.
Feel free to extend the use case but not change the interface. That is a breaking change.
The break can't avoid to get the better design or fix the new challenge, here is the partial list the recent change which break the compatibility in some way:
797bf44
797bf44
a0ce81d
875828b
f92dba2
153eee6
c2244a2
What should we do with these patches?
—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or unsubscribe.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690927791
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#discussion_r486761620
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#discussion_r486761620
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690861100
First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs). Very amateurish.Sent from Samsung tablet.
-------- Original message --------From: Xiang Xiao <no...@github.com> Date: 9/10/20 9:20 PM (GMT-06:00) To: apache/incubator-nuttx <in...@noreply.github.com> Cc: Subscribed <su...@noreply.github.com> Subject: Re: [apache/incubator-nuttx] sched: Rename note_add to sched_note_add (#1747)
I am referring to changing 4e45a66
This schedule_note_xxxx interface has been used for years. It is an API to board-specific logic it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
What it means is It is an API to board-specific logic that should NOT waist any time.
The board interface may change sometime if there is a good techinical reason, for example here is your change:
commit ed5d00edd877c0489b8281c476a5d3dd35628bca
Author: David Sidrane <Da...@NscDg.com>
Date: Tue Aug 4 10:14:54 2020 -0700
board_crashdump:use consistent type from outer function for file name
which also break other people's code. Should we forbid this change and revert it? I don't think so, because the change is good from the technology perspective.
The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
That is to note an event of a specific kind.
Feel free to extend the use case but not change the interface. That is a breaking change.
The break can't avoid to get the better design or fix the new challenge, here is the partial list the recent change which break the compatibility in some way:
797bf44
797bf44
a0ce81d
875828b
f92dba2
153eee6
c2244a2
What should we do with these patches?
—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or unsubscribe.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-691503520
@davids5 do you have more comment? If not, please merge it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690927791
@xiaoxiang781216 - This is foolish, fixing an API is completely different then hijacking it.
In the case you are referencing, the the fix is to change the type. - The compiler tells the user what to fix - Done. We do this all the time.
In this situation it can not be fixed externally. (it can be but using `weak`, or `wrapping` seem almost as foolish as this discussion)
You are **hijacking** a the functions names of EXTERNAL call outs. What do you expect, the remedy to be?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690934444
@xiaoxiang781216 Because you split out the commits discussed herein to a different PR All the comments are not appropriate anymore. Please close this PR and open a new one for this change (which from my perspective, is fine )
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690849523
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690849523
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690934444
@xiaoxiang781216 Because you split out the commits discussed herein to a different PR All the comments are not appropriate anymore. Please close this PR and open a new one for this change (which from my perspective, is fine )
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690598441
> The is a huge breaking change, it a callout not a subsystem. If you want to have an internal implementation subsystem that is fine but you can not break the interface.
Why? This is an internal function expose to the public header files just a couples of days ago, please review the related patch again: https://github.com/apache/incubator-nuttx/pull/1520 To keep https://github.com/apache/incubator-nuttx/pull/1520 change as small as possible, I split the rename patch to here.
BTW, there is no external user yet. so why do you think this patch will make a huge breaking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690598441
> The is a huge breaking change, it a callout not a subsystem. If you want to have an internal implementation subsystem that is fine but you can not break the interface.
Why? This is an internal function expose to the public header files just a couples of days ago, please review the related patch again: https://github.com/apache/incubator-nuttx/pull/1520. To keep https://github.com/apache/incubator-nuttx/pull/1520 change as small as possible, I split the rename patch to here.
BTW, there is no external user yet, why do you think this patch will make a huge breaking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690865979
> First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs).
It isn't wrong, I am use the term from queue definition in C++ standard:
https://en.cppreference.com/w/cpp/container/queue
Also, the term issue shouldn't affect you understand what I mean with the context information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690977394
I will move all comment to PR https://github.com/apache/incubator-nuttx/pull/1520.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690615821
I am referring to changing https://github.com/apache/incubator-nuttx/pull/1747/commits/4e45a66e5142b5d7df37a07b17e30d3fa4bc4262
This schedule_note_xxxx interface has been used for years. It is an API to `board-specific logic` it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
What it means is It is an API to **`board-specific logic`** that should **NOT** waist any time.
The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
That is to note an event of a **specific** kind.
Feel free to extend the use case but not change the interface. That is a breaking change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690598441
> The is a huge breaking change, it a callout not a subsystem. If you want to have an internal implementation subsystem that is fine but you can not break the interface.
Why? This is an internal function expose to the public header files just a couples of days ago, please review the related patch again: https://github.com/apache/incubator-nuttx/pull/1520. To keep https://github.com/apache/incubator-nuttx/pull/1520 change as small as possible, I split the rename patch to here.
BTW, there is no external user yet. so why do you think this patch will make a huge breaking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690849523
> I am referring to changing [4e45a66](https://github.com/apache/incubator-nuttx/commit/4e45a66e5142b5d7df37a07b17e30d3fa4bc4262)
>
> This schedule_note_xxxx interface has been used for years. It is an API to `board-specific logic` it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
>
> What it means is It is an API to **`board-specific logic`** that should **NOT** waist any time.
>
The board interface may change sometime if there is a good techinical reason, for example here is your change:
```
commit ed5d00edd877c0489b8281c476a5d3dd35628bca
Author: David Sidrane <Da...@NscDg.com>
Date: Tue Aug 4 10:14:54 2020 -0700
board_crashdump:use consistent type from outer function for file name
```
which also break other people's code. Should we forbid this change and revert it? I don't think so, because the change is good from the technology perspective.
> The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
> That is to note an event of a **specific** kind.
>
> Feel free to extend the use case but not change the interface. That is a breaking change.
The break can't avoid to get the better design or fix the new challenge, here is the partial list the recent change which break the compatibility in some way:
https://github.com/apache/incubator-nuttx/commit/797bf447d1171e46e42e6bee1036c5511f23f1d1
https://github.com/apache/incubator-nuttx/commit/797bf447d1171e46e42e6bee1036c5511f23f1d1
https://github.com/apache/incubator-nuttx/commit/a0ce81d65914fdedab4f7418442e3fdc12331b71
https://github.com/apache/incubator-nuttx/commit/875828b6981c90238137b22b047158b95e7e0d45
https://github.com/apache/incubator-nuttx/commit/f92dba212d1551f541dfedd61a6ad1401d8aaff4
https://github.com/apache/incubator-nuttx/commit/153eee6de2ec204a886f0edc4e7a3021ab5e8fc4
https://github.com/apache/incubator-nuttx/commit/c2244a2382cf9c62937cc558cc947fae9f211b81
What should we do with these patches?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690849523
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690865979
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ghn-certi commented on a change in pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
ghn-certi commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#discussion_r486633101
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
```suggestion
NOTE: This is an internal OS interface and is called at very
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690849523
> I am referring to changing [4e45a66](https://github.com/apache/incubator-nuttx/commit/4e45a66e5142b5d7df37a07b17e30d3fa4bc4262)
>
> This schedule_note_xxxx interface has been used for years. It is an API to `board-specific logic` it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
>
> What it means is It is an API to **`board-specific logic`** that should **NOT** waist any time.
>
The board interface may change sometime if there is a good techinical reason, for example here is your change:
```
commit ed5d00edd877c0489b8281c476a5d3dd35628bca
Author: David Sidrane <Da...@NscDg.com>
Date: Tue Aug 4 10:14:54 2020 -0700
board_crashdump:use consistent type from outer function for file name
```
which also break other people's code. Should we forbid this change and revert it? I don't think so, because the change is good from the technology perspective.
> The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
> That is to note an event of a **specific** kind.
>
> Feel free to extend the use case but not change the interface. That is a breaking change.
The break can't avoid to get the better design or fix the new challenge sometime, here is the partial list the recent change which break the compatibility in some way:
https://github.com/apache/incubator-nuttx/commit/797bf447d1171e46e42e6bee1036c5511f23f1d1
https://github.com/apache/incubator-nuttx/commit/797bf447d1171e46e42e6bee1036c5511f23f1d1
https://github.com/apache/incubator-nuttx/commit/a0ce81d65914fdedab4f7418442e3fdc12331b71
https://github.com/apache/incubator-nuttx/commit/875828b6981c90238137b22b047158b95e7e0d45
https://github.com/apache/incubator-nuttx/commit/f92dba212d1551f541dfedd61a6ad1401d8aaff4
https://github.com/apache/incubator-nuttx/commit/153eee6de2ec204a886f0edc4e7a3021ab5e8fc4
https://github.com/apache/incubator-nuttx/commit/c2244a2382cf9c62937cc558cc947fae9f211b81
What should we do with these patches?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690861100
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690865979
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690927791
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690849523
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690927791
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690598441
> The is a huge breaking change, it a callout not a subsystem. If you want to have an internal implementation subsystem that is fine but you can not break the interface.
Why? This is an internal function expose to the public header files just a couples of days ago, please review the related patch again: https://github.com/apache/incubator-nuttx/pull/1520. To keep https://github.com/apache/incubator-nuttx/pull/1520 change as small as possible, I split the rename patch to here.
BTW, there is no external user yet, why do you think this patch will make a huge breaking.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690927791
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690849523
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690308550
@davids5 fix the comment in Kconfig.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690861100
First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs). Very amateurish.Sent from Samsung tablet.
-------- Original message --------From: Xiang Xiao <no...@github.com> Date: 9/10/20 9:20 PM (GMT-06:00) To: apache/incubator-nuttx <in...@noreply.github.com> Cc: Subscribed <su...@noreply.github.com> Subject: Re: [apache/incubator-nuttx] sched: Rename note_add to sched_note_add (#1747)
I am referring to changing 4e45a66
This schedule_note_xxxx interface has been used for years. It is an API to board-specific logic it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
What it means is It is an API to board-specific logic that should NOT waist any time.
The board interface may change sometime if there is a good techinical reason, for example here is your change:
commit ed5d00edd877c0489b8281c476a5d3dd35628bca
Author: David Sidrane <Da...@NscDg.com>
Date: Tue Aug 4 10:14:54 2020 -0700
board_crashdump:use consistent type from outer function for file name
which also break other people's code. Should we forbid this change and revert it? I don't think so, because the change is good from the technology perspective.
The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
That is to note an event of a specific kind.
Feel free to extend the use case but not change the interface. That is a breaking change.
The break can't avoid to get the better design or fix the new challenge, here is the partial list the recent change which break the compatibility in some way:
797bf44
797bf44
a0ce81d
875828b
f92dba2
153eee6
c2244a2
What should we do with these patches?
—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or unsubscribe.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690927791
@xiaoxiang781216 - This is foolish, fixing an API is completely different then hijacking it.
In the case you are referencing, the the fix is to change the type. - The compiler tells the user what to fix - Done. We do this all the time.
In this situation it can not be fixed externally. (it can be but using `weak`, or `wrapping` seem almost as foolish as this discussion)
You are **hijacking** a the functions names of EXTERNAL call outs. What do you expect, the remedy to be?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690615821
I am referring to changing https://github.com/apache/incubator-nuttx/pull/1747/commits/4e45a66e5142b5d7df37a07b17e30d3fa4bc4262
This schedule_note_xxxx interface has been used for years. It is an API to `board-specific logic` it does not require a de-multiplexer the APIs are focused. Please do not misinterpret the note: "NOTE: This is an internal OS interfaces and are called at at very critical locations in the OS." (it is the same as SPI selects that are defined by the board)
What it means is It is an API to **`board-specific logic`** that should **NOT** waist any time.
The proposal of 1) queuing a data type and 2) de-queuing it and 3) de-multiplexing it not useful in the original intent:
That is to note an event of a **specific** kind.
Feel free to extend the use case but not change the interface. That is a breaking change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690865979
> First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs).
It isn't wrong, I am use the term from queue definition from C++ standard:
https://en.cppreference.com/w/cpp/container/queue
Also, it's a term issue with the context which doesn't affect you understand what I mean.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo removed a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
patacongo removed a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690861100
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690865979
> First, the naming is totally wrong. You cannot push to or pop from a FIFO. That is incorrect. Push and pop refer only stacks (LIFOs).
It isn't wrong, I am use the term from queue definition in C++ standard:
https://en.cppreference.com/w/cpp/container/queue
Also, the term issue shouldn't affect you understand what I mean with the full context information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690598441
> interface
Why? This is an internal function expose to the public header files just a couples of days ago, please review the related patch again: https://github.com/apache/incubator-nuttx/pull/1520
There is no external user yet. To keep https://github.com/apache/incubator-nuttx/pull/1520 change as small as possible, I split the rename patch to here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#issuecomment-690977394
I will move all comment to PR https://github.com/apache/incubator-nuttx/pull/1520.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747#discussion_r486761620
##########
File path: sched/Kconfig
##########
@@ -894,22 +894,11 @@ config SCHED_INSTRUMENTATION
---help---
Enables instrumentation in scheduler to monitor system performance.
If enabled, then the board-specific logic must provide the following
- functions (see include/sched.h):
+ functions (see include/sched_note.h):
- void sched_note_start(FAR struct tcb_s *tcb);
- void sched_note_stop(FAR struct tcb_s *tcb);
- void sched_note_suspend(FAR struct tcb_s *tcb);
- void sched_note_resume(FAR struct tcb_s *tcb);
+ void sched_note_add(FAR const void *note, size_t notelen);
- If CONFIG_SMP is enabled, then these additional interfaces are
- expected:
-
- void sched_note_cpu_pause(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_paused(FAR struct tcb_s *tcb);
- void sched_note_cpu_resume(FAR struct tcb_s *tcb, int cpu);
- void sched_note_cpu_resumed(FAR struct tcb_s *tcb);
-
- NOTE: These are internal OS interfaces and are called at at very
+ NOTE: This is an internal OS interfaces and are called at at very
Review comment:
@ghn-certi, I split the Kconfig change to another patch: please give your feedback here: https://github.com/apache/incubator-nuttx/pull/1751
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #1747: sched: Rename note_add to sched_note_add
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #1747:
URL: https://github.com/apache/incubator-nuttx/pull/1747
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org