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

bufferlist: fix the bug of memory consolidation #12237

Closed
wants to merge 1 commit into from

Conversation

YankunLi
Copy link
Contributor

Signed-off-by: Yankun Li lioveni99@gmail.com

Signed-off-by: Yankun Li <lioveni99@gmail.com>
@YankunLi YankunLi changed the title bufferlist: fig the bug of memory consolidation bufferlist: fix the bug of memory consolidation Nov 30, 2016
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

i don't think that your fix is correct. if not, could you add a test to verify your fix?

@@ -1826,7 +1826,7 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
if (!_buffers.empty()) {
ptr &l = _buffers.back();
if (l.get_raw() == bp.get_raw() &&
l.end() == bp.start() + off) {
l.end() == off) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the off here is the offset inside of bp which start from _off in the backed raw data, while l.end() is the offset from the raw data. i don't think they are comparable.

Copy link
Contributor Author

@YankunLi YankunLi Nov 30, 2016

Choose a reason for hiding this comment

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

@tchaikov Thinks, but, I don't think so, the off here equal to append_buffer.end() - gap(the length of data that is append), so it is also the offset from the raw data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tchaikov If you are right, this call( #12247 ) must be error。

@tchaikov
Copy link
Contributor

tchaikov commented Dec 1, 2016

closing this PR, as #12247 is the right fix.

@tchaikov tchaikov closed this Dec 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants