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/NVMEDevice: change write_bl to bl #17145

Merged
merged 1 commit into from Aug 31, 2017

Conversation

optimistyzy
Copy link
Contributor

With this patch, both read and write can use this field in
task.

Signed-off-by: Ziye Yang optimistyzy@gmail.com
Signed-off-by: Pan Liu wanjun.lp@alibaba-inc.com

@optimistyzy
Copy link
Contributor Author

@liupan1111

@optimistyzy
Copy link
Contributor Author

Jenkins, retest this please

@optimistyzy
Copy link
Contributor Author

@yuyuyu101 @liupan1111 This is a cleanup.

@@ -323,6 +323,11 @@ struct Task {
io_request.nseg = 0;
}

void copy_to_buf(bufferptr p, uint64_t off, uint64_t len) {
Copy link
Member

Choose a reason for hiding this comment

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

bufferptr &&p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why &&p, I think that bufferptr p is sufficient

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so... @tchaikov plz correct me if wrong

@@ -1090,11 +1095,10 @@ int NVMEDevice::aio_read(
Task *t = new Task(this, IOCommand::READ_COMMAND, off, len);

bufferptr p = buffer::create_page_aligned(len);
pbl->append(p);
pbl->append(t->bl);
Copy link
Member

Choose a reason for hiding this comment

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

std::move(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you explain why use std:move() sice at this time, there is no data copy yet. Also in kernelDevice.cc, it also directly use append.

char *buf = p.c_str();
t->fill_cb = [buf, t]() {
t->copy_to_buf(buf, 0, t->len);
pbl->append(t->bl);
Copy link
Member

Choose a reason for hiding this comment

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

std::move(

@liewegas liewegas changed the title NVMEDevice: change write_bl to bl os/bluestore/NVMEDevice: change write_bl to bl Aug 25, 2017
With this patch, both read and write can use this field in
task.

Signed-off-by: Ziye Yang <optimistyzy@gmail.com>
Signed-off-by: Pan Liu <wanjun.lp@alibaba-inc.com>
@optimistyzy
Copy link
Contributor Author

@yuyuyu101 @liupan1111 Could you review this patch again?

@yuyuyu101 yuyuyu101 merged commit 5923e77 into ceph:master Aug 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants