Skip to content

add holder for mutex#8164

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
flyingfish89:addholder_mutex
Jan 18, 2023
Merged

add holder for mutex#8164
xiaoxiang781216 merged 1 commit into
apache:masterfrom
flyingfish89:addholder_mutex

Conversation

@flyingfish89

@flyingfish89 flyingfish89 commented Jan 17, 2023 •
edited by xiaoxiang781216
Loading

Copy link
Copy Markdown
Contributor

Summary

add holder for mutex, continue the work #7426

Impact

remove holder from rmutex and add holder for mutex

Testing

@flyingfish89 flyingfish89 force-pushed the addholder_mutex branch 4 times, most recently from dd221ae to 2abaf38 Compare January 17, 2023 11:48
@flyingfish89 flyingfish89 force-pushed the addholder_mutex branch 6 times, most recently from e90988f to 2c77e60 Compare January 17, 2023 17:34
@flyingfish89 flyingfish89 force-pushed the addholder_mutex branch 5 times, most recently from 491a556 to f25e732 Compare January 18, 2023 04:19
add holder for mutex
579795d 
Signed-off-by: lilei19 <lilei19@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit fc176ad into apache:master Jan 18, 2023
@masayuki2009

masayuki2009 commented Jan 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@flyingfish89

I noticed that sim:usrsocktest causes assert with this PR.

nsh> uname -a
NuttX  12.0.0 a4c9da9280 Jan 18 2023 20:32:01 sim sim
nsh> usrsocktest
Starting unit-tests...
Testing group "char_dev" =>
	Group "char_dev": [OK]
Testing group "no_daemon" =>
	Group "no_daemon": [OK]
Testing group "basic_daemon" =>
	Group "basic_daemon": [OK]
Testing group "basic_connect" =>
	Group "basic_connect": [OK]
Testing group "basic_connect_delay" =>
	Group "basic_connect_delay": [OK]
Testing group "no_block_connect" =>
	Group "no_block_connect": [OK]
Testing group "basic_send" =>
	Group "basic_send": [OK]
Testing group "no_block_send" =>
	Group "no_block_send": [OK]
Testing group "block_send" =>
	Group "block_send": [OK]
Testing group "no_block_recv" =>
	Group "no_block_recv": [OK]
Testing group "block_recv" =>
	Group "block_recv": [OK]
Testing group "remote_disconnect" =>
	Group "remote_disconnect": [OK]
Testing group "basic_setsockopt" =>
	Group "basic_setsockopt": [OK]
Testing group "basic_getsockopt" =>
	Group "basic_getsockopt": [OK]
Testing group "basic_getsockname" =>
	Group "basic_getsockname": [OK]
Testing group "wake_with_signal" =>
_assert: Current Version: NuttX  12.0.0 a4c9da9280 Jan 18 2023 20:32:01 sim
_assert: Assertion failed at file: usrsock/usrsock_devif.c:709 task: <pthread> 0x5619e6fec5ad

pkarashchenko
pkarashchenko reviewed Jan 18, 2023
View reviewed changes
Comment thread include/semaphore.h
#define SEM_PRIO_INHERIT 1
#define SEM_PRIO_PROTECT 2
#define SEM_PRIO_MASK 3
#define SEM_PRIO_MUTEX 4

@pkarashchenko pkarashchenko Jan 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define SEM_PRIO_MUTEX 4
#define SEM_TYPE_MUTEX 4

@xiaoxiang781216 xiaoxiang781216 Jan 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkarashchenko here is the patch: d45ac2b

@masayuki2009

masayuki2009 commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

@flyingfish89

I noticed that sim:usrsocktest causes assert with this PR.

It seems that this PR causes deadlock with ostest (e.g. rp2040:smp with CONFIG_DEBUG_ASSERTIONS=y).

@xiaoxiang781216

xiaoxiang781216 commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

@flyingfish89

I noticed that sim:usrsocktest causes assert with this PR.

nsh> uname -a
NuttX  12.0.0 a4c9da9280 Jan 18 2023 20:32:01 sim sim
nsh> usrsocktest
Starting unit-tests...
Testing group "char_dev" =>
	Group "char_dev": [OK]
Testing group "no_daemon" =>
	Group "no_daemon": [OK]
Testing group "basic_daemon" =>
	Group "basic_daemon": [OK]
Testing group "basic_connect" =>
	Group "basic_connect": [OK]
Testing group "basic_connect_delay" =>
	Group "basic_connect_delay": [OK]
Testing group "no_block_connect" =>
	Group "no_block_connect": [OK]
Testing group "basic_send" =>
	Group "basic_send": [OK]
Testing group "no_block_send" =>
	Group "no_block_send": [OK]
Testing group "block_send" =>
	Group "block_send": [OK]
Testing group "no_block_recv" =>
	Group "no_block_recv": [OK]
Testing group "block_recv" =>
	Group "block_recv": [OK]
Testing group "remote_disconnect" =>
	Group "remote_disconnect": [OK]
Testing group "basic_setsockopt" =>
	Group "basic_setsockopt": [OK]
Testing group "basic_getsockopt" =>
	Group "basic_getsockopt": [OK]
Testing group "basic_getsockname" =>
	Group "basic_getsockname": [OK]
Testing group "wake_with_signal" =>
_assert: Current Version: NuttX  12.0.0 a4c9da9280 Jan 18 2023 20:32:01 sim
_assert: Assertion failed at file: usrsock/usrsock_devif.c:709 task: <pthread> 0x5619e6fec5ad

I just tried sim:usrsocktest with the last master(c19b5ee), all test can pass.

@masayuki2009

masayuki2009 commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

@xiaoxiang781216

I just tried sim:usrsocktest with the last master(c19b5ee), all test can pass.

Please try the test with CONFIG_DEBUG_ASSERTIONS=y

@masayuki2009

masayuki2009 commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

@flyingfish89
spresense:rndis_smp is unstable now (i.e. ping does not work)

@masayuki2009

masayuki2009 commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

Shall we revert this PR because there are some issues?

@jerpelea

jerpelea commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

+1 for revert

@jerpelea jerpelea mentioned this pull request Jan 18, 2023
Merged
@xiaoxiang781216 xiaoxiang781216 mentioned this pull request Jan 18, 2023
Merged
@xiaoxiang781216

xiaoxiang781216 commented Jan 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@xiaoxiang781216

I just tried sim:usrsocktest with the last master(c19b5ee), all test can pass.

Please try the test with CONFIG_DEBUG_ASSERTIONS=y

Ok, I can repro the problem now. @masayuki2009 could you try this patch: #8164. The error is due to the code forget to convert the error number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pkarashchenko pkarashchenko pkarashchenko left review comments
@xiaoxiang781216 xiaoxiang781216 xiaoxiang781216 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@flyingfish89 @masayuki2009 @xiaoxiang781216 @jerpelea @pkarashchenko