Skip to content

qca-nss-ecm: resolve the cpu high load regarding ecm #3

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

Merged
merged 1 commit into from
Jun 27, 2021
Merged

qca-nss-ecm: resolve the cpu high load regarding ecm #3

merged 1 commit into from
Jun 27, 2021

Conversation

kirdesde
Copy link
Contributor

@kirdesde kirdesde commented Jun 26, 2021

this is based on quarky's patch for NSS 11.2 (just adapted to nss 11.4)

If using ECM, cpu load goes up (around 1.0) and stucks there. This is due to using uninterruptible sleep function, the patch changes this to interruptible sleep function.

-> https://github.com/quarkysg/openwrt/blob/master-ipq806x-nss-qsdk11/package/qca/qca-nss-ecm/patches/200-resolve-high-load.patch

Signed-off-by: Dirk Buchwalder buchwalder@posteo.de

@castiel652
Copy link
Contributor

Please change the commit title to something that looks like this: qca-nss-ecm: resolve the cpu high load and describe why this patch is needed in the commit message.
add your sign off after you finish above.

@kirdesde kirdesde changed the title resolve the cpu high load regarding ecm, based on quarky's patch for… qca-nss-ecm: resolve the cpu high load regarding ecm, based on quarky's patch for… Jun 26, 2021
@kirdesde kirdesde changed the title qca-nss-ecm: resolve the cpu high load regarding ecm, based on quarky's patch for… qca-nss-ecm: resolve the cpu high load regarding ecm Jun 26, 2021
@robimarko
Copy link
Owner

robimarko commented Jun 26, 2021

@kirdesde @castiel652 Pretty much told you what I would have told you.
You changed the PR but not the commit itself.

Please, update the patch numbering and name as well as commit message and SoB.

@kirdesde
Copy link
Contributor Author

I force pushed the commit changes as well as the changed patch numbering. I hope it's all right now.

@robimarko
Copy link
Owner

Ok, its moving in the right direction.
However, can you limit the commit message to 100 characters per line?

Also, the patch itself needs to be applicable with git apply and git am, which means that it has to be exported with git format patch.
Currently, it's lacking the commit name, message, SoB, and hashes that git uses to recreate the tree before applying.

@kirdesde
Copy link
Contributor Author

kirdesde commented Jun 27, 2021

I made a new commit, can you skip that first old commit and only pull that new commit?

Otherwise it's better to reject that PR and i would create a new one.

Sorry for all that noise.

@Ansuel
Copy link
Collaborator

Ansuel commented Jun 27, 2021

@kirdesde you are using the web or command git? if you want i can guide you on how to drop them...

@kirdesde
Copy link
Contributor Author

@kirdesde you are using the web or command git? if you want i can guide you on how to drop them...

I'm using the git command. Yes thanks a guide would be great.

@Ansuel
Copy link
Collaborator

Ansuel commented Jun 27, 2021

@kirdesde considering you have only 2 commits...
git rebase -i HEAD~2

follow instruction on the new view
in details you should replace the first line pick to drop
save
git rebase --continue

and force push
Now you should have only one commit :D

@kirdesde
Copy link
Contributor Author

Thanks Ansuel, now it's only one commit.
@robimarko : is it fine for you as well?

@robimarko
Copy link
Owner

Yes, it's supposed to be one commit.
But, in the patch itself, the whole message is in the subject which is incorrect.
Fix that and I will merge it.

@Ansuel
Copy link
Collaborator

Ansuel commented Jun 27, 2021

(git commit --amend)

@kirdesde
Copy link
Contributor Author

Yes, it's supposed to be one commit.
But, in the patch itself, the whole message is in the subject which is incorrect.
Fix that and I will merge it.

done

@robimarko
Copy link
Owner

Now you removed the commit message in the patch and replaced it with the subject copy, please leave the message in the patch.

If using ECM, cpu load goes up (around 1.0) and stucks there.
This is due to using uninterruptible sleep function,
the patch changes this to interruptible sleep function.

Signed-off-by: Dirk Buchwalder buchwalder@posteo.de
@kirdesde
Copy link
Contributor Author

Now you removed the commit message in the patch and replaced it with the subject copy, please leave the message in the patch.

done

@robimarko robimarko merged commit ce77000 into robimarko:main Jun 27, 2021
@robimarko
Copy link
Owner

Thanks

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.

4 participants