[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 15/18] xen/mem_sharing: VM forking
On Thu, Jan 9, 2020 at 9:03 AM Jan Beulich <jbeulich@xxxxxxxx> wrote: > > On 09.01.2020 16:57, Tamas K Lengyel wrote: > > On Thu, Jan 9, 2020 at 8:34 AM Jan Beulich <jbeulich@xxxxxxxx> wrote: > >> > >> On 09.01.2020 16:10, Roger Pau Monné wrote: > >>> On Thu, Jan 09, 2020 at 06:41:12AM -0700, Tamas K Lengyel wrote: > >>>> On Thu, Jan 9, 2020 at 3:29 AM Julien Grall <julien@xxxxxxx> wrote: > >>>>> > >>>>> Hi Tamas, > >>>>> > >>>>> On 08/01/2020 17:14, Tamas K Lengyel wrote: > >>>>>> +static int mem_sharing_fork(struct domain *d, struct domain *cd) > >>>>>> +{ > >>>>>> + int rc; > >>>>>> + > >>>>>> + if ( !d->controller_pause_count && > >>>>>> + (rc = domain_pause_by_systemcontroller(d)) ) > >>>>> > >>>>> AFAIU, the parent domain will be paused if it wasn't paused before and > >>>>> this will not be unpaused by the same hypercall. Right? > >>>> > >>>> Yes, it needs to remain paused as long as there are forks active from > >>>> it. Afterwards it can be unpaused. > >>> > >>> If you want the parent domain to remain paused for as long as the > >>> forks are active, shouldn't each fork increment the pause count on > >>> creation and decrement it when the fork is destroyed? > >>> > >>> How can you assure no other operation or entity has incremented > >>> controller_pause_count temporary and is likely to decrement it at some > >>> point while forks are still active? > >> > >> The _by_systemcontroller variants look wrong to be used here anyway. > >> Why is this not simply domain_{,un}pause()? > >> > > > > My reasoning was that by default the user should pause the parent VM > > before forking. This sanity checks just mimicks that step in case the > > user didn't do that already. But I guess either would work, I don't > > really see much difference between the two. > > The main difference is that the one you currently use updates > d->controller_pause_count, which can be updated by other domctls, but > which shouldn't be updated behind the back of a component in Xen which > needs the entity paused. > Alright, I'll switch it. Thanks, Tamas _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |