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

rgw: collect skips a specific coroutine stack #10274

Merged
merged 1 commit into from Jul 14, 2016
Merged

Conversation

yehudasa
Copy link
Member

Fixes: http://tracker.ceph.com/issues/16665

Instead of drain_all_but() that specifies number of stacks to leave behind,
added drain_all_but_stack() that has a specific stack specified. This is needed
so that we don't call wakeup() through lease_cr->go_down() on a cr stack that
was already collected.

Signed-off-by: Yehuda Sadeh yehuda@redhat.com

@cbodley
Copy link
Contributor

cbodley commented Jul 13, 2016

looks good. only suggestion would be to make the skip_stack argument optional (default to nullpr)

@yehudasa
Copy link
Member Author

@cbodley yeah, I wanted to make sure that I visited all call sites when going through the change

Fixes: http://tracker.ceph.com/issues/16665

Instead of drain_all_but() that specifies number of stacks to leave behind,
added drain_all_but_stack() that has a specific stack specified. This is needed
so that we don't call wakeup() through lease_cr->go_down() on a cr stack that
was already collected.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa yehudasa changed the title [DNM] rgw: collect skips a specific coroutine stack rgw: collect skips a specific coroutine stack Jul 13, 2016
@cbodley
Copy link
Contributor

cbodley commented Jul 14, 2016

I think we should be holding a reference to these lease_stacks. Even though we only use it to check for pointer equality in collect() and never dereference, it's possible for it to be freed and reallocated by another spawn(). This would cause us to skip the wrong stack

@cbodley cbodley merged commit 49001d9 into ceph:master Jul 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants