Re: [Xen-devel] [PATCH v4 5/7] libxl: support unmapping static shared memory areas during domain destruction

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Feb 12, 2018 at 03:24:26PM +0000, Julien Grall wrote:
> 
> 
> On 12/02/18 15:17, Zhongze Liu wrote:
> > Hi Julien,
> 
> Hi,
> 
> > 
> > 2018-02-12 23:09 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:
> > > Hi,
> > > 
> > > On 12/02/18 14:52, Zhongze Liu wrote:
> > > > 
> > > > 2018-02-08 0:54 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:
> > > > > 
> > > > > On 07/02/18 16:27, Zhongze Liu wrote:
> > > > 
> > > > It seems that I mistakenly use transaction as a global lock. Now I don't
> > > > have
> > > > any reasons not putting the unmap out of the transaction, but this will
> > > > break
> > > > the original transaction into two, and I do think that we need some
> > > > explicit
> > > > locking here.
> > > 
> > > 
> > > Can you explain why you need specific locking here? What are you trying to
> > > protect? Are you trying to protect against two process doing the unmap?
> > > 
> > 
> > Yes.
> 
> I don't think you have to worry about the locking here. With the current
> interface, the regions cannot be modified after the guest has booted. So the
> addresses will always stay valid.
> 
> This code path should never be called concurrently, as a lot of code in
> libxl, so I think someone else will take care about that for you (I will let
> Wei confirm here).
> 
> In any case, the worst that could happen is the unmap is called twice on the
> same region. So you would get spurious error message. Not that bad.

Yeah, not that bad. Not going to be a security issue, not going to leak
resources in the end.

To avoid spurious unmap, can we maybe unmap the pages after the xenstore
transaction is committed? In that case, only the successful one gets to
unmap, the ones that aren't committed will bail.

(Just tossing around ideas)

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel