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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-07-16 at 17:44 +0100, Wei Liu wrote:
> On Wed, Jul 16, 2014 at 05:15:41PM +0100, Ian Campbell wrote:
> > On Thu, 2014-07-10 at 15:32 +0100, Wei Liu wrote:
> > > Simple file lock taken from xl to serialise access to "libxl-json" file.
> > 
> > Do you mean libxl here?
> > 
> 
> No. I did mean "xl". This implementation is "borrowed" from "xl".


Ah, I thought you meant the lock was taken (as in locked) from xl, which
made no sense. Suggest inserting the word "implementation" in that
sentence somewhere.

> > > 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.

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.

> > Did you consider locking the actual file though? That would avoid this
> > lock being a bottleneck...
> > 
> 
> It's just that part of this series was taken from my older series where
> some macro already had its own code to manipulate configuration file,
> plumbing locking code (passing fd around) and those macros together
> would make code look complete new. I was a bit worried I would introduce
> too many changes that might confuse you maintainers. I can lock the file
> itself if you prefer. 
> 
> But I don't quite follow how this lock can be a bottleneck. Locking the
> actual file is different from locking a lock file? Isn't there one file
> to be locked anyway?

If you lock the actual file then you are only serialising operations on
a single domain, operations on other domains can go ahead (each with
their own lock).

If you lock a single file then all operations on all domains are
serialised.

Ian.



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