Re: [Xen-devel] [PATCH v1 02/10] libxl_internal: functions to lock / unlock domain configuration

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-07-17 at 12:41 +0100, Wei Liu wrote:
> > > > > If a thread cannot get hold of the lock it waits due to F_SETLKW.
> > > > 
> > > > How is deadlock avoided here?
> > > > 
> > > 
> > > This patch only provides "primitive" to lock and unlock. Avoiding
> > > deadlock is out of scope of this patch. That pretty much depends on the
> > > users of this lock.
> > 
> > Hrm. It would be conventional to describe for the benefit of those users
> > what the requirements are. e.g. by defining a locking hierarchy for
> > libxl. e.g. relative to the CTX lock.
> > 
> 
> This lock is only used internally by libxl. I guess I should document it
> in libxl_internal.h? There doesn't seem to a existing doc on locking
> hierarchy in either libxl.h or libxl_internal.h. I guess the
> introduction of this lock is the first instance that needs documented?

The context lock is documented in libxl_internal.h. It says there that
it must be the inner most lock, so it would be invalid to take the CTX
lock with your new lock held for example.

> > It might also be necessary  to specify what the various event callbacks
> > are allowed to assume here -- but Ian knows the event code best so I'll
> > let him comment.
> > 
> > > > > In order to generate lock file name, rename userdata_path to
> > > > > libxl__userdata_path and declare it in libxl_internal.h
> > > > 
> > > > Given that you don't appear to be locking the userdata file itself that
> > > > seems like an odd place for a lock to me, why not XEN_LOCK_DIR?
> > > > 
> > > 
> > > So that the lock file itself gets deleted by libxl automatically. Using
> > > XEN_LOCK_DIR requires some extra care to delete the lock file. And if
> > > libxl user crashes for any reason, those files become stale. Leaving
> > > them in one location is easier to clean them up.
> > 
> > There's only one (empty) file though, isn't there? and it will be reused
> > by any subsequent libxl call from any process. IOW I don't think you
> > need to concern yourself with removing it.
> > 
> 
> No, it's per-domain, so that we don't content on it (now I see why you
> said it would be a bottleneck because you thought there's only one
> file). Leaving a bunch of stale files is not that nice, isn't it?

Yes, I misread something earlier and thought there was only one lock
file. Leaving lots of files around is indeed to be avoided.

If you are going to use libxl__userdata_path for this then you should
add libxl-json.lock to the registry of userdata types.

Ian.


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