client: anchor dentries for trimming to make cap traversal safe#22073
Merged
batrick merged 2 commits intoceph:masterfrom May 19, 2018
Merged
client: anchor dentries for trimming to make cap traversal safe#22073batrick merged 2 commits intoceph:masterfrom
batrick merged 2 commits intoceph:masterfrom
Conversation
Member
Author
|
@ukernel thanks for the reproducer. I'm working on adding it to a QA test. I think this fix is simpler than what you had posted in [1]; I don't like the absurd complexity of deleting dentries during traversal, so let's not do it. |
ukernel
reviewed
May 18, 2018
src/client/Client.cc
Outdated
| } | ||
| } | ||
| ldout(cct, 20) << " clearing anchored inodes" << dendl; | ||
| ldout(cct, 20) << " clearing anchored dentries" << dendl; |
Contributor
There was a problem hiding this comment.
"trimming anchored dentries"
ukernel
reviewed
May 18, 2018
src/client/Client.cc
Outdated
| uint64_t trimmed = 0; | ||
| auto p = s->caps.begin(); | ||
| std::set<InodeRef> anchor; /* prevent put_inode from deleting all caps during traversal */ | ||
| std::set<Dentry *> anchor; /* prevent put_inode from deleting all caps during traversal */ |
Contributor
There was a problem hiding this comment.
maybe rename 'anchor' to 'to_trim'
ukernel
approved these changes
May 19, 2018
Contributor
|
I think it's better to not add the qa test. It's too tricky to arrange inodes in session->caps. we don't know if it still works in the future. |
Member
Author
|
It doesn't hurt to run the test so I'm inclined to keep it for now. It serves as a useful template for other tests. I'll add a note that it's unlikely to continue testing as expected. |
Test case by Yan Zheng [1]. [1] http://tracker.ceph.com/issues/24137#note-6 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
Fixes: http://tracker.ceph.com/issues/24137 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
batrick
added a commit
that referenced
this pull request
May 19, 2018
* refs/pull/22073/head: client: delay dentry trimming until after cap traversal qa: test for trim_caps segfault for trimmed dentries Reviewed-by: Zheng Yan <zyan@redhat.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes: http://tracker.ceph.com/issues/24137
Signed-off-by: Patrick Donnelly pdonnell@redhat.com