[lxc-devel] [RFC PATCH] lxclock: Replace named sempahore with flock

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> On Fri, 24 May 2013 08:23:57 -0500
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> > > The problem: if a task is killed while holding a posix semaphore,
> > > there appears to be no way to have the semaphore be reliably
> > > autmoatically released.  The only trick which seemed promising
> > > is to store the pid of the lock holder in some file and have
> > > later lock seekers check whether that task has died.
> > > 
> > > Instead of going down that route, this patch switches from a
> > > named posix semaphore to flock.  The advantage is that when
> > > the task is killed, its fds are closed and locks are automatically
> > > released.
> > > 
> > > The disadvantage of flock is that we can't rely on it to exclude
> > > threads.  Therefore c->slock must now always be wrapped inside
> > > c->privlock.
> > > 
> > > This patch survived basic testing with the lxcapi_create patchset,
> > > where now killing lxc-create while it was holding the lock did
> > > not lock up future api commands.
> > 
> > Two more notes:
> > 
> > 1. the new lock doesn't support timeouts like the old one did.
> > There's no caller which is currently using timeouts, so I will
> > probably remove timeouts from the private semaphore as well.
> > 
> > 2. It doesn't seem necessary to require everyone to understand
> > the details, so I may abstrace away knowledge of c->privlock
> > and c->slock behind two helpers.  Not sure what good names would
> > be, maybe c->memlock() and c->disklock() ?  c->threadlock()
> > and c->globallock()?  Something to indicate that the first is
> > to protect the struct lxc_container from simultaneous updates
> > from other threads, while the latter is to protect the on-disk
> > container.
> 
> That would be nice. I'm not sure this is what you're getting at but it
> would also be nice if the caller didn't have to know they needed the
> thread lock before going for the disk lock (as a result of the

Exactly.  So c->disklock() would grab c->memlock(), simplifying
lxccontainer.c quite a bit already.

> primitive we're using for the disk lock). So getting the disk lock
> would automatically acquire the thread lock for the caller.

Do you have any suggestions for better names?  :)  I don't like mine,
but can't think of anything better.

> I think I'll have a few more comments/questions after I read through the
> rest of the patch.

Excellent, thanks.

-serge