Danny Al-Gaaf [Thu, 8 May 2014 14:09:07 +0000 (16:09 +0200)]
rbd.cc: init 'snap_protected' to fix -Wconditional-uninitialized
Init 'snap_protected' with false to fix:
rbd.cc:544:35: warning: variable 'snap_protected' may be uninitialized
when used here [-Wconditional-uninitialized]
f->dump_string("protected", snap_protected ? "true" : "false");
^~~~~~~~~~~~~~
rbd.cc:482:22: note: initialize the variable 'snap_protected' to silence
this warning
bool snap_protected;
^
= false
Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Danny Al-Gaaf [Thu, 8 May 2014 14:04:18 +0000 (16:04 +0200)]
rbd-fuse.c: init 'rbd' in open_rbd_image()
Init 'rbd' in open_rbd_image() with NULL and add a check for
'rbd' before dereference it to fix:
rbd_fuse/rbd-fuse.c:182:29: warning: variable 'rbd' may be uninitialized
when used here [-Wconditional-uninitialized]
int ret = rbd_open(ioctx, rbd->image_name, &(rbd->image), NULL);
^~~
rbd_fuse/rbd-fuse.c:151:27: note: initialize the variable 'rbd' to silence
this warning
struct rbd_openimage *rbd;
^
= NULL
Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Danny Al-Gaaf [Thu, 8 May 2014 13:54:24 +0000 (15:54 +0200)]
ObjectCacher::_wait_for_write(): init 'bool done'
Init 'bool done' with 'false' to fix:
osdc/Objecter.h:915:27: warning: implicit conversion los: variable 'done'
may be uninitialized when used here [-Wconditional-uninitialized]
while (!done)
^~~~
osdc/ObjectCacher.cc:1399:14: note: initialize the variable 'done' to
silence this warning
bool done;
^
= false
Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Danny Al-Gaaf [Thu, 8 May 2014 13:47:08 +0000 (15:47 +0200)]
Objecter::calc_target(): init best_locality with 0
Init best_locality to fix:
osdc/Objecter.cc:1519:26: warning: variable 'best_locality' may be
uninitialized when used here [-Wconditional-uninitialized]
(locality >= 0 && best_locality >= 0 &&
^~~~~~~~~~~~~
osdc/Objecter.cc:1511:19: note: initialize the variable 'best_locality'
to silence this warning
int best_locality;
^
= 0
Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Danny Al-Gaaf [Wed, 7 May 2014 15:28:31 +0000 (17:28 +0200)]
test_rgw_manifest.cc: fix VLA of non-POD element type
Use vector to fix:
test/rgw/test_rgw_manifest.cc:184:20: error: variable length array
of non-POD element type 'RGWObjManifest'
RGWObjManifest pm[num_parts];
Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Gregory Farnum [Wed, 7 May 2014 06:06:14 +0000 (23:06 -0700)]
Merge pull request #1532 from ceph/wip-fast-dispatch
fast dispatch
This series adds an ms_fast_dispatch interface to the Messenger/Dispatcher, designed so that you can dispatch messages directly from the Pipe threads without going through the Dispatch queue.
It also sets the OSD to make use of this interface for most operations, and switches to finer-grained locking and use of local data in a bunch of different paths to enable that.
Greg Farnum [Mon, 5 May 2014 04:58:04 +0000 (21:58 -0700)]
Pipe: wait for Pipes to finish running, instead of just stop()ing them
Add a stop_and_wait() function that, in addition to closing the Pipe and killing
its socket, waits for any fast_dispatch call which is in-progress. Use this in
several parts of the Pipe and SimpleMessenger code where appropriate.
This fixes several races with fast_dispatch and other avenues; here are two:
1) It could be that we grab the lock while the existing pipe is fast_dispatching
and then proceed to dispatch messages ourself, beating it. Instead, wait for
the other pipe. Add a "reader_dispatching" member which tells bus this is
happening, and when re-locking, signal the cond if we're shutting down.
2) It could be that a normally-dispatched Message in the OSD triggers a
mark_down() on the Connection and then clears out the Session
(Connection::priv) pointer, causing a racing fast_dispatch()'ed function to
assert out in the OSD because it requires a valid Session.
Signed-off-by: Greg Farnum <greg@inktank.com> Reviewed-by: Sage Weil <sage@inktank.com>
Sage Weil [Tue, 6 May 2014 18:01:27 +0000 (11:01 -0700)]
osd/ReplicatedPG: fix whiteouts for other cache mode
We were special casing WRITEBACK mode for handling whiteouts; this needs to
also include the FORWARD and READONLY modes. To avoid having to list
specific cache modes, though, just check != NONE.
Fixes: #8296
Backport: firefly Signed-off-by: Sage Weil <sage@inktank.com>
SimpleMessenger: Don't grab the lock when sending messages if we don't have to
We'd like it if sending a message didn't require any global locks, but the
submit_message() function conditionally needs it in order to create new
Pipes. So:
1) When failing on a dud Pipe, verify that it's still the Pipe the Connection
is linked to; if not, try sending along the newly-linked Pipe.
2) Add an "already_locked" param to submit_message
3) Have the Connection-based interface set this param to false, and
the addr-based interface set it to true, reflecting whether they have
taken the SimpleMessenger::lock.
4) If we discover we need to reference global data structures in
submit_mesage:
4a) if locked, do as we previously have
4b) if not locked, take the lock and call into submit_message again.
The net effect of this is that in the typical case, the Connection-based
_send_message() function no longer acquires global locks, only per-Connection
ones. In the case where the Connection must recreate a Pipe, it falls back to
performing like the addr-based _send_message() does. In the case where
we are racing with somebody else recreating a Pipe(either us or the other
end), we may try twice but we will still only take per-Connection/Pipe locks,
which is a fair tradeoff for not taking the global lock.
Signed-off-by: Greg Farnum <greg@inktank.com> Reviewed-by: Sage Weil <sage@inktank.com>
OSD: scan for dropped PGs in consume_map instead of advance_map
We have to wait until after we know that nobody will be adding ops for
newly-dead PGs to the list. While we're moving it, switch the locking
so we only hold a write lock while deleting the actual lists.
OSD: do not take the pre_publish_lock in connection utility functions
They loop back around for local connections and deadlock, so we use the
map reservation mechanism instead.
TODO: actually that issue is out of date, do we still want this change?
Greg Farnum [Wed, 26 Mar 2014 22:04:39 +0000 (15:04 -0700)]
OSD: enable ms_fast_dispatch
We've been setting it up, now this patch actually adds a fast path for osd ops
which bypasses the osd_lock and should not block on any longly held locks. In
addition to the actual ms_fast_dispatch; we take advantage of the fast_notify
functions in order to create a Session for every peer, since that is now the
data structure around which we handle incoming Messages and waitlisting; and
fast_preprocess in order to track when a peer has already sent us a new map
(otherwise, if we see an op with a too-new epoch, we have to request it from
the monitor).
Signed-off-by: Samuel Just <sam.just@inktank.com> Signed-off-by: Greg Farnum <greg@inktank.com>
Don't hold the old PG's lock in _create_lock_pg. Instead, just copy the
necessary data bits into a holding location. Note that this means we aren't
protecting it against change while the new PG is created, which I *think*
is okay...
Greg Farnum [Tue, 25 Mar 2014 22:32:30 +0000 (15:32 -0700)]
OSD: allow build_incremental_map_msg to fail on lookups
Since we're now building incremental map messages out-of-band with doing
other map updates now, we need to tolerate lookup failures at the bottom
end. Do so by returning a NULL message in that case.
Handle that in send_incremental_map by looping until we get a
message back -- if we fail on the first attempt, we'll get
the OSDSuperblock again and deal with it.
Greg Farnum [Fri, 21 Mar 2014 18:07:36 +0000 (11:07 -0700)]
OSD: share map updates in the op_tp threads instead of the main dispatch thread
Sharing maps can require disk accesses and things. We don't want to do that
in our fast path, so do it in OSD::dequeue_op instead of OSD::handle_op. We're
cheating slightly and still doing it in handle_op if no op actually gets queued,
but we're going to put those into a separate work queue next. We'll also be
moving all the functions necessary for this into OSDService so that our completion
struct doesn't need to be a friend to OSD.
To make this easier, we're adding send_map_update and sent_epoch members to
OpRequest.
Greg Farnum [Wed, 26 Mar 2014 20:14:12 +0000 (13:14 -0700)]
OSD: refactor handle_op error handling cases
We move our map version-checking code earlier (to dispatch_op) and refactor
our other fail-to-dispatch cases. This is friendlier for the no-lock
message processing we'll use with fast dispatch.
Signed-off-by: Samuel Just <sam.just@inktank.com> Signed-off-by: Greg Farnum <greg@inktank.com>
Greg Farnum [Thu, 20 Mar 2014 23:12:05 +0000 (16:12 -0700)]
OSD: change Session handling around _share_map_incoming
Move responsibility for the reference up to _share_map_incoming's caller,
and start using the Session::sent_epoch_lock. This looks a little silly
now, but we're going to split up the decide-to-send-maps and send-maps steps
and don't want to block in the decide-to-send step, so we need some
pretty flexible locking up at this level.
Greg Farnum [Thu, 20 Mar 2014 17:05:14 +0000 (10:05 -0700)]
OSD: use safe params in map-sharing functions
We were previously using unprotected access to OSD members.
Unfortunately, this does not make them completely safe: we are looking up
maps asynchronously from when we got access to the cached map bounds, and
so the OSD could delete a map out from underneath us. Fixing that will
require some kind of map bounds lock. :/
Samuel Just [Fri, 8 Nov 2013 20:41:08 +0000 (12:41 -0800)]
OSD::_share_map_incoming: pass osdmap in explicitly
We'll want to be able to use this method without the osd_lock. Note
that we can't do so yet -- we call send_incremental_map, which is not
safe to call unlocked.
Signed-off-by: Samuel Just <sam.just@inktank.com> Reviewed-by: Greg Farnum <greg@inktank.com>
This member was previously protected by the osd_lock (although setting
SHUTDOWN was synchronized with the heartbeat lock, too), but we need
to read it for fast dispatch, so protect it under its own lock at all times.
OSD: Push responsibility for grabbing pg_map_lock up to callers of _remove_pg()
The atomicity requirements of other systems prevent us dropping the PG lock
inside that function, and the PG lock is ordered underneath the pg_map_lock.
Samuel Just [Fri, 8 Nov 2013 23:20:49 +0000 (15:20 -0800)]
OSD: wake_pg_waiters atomically with pg_map update
Also, call enqueue_op directly rather than going back
through the entire dispatch machinery.
Be sure to grab the pg lock under the pg_map_lock in _open_lock_pg() to
preserve necessary lock ordering.
Signed-off-by: Samuel Just <sam.just@inktank.com> Signed-off-by: Greg Farnum <greg@inktank.com>
Samuel Just [Fri, 8 Nov 2013 17:35:54 +0000 (09:35 -0800)]
OSD: pass osdmap to handle_op and handle_replica_op
We need a map to process them, and we don't want to
take the OSD lock to access one. (And we can't just
use the service because we need all processing of
a message to be done with the same map.)
Signed-off-by: Samuel Just <sam.just@inktank.com> Signed-off-by: Greg Farnum <greg@inktank.com>
Greg Farnum [Fri, 14 Mar 2014 22:46:46 +0000 (15:46 -0700)]
OSD: add a RWLock pg_map_lock
If we're going to dispatch ops without grabbing the osd lock, we need
something else to protect the pg map (and it'll be a little
contended, so use a read-write lock).
We repurpose the (previously oddly-named) _lookup_lock_pg_with_map_lock_held()
function to refer to the pg_map_lock. handle_pg_query and handle_pg_remove
switch to use that version, because they're holding pg_map_lock already and
we know the PG they're referring to exists.
Signed-off-by: Samuel Just <sam.just@inktank.com> Signed-off-by: Greg Farnum <greg@inktank.com>
Samuel Just [Thu, 7 Nov 2013 20:48:27 +0000 (12:48 -0800)]
OSDService: add osdmap reservation mechanism
The goal here is to be able to get "reserved" refs
to next_map, and ensure that pgs won't see a newer
map until the ref is "released". I haven't done
a cute RAII trick here yet...probably not worth
the effort.
Signed-off-by: Samuel Just <sam.just@inktank.com> Reviewed-by: Greg Farnum <greg@inktank.com>
Conflicts:
src/osd/OSD.h Signed-off-by: Greg Farnum <greg@inktank.com>
Since we're doing fast_dispatch out of the delay queue, we don't want to
flush while holding the pipe lock. Instead, make flush set it up for instant
delivery, and steal the delay queue when replacing pipes. If we're shutting
down a pipe, wait until flushing has completed before doing so.
We do two things:
1) Call ms_handle_fast_connect() when setting up the local connection, so
the Dispatcher can set up any state it needs
2)Move local_delivery into a separate thread from the sender's. fast_dispatch
makes this entirely necessary since otherwise we're dipping back in to the
Dispatcher while holding whatever locks it held when it sent the Message.
Implementation starts with a thread and a list of messages to process and
proceeds as you'd expect from that.
This adds a Dispatcher interface allowing the implementation
to accept ms_fast_dispatch calls for some messages without
going through the DispatchQueue. To support that, we also add
1) new synchronous notifications on connect and accept events
2) a fast_preprocess mechanism
Signed-off-by: Samuel Just <sam.just@inktank.com> Signed-off-by: Greg Farnum <greg@inktank.com>
Yan, Zheng [Sat, 3 May 2014 21:17:15 +0000 (05:17 +0800)]
osd: check blacklisted clients in ReplicatedPG::do_op()
OSD checks if client is blacklisted only when receiving OSD request.
It's possible that OSD request's sender get blacklisted while OSD
request in in some waiting list.