On Mon, Apr 04, 2016 at 09:06:47AM -0600, Jan Beulich wrote:
> >>> On 24.03.16 at 21:00, <konrad.w...@oracle.com> wrote:
> > --- a/xen/common/xsplice.c
> > +++ b/xen/common/xsplice.c
> > @@ -566,6 +566,27 @@ static int prepare_payload(struct payload *payload,
> > if ( !payload->id.len || !payload->id.p )
> > return -EINVAL;
> > }
> > + /* Make sure it is not a duplicate. */
> > + if ( payload->id.len )
>
> The conditional is pointless considering the one visible in context
> above.
/me nods. Let me move it inside the braces above.
>
> > + {
> > + struct payload *data;
> > +
> > + spin_lock_recursive(&payload_lock);
> > + list_for_each_entry ( data, &payload_list, list )
> > + {
> > + /* No way payload is on the list. */
> > + ASSERT( data != payload );
> > + if ( data->id.len &&
> > + !memcmp(data->id.p, payload->id.p, data->id.len) )
> > + {
> > + spin_unlock_recursive(&payload_lock);
> > + dprintk(XENLOG_DEBUG, "%s%s: Already loaded as %s!\n",
> > + XSPLICE, elf->name, data->name);
> > + return -EEXIST;
> > + }
> > + }
> > + spin_unlock_recursive(&payload_lock);
>
> Similar question as asked elsewhere - with the lock getting dropped
> here, how is the "no duplicate" state going to be ensured by the
> time you actually load and insert this payload?
It won't. I've removed the spinlock usage here and we are keeping the spinlock
held at xsplice_upload.
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
