Bug #798
closedshould have a empty() test on list<Messegger *>
Added by longguang yue over 13 years ago. Updated about 5 years ago.
0%
Description
void SimpleMessenger::dispatch_entry(){
...
pipe->pipe_lock.Lock();
list<Message *>& m_queue = pipe->in_q[priority];
If(!m_queue.empty())
{
Message *m = m_queue.front();
m_queue.pop_front();
}
// Message *m = m_queue.front();
// m_queue.pop_front();Updated by longguang yue over 13 years ago
void SimpleMessenger::dispatch_entry(){
...
pipe->pipe_lock.Lock();
list<Message *>& m_queue = pipe->in_q[priority];
Message *m = NULL;
If(!m_queue.empty()) {
m = m_queue.front();
m_queue.pop_front();
}
Updated by Sage Weil over 13 years ago
- Category set to msgr
- Status changed from New to In Progress
- Assignee set to Sage Weil
Hi Longguang,
I took a look at this and think it's okay. Basically the invariant is that the pipe should be in the dispatch queue if and only if the pipe queue is non-empty. Actually, the queuing side was a bit sloppy.. this patch cleans it up I think... does that look reasonable? Or is there a race case I'm missing?
Thanks!
sage
Subject: [PATCH] msgr: clean up Pipe::queue_received locking
Ensure we maintain the invariant that a pipe has a non-empty queue IFF
the pipe is queued.
Prompted by #798.
Signed-off-by: Sage Weil <sage.weil@dreamhost.com>
---
src/msg/SimpleMessenger.cc | 44 +++++++++++++++++++++++++-------------------
1 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/src/msg/SimpleMessenger.cc b/src/msg/SimpleMessenger.cc
index 88f1b8a..6200627 100644
--- a/src/msg/SimpleMessenger.cc
+++ b/src/msg/SimpleMessenger.cc
@@ -534,38 +534,44 @@ void SimpleMessenger::Pipe::queue_received(Message *m, int priority)
assert(pipe_lock.is_locked());
list<Message *>& queue = in_q[priority];
- bool was_empty;
if (halt_delivery)
goto halt;
- was_empty = queue.empty();
- queue.push_back(m);
-
- //increment queue length counters
- in_qlen++;
- messenger->dispatch_queue.qlen_lock.lock();
- ++messenger->dispatch_queue.qlen;
- messenger->dispatch_queue.qlen_lock.unlock();
-
- if (was_empty) { //this pipe isn't on the endpoint queue
+ if (queue.empty()) {
+ // queue pipe AND message under pipe AND dispatch_queue locks.
pipe_lock.Unlock();
messenger->dispatch_queue.lock.Lock();
pipe_lock.Lock();
-
+
if (halt_delivery) {
messenger->dispatch_queue.lock.Unlock();
goto halt;
}
-
- dout(20) << "queue_received queuing pipe" << dendl;
- if (!queue_items.count(priority))
- queue_items[priority] = new xlist<Pipe *>::item(this);
- if (messenger->dispatch_queue.queued_pipes.empty())
- messenger->dispatch_queue.cond.Signal();
- messenger->dispatch_queue.queued_pipes[priority].push_back(queue_items[priority]);
+
+ if (queue.empty()) {
+ dout(20) << "queue_received queuing pipe" << dendl;
+ if (!queue_items.count(priority))
+ queue_items[priority] = new xlist<Pipe *>::item(this);
+ if (messenger->dispatch_queue.queued_pipes.empty())
+ messenger->dispatch_queue.cond.Signal();
+ messenger->dispatch_queue.queued_pipes[priority].push_back(queue_items[priority]);
+ }
+
+ queue.push_back(m);
+
messenger->dispatch_queue.lock.Unlock();
+ } else {
+ // just queue message under pipe lock.
+ queue.push_back(m);
}
+
+ // increment queue length counters
+ in_qlen++;
+ messenger->dispatch_queue.qlen_lock.lock();
+ ++messenger->dispatch_queue.qlen;
+ messenger->dispatch_queue.qlen_lock.unlock();
+
return;
halt:
--
Updated by longguang yue over 13 years ago
a exception is throwed out when pop_front()
------gdb msg--------------------------
-----------------
[root@mods2 ~]# whereis cmon
cmon: /usr/bin/cmon
[root@mods2 ~]# gdb /usr/bin/cm -c core.14331
cmds cmon cmp
[root@mods2 ~]# gdb /usr/bin/cmon -c core.14331
GNU gdb (GDB) Fedora (7.0-3.fc12)
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/bin/cmon...Reading symbols from /usr/lib/debug/usr/bin/cmon.debug...done.
done.
[New Thread 14338]
[New Thread 14336]
[New Thread 14331]
[New Thread 14332]
[New Thread 14589]
[New Thread 14368]
[New Thread 14388]
[New Thread 14389]
[New Thread 14340]
[New Thread 14341]
[New Thread 14333]
[New Thread 14369]
[New Thread 14590]
[New Thread 14339]
Reading symbols from /lib64/libpthread.so.0...(no debugging symbols found)...done.
Loaded symbols for /lib64/libpthread.so.0
Reading symbols from /usr/lib64/libcrypto.so.10...(no debugging symbols found)...done.
Loaded symbols for /usr/lib64/libcrypto.so.10
Reading symbols from /usr/lib64/libstdc++.so.6...(no debugging symbols found)...done.
Loaded symbols for /usr/lib64/libstdc++.so.6
Reading symbols from /lib64/libm.so.6...(no debugging symbols found)...done.
Loaded symbols for /lib64/libm.so.6
Reading symbols from /lib64/libgcc_s.so.1...(no debugging symbols found)...done.
Loaded symbols for /lib64/libgcc_s.so.1
Reading symbols from /lib64/libc.so.6...(no debugging symbols found)...done.
Loaded symbols for /lib64/libc.so.6
Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
Reading symbols from /lib64/libdl.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib64/libdl.so.2
Reading symbols from /lib64/libz.so.1...(no debugging symbols found)...done.
Loaded symbols for /lib64/libz.so.1
Core was generated by `/usr/bin/cmon -i 2 -c /tmp/ceph.conf.23467'.
Program terminated with signal 6, Aborted.
#0 0x00007fd99adc96c5 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install glibc-2.11.2-1.x86_64 libgcc-4.4.2-7.fc12.x86_64 libstdc++-4.4.2-7.fc12.x86_64 openssl-1.0.0-4.fc12.x86_64 zlib-1.2.3-23.fc12.x86_64
(gdb) bt
#0 0x00007fd99adc96c5 in raise () from /lib64/libc.so.6
#1 0x00007fd99adcaea5 in abort () from /lib64/libc.so.6
#2 0x00007fd99ae06133 in __libc_message () from /lib64/libc.so.6
#3 0x00007fd99ae0bac6 in malloc_printerr () from /lib64/libc.so.6
#4 0x0000000000454cf2 in deallocate (this=<value optimized out>, __p=<value optimized out>)
at /usr/include/c++/4.4.2/ext/new_allocator.h:95
#5 _M_put_node (this=<value optimized out>, __p=<value optimized out>) at /usr/include/c++/4.4.2/bits/stl_list.h:320
#6 _M_erase (this=<value optimized out>, __p=<value optimized out>) at /usr/include/c++/4.4.2/bits/stl_list.h:1431
#7 pop_front (this=<value optimized out>, __p=<value optimized out>) at /usr/include/c++/4.4.2/bits/stl_list.h:906
#8 SimpleMessenger::dispatch_entry (this=<value optimized out>, __p=<value optimized out>) at msg/SimpleMessenger.cc:290
#9 0x000000000044525c in SimpleMessenger::DispatchThread::entry (this=0x20a5818) at msg/SimpleMessenger.h:558
#10 0x000000000045a9da in Thread::_entry_func (arg=<value optimized out>) at common/Thread.h:39
#11 0x00007fd99bc57a3a in start_thread () from /lib64/libpthread.so.0
#12 0x00007fd99ae7577d in clone () from /lib64/libc.so.6
#13 0x0000000000000000 in ?? ()
Updated by Sage Weil about 13 years ago
This was before 940d58d94ef6dec16affea88c5c79b1f5eba16db? Can you try to reproduce with the latest?
Thanks!
Updated by Sage Weil about 13 years ago
- Status changed from In Progress to Can't reproduce
Updated by Greg Farnum about 5 years ago
- Project changed from Ceph to Messengers
- Category deleted (
msgr)