-
Notifications
You must be signed in to change notification settings - Fork 1.5k
mm: handle take mm sem in IRQ #5539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The issue still happens. |
|
@GUIDINGLI |
|
@masayuki2009 But nfsmount will meet failed with 13.Do you have any ideas ? |
The following line is my setting in /etc/exports |
|
It worked, thanks ! |
|
Could you add a assert(0) at mm_takesemaphore() to see if there is someone who called it in IRQ context ? |
@GUIDINGLI |
|
OK, add telnet. |
You need to specify the port number |
@GUIDINGLI However, it stopped at another place due to memory corruption. |
|
@masayuki2009 That is because lots of users use mm_takesemaphore() in idle thread, and don't care returns, like mm_malloc. So I obey the original design, don't check the IDLE in mm_takesemaphore(), let it call sem_wait |
|
@masayuki2009 |
@GUIDINGLI |
Please change the commit message correctly.
|
@GUIDINGLI |
|
Posting a question for a thread as is was resolved, but question was not answered: Should we get back return _SEM_TRYWAIT(&heap->mm_semaphore) >= 0; at least for the else if (sched_idletask())? |
|
Another question: or similar? |
b288a45 to
b8e6d45
Compare
OK, done |
Yes, we can do this, but that will be another PR. |
This is true only if |
again, this is wrong. only priority is not boosted |
|
I already pointed out the code that is affected by removal of |
|
@masayuki2009 Take cmd_md5 as a loop, take cmd_free as a loop. Then after about half an hour, the system stuck. |
Thanks for the report. |
We always use So, I commit another patch: |
there is |
If you keeping this un-handled, but how to implement up_idle(), in IDLE, user can call
Then you want the user who implement up_idle() to divide the situation ? |
I reexamined the changes again and see that you routed IDLE task case to |
|
@pkarashchenko @masayuki2009 @xiaoxiang781216 I want to give a limitation to idle thread, that user can't call wait/trywait in IDLE thread. |
Let's move the change to new PR? It isn't related to heap at all. |
Agree. This is a point for wider discussion. Common sense whispering to me we can even assert i case if IDLE thread tries to pen on sync primitive, but maybe I'm missing some points. so do not want to mix the things. |
|
OK, that the new PR: |
This is a fix of: 0169a51 This is caused by wrong memory sem operation in IDLE. Fix: Obey the original design, don't check the IDLE in mm_takesemaphore() Signed-off-by: ligd <liguiding1@xiaomi.com>
Summary
This is a amend of:
0169a51
caused by wrong merge operation
related PR:
#5504
Impact
mem sem_take in IRQ
Testing
VELA