Messages in this thread | | | Date | Thu, 29 Jan 2015 10:18:39 +0008 | From | Jason Wang <> | 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@redhat.com> wrote: > Dexuan Cui <decui@microsoft.com> writes: > >>> -----Original Message----- >>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com] >>> Sent: Wednesday, January 28, 2015 20:09 PM >>> To: Dexuan Cui >>> Cc: KY Srinivasan; devel@linuxdriverproject.org; Haiyang Zhang; >>> linux- >>> kernel@vger.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@microsoft.com> writes: >>> >>> >> -----Original Message----- >>> >> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com] >>> >> Sent: Tuesday, January 20, 2015 23:45 PM >>> >> To: KY Srinivasan; devel@linuxdriverproject.org >>> >> Cc: Haiyang Zhang; linux-kernel@vger.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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
| |