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

os/bluestore: separate finisher for deferred_try_submit #17409

Merged
merged 1 commit into from Sep 4, 2017

Conversation

liewegas
Copy link
Member

Reusing finishers[0], which is used for completions back into the OSD,
is deadlock-prone: the OSD code might block trying to submit new IO or
while waiting for some other bluestore work to complete.

Fixes: http://tracker.ceph.com/issues/21207
Signed-off-by: Sage Weil sage@redhat.com

Reusing finishers[0], which is used for completions back into the OSD,
is deadlock-prone: the OSD code might block trying to submit new IO or
while waiting for some other bluestore work to complete.

Fixes: http://tracker.ceph.com/issues/21207
Signed-off-by: Sage Weil <sage@redhat.com>
@varadakari
Copy link
Contributor

LGTM, but we are adding many threads into bluestore IO path, which adds more context switching overhead.

Copy link
Contributor

@varadakari varadakari left a comment

Choose a reason for hiding this comment

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

LGTM

@xiexingguo
Copy link
Member

we are adding many threads into bluestore IO path, which adds more context switching overhead.

there were a couple of PRs related to this issue, and I think this is fine as long as it turns out to be the right and hopefully final cure...

@varadakari
Copy link
Contributor

Yeah, i agree, but just wanted to be little cautious on adding more threads and context switches.

@liewegas
Copy link
Member Author

liewegas commented Sep 1, 2017

This is an exceptional case.. it only comes up during split or when the deferred io throttles are saturated, and is only there to make sure the pipeline doesn't stall. I don't think it will be a problem.

@xiexingguo xiexingguo merged commit c93c20f into ceph:master Sep 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants