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/02/16 06:24:37 UTC
[GitHub] [incubator-nuttx] masayuki2009 opened a new pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
masayuki2009 opened a new pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858
## Summary
- Recently I noticed that ESP32-DevKitC-32D suddenly stops
during receiving ping packets from PC after 10-20mins
- Actually, sometimes memory leak happened when some device
sent a big broadcast packet periodically on the network
- This commit fixes this issue
## Impact
- None
## Testing
- Tested with esp32-devkitc:wapi
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779640927
Hmm, Build / Linux (sim) failed but this PR does not affect sim.
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-780245519
> > I noticed that esp_wifi_free_eb() must be called before calling work_queue(), otherwise Wi-Fi stops in SMP mode after 5mins.
>
> do you understand why? i don't.
@yamt
When I refactored the logic, the normal sequence was also affected.
I thought this would work. Actually, it worked in non-SMP mode.
However, it did not work in SMP mode. Perhaps, the work_queue() is processed in parallel.
So I decided not to modify the normal sequence.
----------------------------------------------------------------
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] yamt commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-780240056
> I noticed that esp_wifi_free_eb() must be called before calling work_queue(), otherwise Wi-Fi stops in SMP mode after 5mins.
do you understand why? i don't.
----------------------------------------------------------------
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] yamt commented on a change in pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#discussion_r576629818
##########
File path: arch/xtensa/src/esp32/esp32_wlan.c
##########
@@ -493,6 +493,12 @@ static int wlan_rx_done(void *buffer, uint16_t len, void *eb)
{
nwarn("ERROR: Wlan receive %d larger than %d\n",
len, WLAN_BUF_SIZE);
+
+ if (eb)
+ {
+ esp_wifi_free_eb(eb);
+ }
+
Review comment:
i guess we should do the same for the above `if (!priv->ifup)` block as well. how do you think?
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779688028
@yamt
I've just refactored the code and pushed with -f.
----------------------------------------------------------------
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] masayuki2009 edited a comment on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 edited a comment on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779694233
@yamt
Let me check if the code works in SMP mode which I've just started for 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] masayuki2009 commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779699897
>Let me check if the code works in SMP mode which I've just started for testing
Hmm, I need to modify the code again.
I noticed that esp_wifi_free_eb() must be called before calling work_queue(), otherwise Wi-Fi stops in SMP mode after 5mins.
----------------------------------------------------------------
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 #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779649543
Fixed here: https://github.com/apache/incubator-nuttx-apps/pull/593
----------------------------------------------------------------
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] masayuki2009 edited a comment on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 edited a comment on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779640927
Hmm, Build / Linux (sim) failed but this PR does not affect sim.
```
/github/workspace/sources/apps/wireless/bluetooth/nimble/mynewt-nimble/porting/nimble/src/os_mbuf.c: In function 'os_mbuf_get':
Error: /github/workspace/sources/apps/wireless/bluetooth/nimble/mynewt-nimble/porting/nimble/src/os_mbuf.c:247:46: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
247 | os_trace_api_u32x2(OS_TRACE_ID_MBUF_GET, (uint32_t)omp,
```
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779615577
@yamt
I've just created a new 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779694233
@yamt
Let me check if the code works in SMP mode.
----------------------------------------------------------------
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] yamt commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-780279338
> > > I noticed that esp_wifi_free_eb() must be called before calling work_queue(), otherwise Wi-Fi stops in SMP mode after 5mins.
> >
> >
> > do you understand why? i don't.
>
> @yamt
>
> When I refactored the logic, the normal sequence was also affected.
> I thought this would work. Actually, it worked in non-SMP mode.
> However, it did not work in SMP mode. Perhaps, the work_queue() is processed in parallel.
>
> So I decided not to modify the normal sequence.
ok. it makes sense.
----------------------------------------------------------------
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] yamt merged pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
yamt merged pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779735021
The latest commit works in both non-SMP and SMP mode.
Stress tests are still running over 1 hour.
----------------------------------------------------------------
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] masayuki2009 commented on a change in pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#discussion_r576639268
##########
File path: arch/xtensa/src/esp32/esp32_wlan.c
##########
@@ -493,6 +493,12 @@ static int wlan_rx_done(void *buffer, uint16_t len, void *eb)
{
nwarn("ERROR: Wlan receive %d larger than %d\n",
len, WLAN_BUF_SIZE);
+
+ if (eb)
+ {
+ esp_wifi_free_eb(eb);
+ }
+
Review comment:
>i guess we should do the same for the above if (!priv->ifup) block as well. how do you think?
I think we should add the logic too.
However, I think I should refactor the logic to make it simple.
----------------------------------------------------------------
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] masayuki2009 edited a comment on pull request #2858: arch: esp32: Fix a memory leak when discarding a large packet.
Posted by GitBox <gi...@apache.org>.
masayuki2009 edited a comment on pull request #2858:
URL: https://github.com/apache/incubator-nuttx/pull/2858#issuecomment-779688028
@yamt
I've just refactored the code and pushed with -f.
Also, updated the commit log and summary for 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org