[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <1422526239.14137.3@smtp.corp.redhat.com>
Date: Thu, 29 Jan 2015 10:18:39 +0008
From: Jason Wang <jasowang@...hat.com>
To: Vitaly Kuznetsov <vkuznets@...hat.com>
Cc: Dexuan Cui <decui@...rosoft.com>,
KY Srinivasan <kys@...rosoft.com>,
"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
Haiyang Zhang <haiyangz@...rosoft.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Radim Krčmář <rkrcmar@...hat.com>,
Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH v3 3/3] Drivers: hv: vmbus: serialize Offer and Rescind
offer
On Wed, Jan 28, 2015 at 9:09 PM, Vitaly Kuznetsov <vkuznets@...hat.com>
wrote:
> Dexuan Cui <decui@...rosoft.com> writes:
>
>>> -----Original Message-----
>>> From: Vitaly Kuznetsov [mailto:vkuznets@...hat.com]
>>> Sent: Wednesday, January 28, 2015 20:09 PM
>>> To: Dexuan Cui
>>> Cc: KY Srinivasan; devel@...uxdriverproject.org; Haiyang Zhang;
>>> linux-
>>> kernel@...r.kernel.org; Jason Wang; Radim Krčmář; Dan Carpenter
>>> Subject: Re: [PATCH v3 3/3] Drivers: hv: vmbus: serialize Offer
>>> and Rescind
>>> offer
>>>
>>> Dexuan Cui <decui@...rosoft.com> writes:
>>>
>>> >> -----Original Message-----
>>> >> From: Vitaly Kuznetsov [mailto:vkuznets@...hat.com]
>>> >> Sent: Tuesday, January 20, 2015 23:45 PM
>>> >> To: KY Srinivasan; devel@...uxdriverproject.org
>>> >> Cc: Haiyang Zhang; linux-kernel@...r.kernel.org; Dexuan Cui;
>>> Jason Wang;
>>> >> Radim Krčmář; Dan Carpenter
>>> >> Subject: [PATCH v3 3/3] Drivers: hv: vmbus: serialize Offer and
>>> Rescind
>>> offer
>>> ...
>>> >
>>> > Hi Vitaly and all,
>>> > I have 2 questions:
>>> > In vmbus_process_offer(), in the cases of "goto err_free_chan",
>>> > should we consider the possibility a rescind message could be
>>> pending for
>>> > the new channel?
>>> > In the cases, because we don't run
>>> > "INIT_WORK(&newchannel->work, vmbus_process_rescind_offer); ",
>>> > vmbus_onoffer_rescind() will do nothing and as a result,
>>> > vmbus_process_rescind_offer() won't be invoked.
>>>
>>> Yes, but processing the rescind offer results in freeing the
>>> channel
>>> (and this processing supposes the channel wasn't freed before) so
>>> there is no difference... or is it?
>>>
>>> >
>>> > Question 2: in vmbus_process_offer(), in the case
>>> > vmbus_device_register() fails, we'll run
>>> > "list_del(&newchannel->listentry);" -- just after this line,
>>> > what will happen at this time if relid2channel() returns NULL
>>> > in vmbus_onoffer_rescind()?
>>> >
>>> > I think we'll lose the rescind message.
>>> >
>>>
>>> Yes, but same logic applies - we already freed the channes so no
>>> rescind
>>> proccessing required.
>> free_channel() and vmbus_process_rescind_offer() are different,
>> because
>> the latter does more work, e.g., sending the host a message
>> CHANNELMSG_RELID_RELEASED.
>>
>> In the cases of "goto err_free_chan" + "a pending rescind message",
>> the host may expect the message CHANNELMSG_RELID_RELEASED and
>> could reoffer the channel once the message is received.
>>
>> It would be better if the VM doesn't lose the rescind message
>> here. :-)
>
> Ah, I see, CHANNELMSG_RELID_RELEASED is expected from us in any
> case. I'll doing that in a separate patch is noone objects.
All the evil come from the un-serialized processing of message. I
wonder whether we can do all the processing in workqueue and then those
were automatically serialized.
>
> Thanks for the review,
>
>>
>>> If we still need to do something we need to
>>> add support for already freed channel to the rescind offer
>>> processing path.
>>>
>>
>> Thanks,
>> -- Dexuan
>
> --
> Vitaly
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists