osd: fix reference count, rare race condition etc. #8254
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xiexingguo
force-pushed
the
xxg-wip-osd
branch
3 times, most recently
from
7bcaedb
to
a526d99
Compare
| // we drop the pg_map_lock and give it back to caller. | ||
| // This is necessary because we may race with _remove_pg(), | ||
| // who will remove the last reference and destruct PG in most case. | ||
| out->get("get_pg_or_queue_for_pg"); |
There was a problem hiding this comment.
but they are both guarded by osd_lock, i believe?
|
@tchaikov Dropped those two changes. Thanks for the review. |
|
lgtm |
xiexingguo
force-pushed
the
xxg-wip-osd
branch
2 times, most recently
from
f37425d
to
03c5a93
Compare
The lock() method will ensure 'dirty_info' will be false, and within this function's scope I see no possibility to set 'dirty_info' true. Thus I guess the transaction cleanup logic before exit is never reachable and therefore shall be considered as redundant and can be safely removed. Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
On returning false, the require_mon_peer() will internally decrease the reference of the input message. So the put() method here is duplicated and will cause reference underflow and thus need to be dropped. Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
Theoretically even if _have_pg() returns ture, we still can't assert that _lookup_lock_pg() will always succeed. This is because when we switch between these two methods, we will drop pg_map_lock, and thus may let a pg removal sneak in, which may eventually cause divergence. However this is a really rare case, and is less likely to happen in a production environment. But this pr provided a safer way to achieve the same goal and is a little faster by eliminating a duplicated search from the pg_map, which makes it meaningful. Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
|
lgtm. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.