Re: [PATCH v3 3/3] Drivers: hv: vmbus: serialize Offer and Rescind offer

[Jason Wang]

On Wed, Jan 28, 2015 at 9:09 PM, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> 
wrote:
> Dexuan Cui <decui@xxxxxxxxxxxxx> writes:
>
> > >  -----Original Message-----
> > >  From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> > >  Sent: Wednesday, January 28, 2015 20:09 PM
> > >  To: Dexuan Cui
> > >  Cc: KY Srinivasan; devel@xxxxxxxxxxxxxxxxxxxxxx; Haiyang Zhang; 
> > > linux-
> > >  kernel@xxxxxxxxxxxxxxx; Jason Wang; Radim KrÄmÃÅ; Dan Carpenter
> > >  Subject: Re: [PATCH v3 3/3] Drivers: hv: vmbus: serialize Offer 
> > > and Rescind
> > >  offer
> > >  
> > >  Dexuan Cui <decui@xxxxxxxxxxxxx> writes:
> > >  
> > >  >> -----Original Message-----
> > >  >> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> > >  >> Sent: Tuesday, January 20, 2015 23:45 PM
> > >  >> To: KY Srinivasan; devel@xxxxxxxxxxxxxxxxxxxxxx
> > >  >> Cc: Haiyang Zhang; linux-kernel@xxxxxxxxxxxxxxx; 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@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/