[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v12 2/3] x86/mem_sharing: reset a fork



On Thu, Mar 26, 2020 at 8:53 AM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
>
> On Thu, Mar 26, 2020 at 8:52 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> > On 26.03.2020 15:48, Tamas K Lengyel wrote:
> > > On Thu, Mar 26, 2020 at 4:17 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > >>
> > >> On 23.03.2020 18:04, Tamas K Lengyel wrote:
> > >>> +static int mem_sharing_fork_reset(struct domain *d, struct domain *pd)
> > >>> +{
> > >>> +    int rc;
> > >>> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > >>> +    struct page_info *page, *tmp;
> > >>> +
> > >>> +    spin_lock(&d->page_alloc_lock);
> > >>> +    domain_pause(d);
> > >>
> > >> Why do you take the lock first?
> > >
> > > No particular reason - does the order matter?
> >
> > I think you'd better avoid holding a lock for extended periods
> > of time. And what's perhaps worse, what if a vCPU of the domain
> > sits in Xen trying to acquire this lock - you'd deadlock trying
> > to pause the domain then.
>
> OK, I'll invert them order then.

It turns out we also need to take the recursive lock here since we'll
free the pages. Fixed now and everything works as expected.

Tamas



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.