[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/6] net: defer nested call to BH
From: |
liu ping fan |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/6] net: defer nested call to BH |
Date: |
Thu, 20 Jun 2013 14:30:56 +0800 |
On Tue, Jun 18, 2013 at 8:57 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Thu, Jun 13, 2013 at 05:03:05PM +0800, Liu Ping Fan wrote:
>> From: Liu Ping Fan <address@hidden>
>>
>> Nested call caused by ->receive() will raise issue like deadlock,
>> so postphone it to BH.
>>
>> Signed-off-by: Liu Ping Fan <address@hidden>
>> ---
>> 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
- Re: [Qemu-devel] [PATCH v2 2/6] net: introduce lock to protect NetClientState's peer's access, (continued)
[Qemu-devel] [PATCH v2 3/6] net: make netclient re-entrant with refcnt, Liu Ping Fan, 2013/06/13
[Qemu-devel] [PATCH v2 4/6] net: force NetQue opaque to be NetClientState, Liu Ping Fan, 2013/06/13
[Qemu-devel] [PATCH v2 5/6] net: defer nested call to BH, Liu Ping Fan, 2013/06/13
[Qemu-devel] [PATCH v2 6/6] net: hub use lock to protect ports list, Liu Ping Fan, 2013/06/13
Re: [Qemu-devel] [PATCH v2 0/6] port network layer onto glib, Stefan Hajnoczi, 2013/06/18