From cd5f2bfdbb7fbf9237bef482845644cc41fa66de Mon Sep 17 00:00:00 2001 From: Josh Durgin Date: Thu, 15 Nov 2012 16:20:33 -0800 Subject: ObjectCacher: fix off-by-one error in split This error left a completion that should have been attached to the right BufferHead on the left BufferHead, which would result in the completion never being called unless the buffers were merged before it's original read completed. This would cause a hang in any higher level waiting for a read to complete. The existing loop went backwards (using a forward iterator), but stopped when the iterator reached the beginning of the map, or when a waiter belonged to the left BufferHead. If the first list of waiters should have been moved to the right BufferHead, it was skipped because at that point the iterator was at the beginning of the map, which was the main condition of the loop. Restructure the waiters-moving loop to go forward in the map instead, so it's harder to make an off-by-one error. Possibly-fixes: #3286 Signed-off-by: Josh Durgin (cherry picked from commit 2e862f4d183d8b57b43b0777737886f18f68bf00) --- src/osdc/ObjectCacher.cc | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/osdc/ObjectCacher.cc b/src/osdc/ObjectCacher.cc index a46ffb3bece..d8b51089f3d 100644 --- a/src/osdc/ObjectCacher.cc +++ b/src/osdc/ObjectCacher.cc @@ -52,21 +52,22 @@ ObjectCacher::BufferHead *ObjectCacher::Object::split(BufferHead *left, loff_t o right->bl.substr_of(bl, left->length(), right->length()); left->bl.substr_of(bl, 0, left->length()); } - + // move read waiters if (!left->waitfor_read.empty()) { - map >::iterator o, p = left->waitfor_read.end(); - p--; - while (p != left->waitfor_read.begin()) { - if (p->first < right->start()) break; + map >::iterator start_remove = left->waitfor_read.begin(); + while (start_remove != left->waitfor_read.end() && + start_remove->first < right->start()) + ++start_remove; + for (map >::iterator p = start_remove; + p != left->waitfor_read.end(); ++p) { ldout(oc->cct, 0) << "split moving waiters at byte " << p->first << " to right bh" << dendl; right->waitfor_read[p->first].swap( p->second ); - o = p; - p--; - left->waitfor_read.erase(o); + assert(p->second.empty()); } + left->waitfor_read.erase(start_remove, left->waitfor_read.end()); } - + ldout(oc->cct, 20) << "split left is " << *left << dendl; ldout(oc->cct, 20) << "split right is " << *right << dendl; return right; -- cgit v1.2.1