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: enlarege aligned_size avoid too many vector(> IOV_MAX) #18828
Conversation
src/os/bluestore/KernelDevice.cc
Outdated
uint64_t aligned_size = block_size; | ||
// Avoid after rebuild_aligned_size_and_memory, get_num_buffers() > IOV_MAX | ||
if (bl.get_num_buffers() >= IOV_MAX && (len > (IOV_MAX * block_size))) { | ||
aligned_size = ROUND_UP_TO(len / IOV_MAX, block_size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if len is not the multiple of (IOV_MAX * block_size)
? i think it should be
ROUND_UP_TO(ROUND_UP_TO(len, IOV_MAX) / IOV_MAX, block_size)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
src/os/bluestore/KernelDevice.cc
Outdated
aligned_size = ROUND_UP_TO(len / IOV_MAX, block_size); | ||
} | ||
if (bl.rebuild_aligned_size_and_memory(aligned_size, block_size)) { | ||
dout(20) << __func__ << " rebuilding buffer to be aligned" << dendl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you print out the aligned_size
here also? i think it could help with the performance related diagnosis.
update. |
src/os/bluestore/KernelDevice.cc
Outdated
if ((!buffered || bl.get_num_buffers() >= IOV_MAX)) { | ||
uint64_t aligned_size = block_size; | ||
// Avoid after rebuild_aligned_size_and_memory, get_num_buffers() > IOV_MAX | ||
if (bl.get_num_buffers() >= IOV_MAX && (len > (IOV_MAX * block_size))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be more consistent, could you please remove the parenthesis around len > (IOV_MAX * block_size)
?
src/os/bluestore/KernelDevice.cc
Outdated
// Avoid after rebuild_aligned_size_and_memory, get_num_buffers() > IOV_MAX | ||
if (bl.get_num_buffers() >= IOV_MAX && (len > (IOV_MAX * block_size))) { | ||
aligned_size = ROUND_UP_TO(ROUND_UP_TO(len, IOV_MAX) / IOV_MAX, block_size); | ||
dout(5) << __func__ << " because too many buffers, using " << aligned_size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am not quite sure about dout(5)
. is a large bl.get_num_buffers()
an unusual thing and we should watch out? actually what i suggested in last comment is to print the aligned_size
at the "rebuilding buffer to be aligned" line with dout(20)
.
update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! It would be nice to move this into a helper at some point. rebuild_aligned_size_and_memory could even take IOV_MAX as an arg.
8847754
to
fba63ce
Compare
Fixes: http://tracker.ceph.com/issues/21932 Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Fixes: http://tracker.ceph.com/issues/21932
Signed-off-by: Jianpeng Ma jianpeng.ma@intel.com