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 2021/11/10 01:23:56 UTC
[GitHub] [incubator-nuttx] CheryDan opened a new pull request #4814: net/can/can_recvmsg.c : put can_readahead in critical section
CheryDan opened a new pull request #4814:
URL: https://github.com/apache/incubator-nuttx/pull/4814
## Summary
net/can/can_recvmsg.c : put can_readahead in critical section
## Impact
avoid readahead changed by CAN interrupt , which will lead to data drop , IOB memory leak and NET stack crash
## 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4814: net/can/can_recvmsg.c : put can_readahead in critical section
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4814:
URL: https://github.com/apache/incubator-nuttx/pull/4814#discussion_r746208839
##########
File path: net/can/can_recvmsg.c
##########
@@ -622,7 +623,14 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
* socket has been disconnected.
*/
+ /* The following operations must be performed with interrupts
+ * disabled because iob_tryadd_queue() may be called from an interrupt
Review comment:
iob_tryalloc_qentry already hold critical section, why we need hold here again?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4814: net/can/can_recvmsg.c : put can_readahead in critical section
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4814:
URL: https://github.com/apache/incubator-nuttx/pull/4814#issuecomment-965969812
Let's merge two patch into one.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4814: net/can/can_recvmsg.c : put can_readahead in critical section
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4814:
URL: https://github.com/apache/incubator-nuttx/pull/4814#discussion_r746265219
##########
File path: net/can/can_recvmsg.c
##########
@@ -622,7 +623,14 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
* socket has been disconnected.
*/
+ /* The following operations must be performed with interrupts
+ * disabled because iob_tryadd_queue() may be called from an interrupt
Review comment:
we should fix IOB function instead, so not only can socket, but also other module which invoke IOB function can work 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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4814: net/can/can_recvmsg.c : put can_readahead in critical section
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4814:
URL: https://github.com/apache/incubator-nuttx/pull/4814#discussion_r746812221
##########
File path: mm/iob/iob_free_queue_qentry.c
##########
@@ -57,6 +57,7 @@ void iob_free_queue_qentry(FAR struct iob_s *iob,
FAR struct iob_qentry_s *prev = NULL;
FAR struct iob_qentry_s *qentry;
+ irqstate_t iflags = enter_critical_section();
Review comment:
change all iflags to flags, and remove the first and second patch
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #4814: net/can/can_recvmsg.c : put can_readahead in critical section
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4814:
URL: https://github.com/apache/incubator-nuttx/pull/4814
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CheryDan commented on a change in pull request #4814: net/can/can_recvmsg.c : put can_readahead in critical section
Posted by GitBox <gi...@apache.org>.
CheryDan commented on a change in pull request #4814:
URL: https://github.com/apache/incubator-nuttx/pull/4814#discussion_r746211212
##########
File path: net/can/can_recvmsg.c
##########
@@ -622,7 +623,14 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct msghdr *msg,
* socket has been disconnected.
*/
+ /* The following operations must be performed with interrupts
+ * disabled because iob_tryadd_queue() may be called from an interrupt
Review comment:
iob_tryalloc_qentry is already in CAN interrupt ,but iob_remove_queue in can_recvmsg does not hold critical section
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org