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: print aio in batch #18873
Conversation
This PR looks more clean than #18803 . |
02049a5
to
f2d526e
Compare
src/os/bluestore/aio.cc
Outdated
unsigned i = 0; | ||
os << "aio: "; | ||
for (auto& iov : aio.iov) { | ||
os << "\n [" << i++ << "] " << iov.iov_base << " " << iov.iov_len; |
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.
Do you think if it makes sense to print iov_base/iov_len in hex?
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.
iov_base
is a pointer so it's printed in hex already. but iov_len
is a length, i am not sure if we should print it in hex. why shall we do so? because hex digits can be printed aligned and look better? but so do the dec digits..
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.
everything else in bluestore is in hex (offsets and lengths). it's way easier to see block alignment etc. we should do hex here imo
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.
f2d526e
to
a06360b
Compare
src/os/bluestore/aio.cc
Outdated
os << "aio: "; | ||
for (auto& iov : aio.iov) { | ||
os << "\n [" << i++ << "] " << std::hex | ||
<< iov.iov_base << " " << iov.iov_len; |
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.
please reset to decimal on completion to avoid potential side effects..
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.
@ifed01 fixed and repushed
KernelDevice::aio_{submit,write,read}() are critical paths. calling cct->_conf->subsys.should_gather() multi-times is not optimal. the downside of this issue is that if the aio is printed, the size of buffer in PrebufferedStreambuf could be large if the number of iov is large, that could be a heavy load to the memory subsystem. Signed-off-by: Kefu Chai <kchai@redhat.com>
a06360b
to
67c6f76
Compare
KernelDevice::aio_{submit,write,read}() are critical paths. calling
cct->_conf->subsys.should_gather() multi-times is not optimal. the
downside of this issue is that if the aio is printed, the size of
buffer in PrebufferedStreambuf could be large if the number of iov is
large, that could be a heavy load to the memory subsystem.
Signed-off-by: Kefu Chai kchai@redhat.com