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

Fix for cm_svc_run #1484

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Fix for cm_svc_run #1484

merged 1 commit into from
Jul 25, 2024

Conversation

linxiaochou
Copy link
Contributor

When the cm_svc_run thread starts, the four element size rss will be allocated to svc->contexts. When the rss reallocates memory due to an increase in the number of elements in the cm_svc_process_sock function, the rss will point to a new address, but the cm_svc_run function does not get the new address again, leading to an exception in the future

cm_svc_process_sock(svc);
/* svc->contexts will be changed, so need to assign again */
fds = svc->contexts;
Copy link
Member

Choose a reason for hiding this comment

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

Please write an explanation to the change in commit message, together with exception.

We are following kernel coding style, so Fixes line should be added too.
Fixes: b60c79d ("rsockets: Use service thread to accept connections")

@shefty

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would say 'svc->contexts may have been reallocated' rather than 'will be changed' Fix looks good.

@linxiaochou
Copy link
Contributor Author

linxiaochou commented Jul 24, 2024

@rleon I'm glad you responded and pointed out my problem. I've updated the commit msg. Hope it works this time.

@rleon
Copy link
Member

rleon commented Jul 24, 2024

@rleon I'm glad you responded and pointed out my problem. I've updated the commit msg. Hope it works this time.

Sure, let's wait for @shefty approval.

When the cm_svc_run thread starts, svc->contexts will be initialized with four elements. When reallocating memory due to an increased element count, svc->contexts will point to new addresses, whereas the fds variable in the cm_svc_run thread is free of the address, which will result in subsequent cm_svc_run threads running incorrectly. So we need to update the fds variable every time the cm_svc_process_sock function is processed.

Fixes: b60c79d ("rsockets: Use service thread to accept connections")
Signed-off-by: linxiaochou <929331108@qq.com>
@linxiaochou
Copy link
Contributor Author

linxiaochou commented Jul 25, 2024

@shefty Thank you for your reply and suggestions. I have already revised the corresponding notes. Hope it can work this time.

@rleon rleon merged commit c6fef6f into linux-rdma:master Jul 25, 2024
14 checks passed
@linxiaochou linxiaochou deleted the fix-cm_svc_run branch July 25, 2024 08:51
@linxiaochou linxiaochou restored the fix-cm_svc_run branch July 25, 2024 08:52
@linxiaochou linxiaochou deleted the fix-cm_svc_run branch July 25, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants