Hi all,
Sorry for coming too late to the discussion.
In fact, I've already acked [1] the changeset.
But after sleeping more on it, I'm concerned about __thread and bionic.
Quoting android-ndk-r9c/docs/text/system/libc/OVERVIEW.text:
At the moment, thread-local storage defined through the __thread compiler
keyword is not supported by the Bionic C library and dynamic linker.
The same OVERVIEW.TXT was present until recently in bionic sources [2].
Moreover, grepping (today's git HEAD) bionic sources for __thread, PT_TLS
(thread-local storage segment), .tbss and .tdata section names shows nothing.
So, while utils.c compiles fine (due to GNU gcc/binutils support for __thread), lxc-*
will likely segfault on the first access to the "values" array (because of
missing memory segment due to bionic dynamic loader's lack of __thread support).
I suppose, we could allocate a TLS slot for "values" pointer via
pthread_key_create() and set it to a per-thread malloc()'ed memory chunk via
pthread_setspecific(). (this seems to be quite verbose though)
Saying this, it would be very nice to run-time test related changes.
On Thu, 19 Dec 2013 16:29:52 +0400, Stéphane Graber <[email protected]> wrote:
On Thu, Dec 19, 2013 at 11:25:33AM +0100, Stéphane Graber wrote:
On Wed, Dec 18, 2013 at 11:22:27PM -0600, Serge Hallyn wrote:
> Quoting S.Çağlar Onur ([email protected]):
[snip]
> > I ended up placing values in the thread local storage pool, instead of doing
"unlock the lock in the child" dance
> >
> > Signed-off-by: S.Çağlar Onur <[email protected]>
>
> only question is, does this work in bionic? So long as it does,
>
> Acked-by: Serge E. Hallyn <[email protected]>
I'll do a test build in a minute, if that works, I'll push to master.
Test build passed fine using the Android NDK.
I don't have hardware around me to test the result, but I think the
concerned was more about building/linking than run time.
Commit pushed to master.
[snip]
> > diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> > index b605307..785f3e6 100644
> > --- a/src/lxc/utils.c
> > +++ b/src/lxc/utils.c
> > @@ -253,8 +253,8 @@ const char *lxc_global_config_value(const char
*option_name)
> > { "cgroup.use", NULL },
> > { NULL, NULL },
> > };
> > - /* Protected by a mutex to eliminate conflicting load and store
operations */
> > - static const char *values[sizeof(options) / sizeof(options[0])] = { 0
};
> > + /* placed in the thread local storage pool */
> > + static __thread const char *values[sizeof(options) /
sizeof(options[0])] = { 0 };
> > const char *(*ptr)[2];
> > const char *value;
> > size_t i;
[snip]
P.S.
It seems, that linuxcontainers.org points to [3] while the actual mailing-list
info and archive reside at [4] (note https vs http).
Is it a transient error?
[1] https://github.com/lxc/lxc/pull/106#issuecomment-30920691
[2]
https://android.googlesource.com/platform/bionic/+/0493a6f7be42e22d68e1d6ddb8eb2edaf818756f
[3] https://lists.linuxcontainers.org/listinfo/lxc-devel
[4] http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Andrey Mazo.
_______________________________________________
lxc-devel mailing list
[email protected]
http://lists.linuxcontainers.org/listinfo/lxc-devel
