On Tue, Jun 18, 2013 at 8:57 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Thu, Jun 13, 2013 at 05:03:05PM +0800, Liu Ping Fan wrote:
>> From: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
>>
>> Nested call caused by ->receive() will raise issue like deadlock,
>> so postphone it to BH.
>>
>> Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
>> ---
>> net/queue.c | 40 ++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> Does this patch belong before the netqueue lock patch? The commit
> history should be bisectable without temporary failures/deadlocks.
>
Ok.
>> diff --git a/net/queue.c b/net/queue.c
>> index 58222b0..9c343ab 100644
>> --- a/net/queue.c
>> +++ b/net/queue.c
>> @@ -24,6 +24,8 @@
>> #include "net/queue.h"
>> #include "qemu/queue.h"
>> #include "net/net.h"
>> +#include "block/aio.h"
>> +#include "qemu/main-loop.h"
>>
>> /* The delivery handler may only return zero if it will call
>> * qemu_net_queue_flush() when it determines that it is once again able
>> @@ -183,6 +185,22 @@ static ssize_t qemu_net_queue_deliver_iov(NetQueue
>> *queue,
>> return ret;
>> }
>>
>> +typedef struct NetQueBH {
>
> This file uses "Queue" consistently, please don't add "Que" here.
>
>> @@ -192,8 +210,17 @@ ssize_t qemu_net_queue_send(NetQueue *queue,
>> {
>> ssize_t ret;
>>
>> - if (queue->delivering || !qemu_can_send_packet_nolock(sender)) {
>> + if (queue->delivering || !qemu_can_send_packet_nolock(sender)
>> + || sender->send_queue->delivering) {
>
> Not sure this is safe, we're only holding one NetClientState->peer_lock
> and one NetQueue->lock. How can we access both queue->delivering and
> sender->send_queue->delivering safely?
Yes, you are right, it is not safely. The queue->delivering is
protected by peer_lock and we do not take the verse direction lock .
So finally the above code can not tell out the nested calling
"A-->B-->A" from "A-->B, B-->A" (where A, B stands for a
NetClientState).
What about using TLS to trace the nested calling? With it, we can
avoid AB-BA lock problem.
Thx & regards,
Pingfan
