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
librbd: minor cleanup of the IO pathway #20560
Conversation
84f1446
to
dee3e6d
Compare
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.
Otherwise LGTM
src/librbd/io/ImageRequest.cc
Outdated
journal_tid); | ||
ctx = new C_FlushJournalCommit<I>(image_ctx, aio_comp, journal_tid); | ||
ctx = new FunctionContext( | ||
[aio_comp, &image_ctx, journal_tid, ctx] (int r) { |
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.
nit: aio_comp
is not used here any more
src/librbd/io/ReadResult.cc
Outdated
CephContext *cct = aio_completion->ictx->cct; | ||
ldout(cct, 10) << "C_SparseReadRequestBase: r = " << r | ||
ldout(cct, 10) << "C_ObjectReadRequest: r = " << r |
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.
nit: while here could you please remove white spaces: ": r=", as we have in other log messages (it is easier to grep for an error when it is uniform).
src/librbd/io/ReadResult.cc
Outdated
AioCompletion *aio_completion, uint64_t object_off, uint64_t object_len, | ||
Extents&& buffer_extents, bool ignore_enoent) | ||
: aio_completion(aio_completion), object_off(object_off), | ||
object_len(object_len),buffer_extents(std::move(buffer_extents)), |
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.
nit: needs a whitespace after the comma
src/librbd/LibrbdWriteback.cc
Outdated
uint64_t _journal_tid, | ||
const ZTracer::Trace &trace, Context *_req_comp) | ||
: image_ctx(_image_ctx), oid(_oid), object_no(_object_no), off(_off), | ||
bl(_bl), snapc(_snapc), journal_tid(_journal_tid), | ||
trace(trace), req_comp(_req_comp) { | ||
length(_bl.length()), bl(std::move(_bl)), snapc(_snapc), |
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.
nit: double whitespace after the comma
This breaks the tight coupling between the IO work queue and the actual dispatch of IO requests. Signed-off-by: Jason Dillaman <dillaman@redhat.com>
This will be used in a later cleanup to remove the IO flush handling from ImageCtx. Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
dee3e6d
to
18bce0a
Compare
tweaks pushed |
This is the first part in a series that aims to remove the tied coupling of the object cacher from the IO pathway (to permit a plugable IO dispatcher).