Re: [Xen-devel] [PATCH v1 7/8]: PVH privcmd changes

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-10-08 at 20:10 +0100, Mukesh Rathor wrote:
> On Mon, 8 Oct 2012 10:21:42 +0100
> Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> 
> > On Fri, 2012-10-05 at 22:22 +0100, Mukesh Rathor wrote:
> > > On Fri, 5 Oct 2012 10:21:18 +0100
> > > Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > > 
> > > > On Thu, 2012-10-04 at 19:20 +0100, Mukesh Rathor wrote:
> > > > > On Thu, 4 Oct 2012 09:50:42 +0100
> > > > > Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > > > > 
> > > > > > 
> > > > > > Won't that break because on the second call you will pass in
> > > > > > the freshly allocated pointer and overwrite the exiting
> > > > > > (useful) one with it?
> > > > > 
> > > > > No, for xlate, I just check for NULL. I didn't think it was big 
> > > > > deal to special case xlate in this case. We got so many if
> > > > > xlate cases already thru the code. It leaves the semantics easy
> > > > > to understand: NULL == avail. 1 == locked PV. PTR == Locked
> > > > > PVH. I'll add a comment this time :).
> > > > 
> > > > The transition from NULL => Locked PVH still needs to be done
> > > > atomically and without clobbering any existing non-NULL value,
> > > > otherwise it doesn't actually protect against multiple mappings
> > > > like it is supposed to.
> > 
> > > Ok, changed it to, and tested it:
> > > 
> > > static int privcmd_enforce_singleshot_mapping(struct vm_area_struct
> > > *vma) {
> > >         if (xen_feature(XENFEAT_auto_translated_physmap)) {
> > >                 int sz = sizeof(vma->vm_private_data);
> > >                 return (!__cmpxchg(&vma->vm_private_data, NULL,
> > > NULL, sz));
> > 
> > Passing NULL for both old and new values can't be right, can it? Did
> > you test with something which tries to map twice?
> 
> well, if it's already set to pointer, then __cmpxchg would leave the ptr 
> alone and return it. The function would then return false which would 
> fail the api. OTOH, if it's NULL, it would continue and get set later.

What happens if a second thread tries to create a mapping while the
first is between the cmpxchg and the assignment? i.e. while the value is
still NULL.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel