Quoting Dwight Engen (dwight.en...@oracle.com):
> On Wed, 29 May 2013 22:21:22 -0500
> Serge Hallyn <serge.hal...@ubuntu.com> wrote:
>
> > And update the comment explaining the locking.
> >
> > Also take memlock in want_daemonize.
>
> Hi Serge, could you explain a bit what the locking is protecting about
> open/close? It looks like you are locking around just the open or
> just the close, and not the whole open to close region so I'm a bit
> confused on what scenario the locks are protecting against.
My understanding is that open and close themselves, i.e. the
modification of the fdtable, are not thread-safe. But once
open has completed, read/write will be thread-safe.
> > Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>
> > ---
> > src/lxc/lxccontainer.c | 153
> > +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 115
> > insertions(+), 38 deletions(-)
> >
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index 9bc1caf..145ed7c 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -131,7 +131,12 @@ void remove_partial(struct lxc_container *c, int
> > fd) char *path = alloca(len);
> > int ret;
> >
> > + if (process_lock()) {
> > + ERROR("Failed getting process lock");
> > + return;
> > + }
> > close(fd);
> > + process_unlock();
> > ret = snprintf(path, len, "%s/%s/partial", c->config_path,
> > c->name); if (ret < 0 || ret >= len) {
> > ERROR("Error writing partial pathname");
> > @@ -145,11 +150,11 @@ void remove_partial(struct lxc_container *c,
> > int fd) }
> >
> > /* LOCKING
> > - * 1. c->privlock protects the struct lxc_container from multiple
> > threads.
> > - * 2. c->slock protects the on-disk container data
> > + * 1. container_mem_lock(c) protects the struct lxc_container from
> > multiple threads.
> > + * 2. container_disk_lock(c) protects the on-disk container data -
> > in particular the
> > + * container configuration file.
> > + * The container_disk_lock also takes the container_mem_lock.
> > * 3. thread_mutex protects process data (ex: fd table) from
> > multiple threads
> > - * slock is an flock, which does not exclude threads. Therefore
> > slock should
> > - * always be wrapped inside privlock.
> > * NOTHING mutexes two independent programs with their own struct
> > * lxc_container for the same c->name, between API calls. For
> > instance,
> > * c->config_read(); c->start(); Between those calls, data on disk
> > @@ -160,7 +165,7 @@ void remove_partial(struct lxc_container *c, int
> > fd)
> > * due to hung callers. So I prefer to keep the locks only within
> > our own
> > * functions, not across functions.
> > *
> > - * If you're going to fork while holding a lxccontainer, increment
> > + * If you're going to clone while holding a lxccontainer, increment
> > * c->numthreads (under privlock) before forking. When deleting,
> > * decrement numthreads under privlock, then if it hits 0 you can
> > delete.
> > * Do not ever use a lxccontainer whose numthreads you did not bump.
> > @@ -358,11 +363,14 @@ static pid_t lxcapi_init_pid(struct
> > lxc_container *c)
> > static bool load_config_locked(struct lxc_container *c, const char
> > *fname) {
> > + bool ret = false;
> > if (!c->lxc_conf)
> > c->lxc_conf = lxc_conf_init();
> > + process_lock();
>
> From here on you start not checking if process_lock() failed. Just
Yeah...
> curious as to why we need to check for failure above but not in the
> rest of these.
I was punting on making the decision :) Do you know whether it will
ever fail unless we're having a catastrophic error? I'm tempted to say
that if it fails, then process_lock() rather than returnign an error
will log the error and exit the calling program.
> FWIW I'd much prefer not having to check in all calls if we can
> convince ourselves its not needed. The only reasons I can see that
> process_lock()'s pthread_mutex_lock() would fail would be EBUSY, EINVAL,
> EDEADLK all of which would be because of errors in lxc itself so maybe
> we could make process_lock() abort() or exit() if pthread_mutex_lock()
> returns an error?
Sounds good to me then!
-serge
------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
