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
Distribute worker threads to all cores #197
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: hongzhou zhang <zhang.hongzhou@hotmail.com>
@zhanghongzhou thanks a lot for your PR. Could you provide benchmarking result? Of course it doesn't have to be good for now. |
@johnzhang1985 please comment in English :) |
I can test this patch and compare to old version. |
@zhanghongzhou @johnzhang1985 @vtolstov I created an additional patch for creating per cpu work queue. You can find it here: https://github.com/sheepdog/sheepdog/tree/per-cpu-work-queue It is not tested well. I'm glad if you can test and share benchmark result. |
We plan to test sheepdog/per-cpu-work-queue next quarter. The branch is based on a-little-old master (04fe301 committed on 13 Nov 2015), so first I try to rebase onto the latest master (8772904 22 days ago). However, I encountered rebase conflict. Some trial-and-error have done and I found that the branch conflicted with the commit 698e5d2 merging PR #217. So I made my own two branches:
I would be very glad if you review my branches, especially the last some commits in the second one. I think it is a little better than the first one. |
@tmenjo I think the second branch would be ok. Could you squash the commits that resolve conflict? Then they can be merged. |
@mitake I force-updated per-cpu-work-queue-2. Past four commits are squashed into three commits; zhanghongzhou's one, mitake's one (which my one resolving conflict is meld into) and my another one resolving compile error. Could you review again? |
I tried https://github.com/tmenjo/sheepdog/tree/per-cpu-work-queue-2 branch. I started sheepdog and executed taskset command.
A result of cause analysis,
'wi->q.wq_state' is always 0. I think 'wi->q.wq_state' should change to 'wi->tc'.
I started sheepdog by code as above, and executed taskset command.
|
@sa-kura thanks a lot for your analysis. Is it possible to evaluate performance after your change? |
This fixes an issue reported by sa-kura. See: sheepdog#197 (comment) Signed-off-by: Takashi Menjo <menjo.takashi@lab.ntt.co.jp>
@vtolstov I'm not sure yet. But if you can benchmark https://github.com/sheepdog/sheepdog/tree/per-cpu-work-queue with rebasing on the master, it will be helpful. |
Signed-off-by: hongzhou zhang zhang.hongzhou@hotmail.com