>>> On 06.07.16 at 17:51, <cz...@bitdefender.com> wrote: > @@ -492,8 +493,12 @@ int vcpu_initialise(struct vcpu *v) > > void vcpu_destroy(struct vcpu *v) > { > - xfree(v->arch.vm_event); > - v->arch.vm_event = NULL; > + if ( unlikely(v->arch.vm_event) ) > + { > + xfree(v->arch.vm_event->emul_read_data); > + xfree(v->arch.vm_event); > + v->arch.vm_event = NULL; > + }
Considering the repeat of this pattern ... > @@ -52,8 +58,25 @@ void vm_event_cleanup_domain(struct domain *d) > > for_each_vcpu ( d, v ) > { > - xfree(v->arch.vm_event); > - v->arch.vm_event = NULL; > + if ( likely(!v->arch.vm_event) ) > + continue; > + > + /* > + * Only xfree the entire arch_vm_event if write_data was fully > handled. > + * Otherwise defer entire xfree until domain/vcpu destroyal. > + */ > + if ( likely(!v->arch.vm_event->write_data.uncommitted_writes) ) > + { > + xfree(v->arch.vm_event->emul_read_data); > + xfree(v->arch.vm_event); > + v->arch.vm_event = NULL; > + continue; > + } ... here, please consider making this another helper (inline?) function. > + /* write_data not fully handled, only xfree emul_read_data */ Comment style again (and more below). > --- a/xen/common/vm_event.c > +++ b/xen/common/vm_event.c > @@ -534,6 +534,8 @@ static void mem_sharing_notification(struct vcpu *v, > unsigned int port) > /* Clean up on domain destruction */ > void vm_event_cleanup(struct domain *d) > { > + struct vcpu *v; > + > #ifdef CONFIG_HAS_MEM_PAGING > if ( d->vm_event->paging.ring_page ) > { > @@ -560,6 +562,16 @@ void vm_event_cleanup(struct domain *d) > (void)vm_event_disable(d, &d->vm_event->share); > } > #endif > + > + for_each_vcpu ( d, v ) > + { > + if ( unlikely(v->arch.vm_event) ) > + { > + /* vm_event->emul_read_data freed in vm_event_cleanup_domain */ Perhaps worthwhile adding a respective ASSERT()? > +static inline bool_t vm_event_vcpu_initialised(struct vcpu *v) > +{ > + return (v->arch.vm_event && v->arch.vm_event->emul_read_data); > +} > + > +static inline bool_t vm_event_domain_initialised(struct domain *d) > +{ > + return (d->max_vcpus && d->vcpu[0] && > + vm_event_vcpu_initialised(d->vcpu[0])); > +} Both functions' parameters should be const. Pointless parentheses in both return statements. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel