Skip to content
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

Check MDIO server thread joinable before join the thread #1342

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

jiahua-wang
Copy link
Contributor

Signed-off-by: Jiahua Wang <jiahua.wang@broadcom.com>
@kcudnik
Copy link
Collaborator

kcudnik commented Jan 25, 2024

Is the root case already known? Was it double join?

@jiahua-wang
Copy link
Contributor Author

Double join could be an explanation. More likely scenario is that m_taskThread is not joinable because after MdioIpcServer constructor startMdioThread() was not called or because startMdioThread() failed. @kenneth-arista can provide more test details.

@kcudnik
Copy link
Collaborator

kcudnik commented Jan 26, 2024

Double join could be an explanation. More likely scenario is that m_taskThread is not joinable because after MdioIpcServer constructor startMdioThread() was not called or because startMdioThread() failed. @kenneth-arista can provide more test details.

Can you investigate which exactly scenario is that ?

@kenneth-arista
Copy link

Double join could be an explanation. More likely scenario is that m_taskThread is not joinable because after MdioIpcServer constructor startMdioThread() was not called or because startMdioThread() failed. @kenneth-arista can provide more test details.

Can you investigate which exactly scenario is that ?

Unfortunately, I don't have all the logs when the problem occurred before integrating Jiahua's patch. Prior to the patch, I would often seen several syncd core files after each full sonic-mgmt test runs. I suspect these crashes are triggered in tests involving reboot.

@kenneth-arista
Copy link

I recall that a syncd crash was observed once during sonic-mgmt override_config_table tests.

@jiahua-wang
Copy link
Contributor Author

This patch will address both cases of double join and thread not starting properly.

@rlhui
Copy link

rlhui commented Feb 21, 2024

@jiahua-wang - please update the branch? thanks

@xumia
Copy link
Collaborator

xumia commented Feb 22, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jiahua-wang
Copy link
Contributor Author

/azp run

Copy link

Commenter does not have sufficient privileges for PR 1342 in repo sonic-net/sonic-sairedis

@jiahua-wang
Copy link
Contributor Author

/AzurePipelines run Azure.sonic-sairedis

Copy link

Commenter does not have sufficient privileges for PR 1342 in repo sonic-net/sonic-sairedis

@judyjoseph
Copy link
Contributor

@kcudnik Can we merge this PR

@judyjoseph
Copy link
Contributor

/AzurePipelines run Azure.sonic-sairedis

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jiahua-wang
Copy link
Contributor Author

@kcudnik @rlhui @judyjoseph All checks were passed when I created the PR for the first time. After I merged other commits, 2 checks always fail at the place not related to my code change. Should I close this one and create another one with the same code change?

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 7, 2024

All passed do you want me to merge?

@jiahua-wang
Copy link
Contributor Author

@kcudnik Please merge.

@kcudnik kcudnik merged commit bb948f6 into sonic-net:master Mar 7, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants