From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 78323 invoked by alias); 18 May 2018 19:02:03 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 76043 invoked by uid 89); 18 May 2018 19:02:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=Further, initialised, user's X-HELO: mail-oi0-f66.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp :organization:message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=+Dk/8SPMxG0lj3Hp2z46VwfXtIUY8QxFsWyb2FBjWus=; b=nO8EvFnttedWg+M6VhEpBEF9pMZV5vqYc9c/j9ImBKnwHFQNB3v7JoswH0JApgf5m8 RxAEAOP1V32Z+50fRK3gRrOrkaIFl8zSvvM/tCrDVUgNdCszjMrfR/3oDLbNVH7CDN0g dbU8TMTirp9xiroI+6mYoergyX7IjFk/sZsGPFrumx6P10JGFTaz8C8QgtlMEjuVG3BU 4nMQxWgNJuEm6digDlguahi0XGquqMrzJUOkf5iLQ6Uqs0jy7nxaWM1E8Ld6Sf+QdtsT vLdEX7vr2WXJCw2NL+3A7DvPMtJ6WjGwIeeGTd18HfJkrVouB2lpT7Wux3+rHh7EL/jo wFXQ== X-Gm-Message-State: ALKqPwcCsaYjGredDZaH4q3KpYkOix6MITY2SRZu2LgopIXOb4vuTSsg 2QyL3ZZLy7mWle59Dms0PRJA0A== X-Google-Smtp-Source: AB8JxZpCZgs6wincAhApuLx9dw8xqtcaOnjzLIACvf6Z0Ul1nVcK5ltPLBuNL92uoooeT/DMgmIt1Q== X-Received: by 2002:aca:1a11:: with SMTP id a17-v6mr6550559oia.35.1526670118674; Fri, 18 May 2018 12:01:58 -0700 (PDT) Subject: Re: [RFC][PATCH v1 4/5] elf/dl-load.c, elf-dl-open.c: Implement RTLD_SHARED dlmopen cloning To: =?UTF-8?Q?Vivek_Das=c2=a0Mohapatra?= , libc-alpha@sourceware.org Cc: =?UTF-8?Q?Vivek_Das=c2=a0Mohapatra?= References: <20180516171124.24962-1-vivek@collabora.com> <20180516171124.24962-5-vivek@collabora.com> From: Carlos O'Donell Openpgp: preference=signencrypt Message-ID: <55e6d12d-2605-7b49-5ce5-fa142eb8cccf@redhat.com> Date: Fri, 18 May 2018 19:02:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180516171124.24962-5-vivek@collabora.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-05/txt/msg00668.txt.bz2 On 05/16/2018 01:11 PM, Vivek Das Mohapatra wrote: > From: Vivek Das Mohapatra > > This uses the new infrastructure to implement RTLD_SHARED object > cloning via dlmopen: Instead of opening the specified object in > the requested namespace we open it in the main namespace (if it > is not already present there) and clone it into the requated > namespace. My comment about "proxy' applies here too, it is a better name than clone IMO, but feel free to discuss. > The following rules apply: > > If a clone of the object is already present in the requested namespace, > we simply return it (with an incremented direct-open count). > > If the object is already present in the requested namespace, a dl > error is signalled, since we cannot satisfy the user's request. > > Clones are never created in the main namespace: RTLD_SHARED has no > effect when the requested namespace is LM_ID_BASE. Sounds good to me. Where are you ensuring that libc and libpthread are proxied into all the new namespaces? I assume that's going to be in some other patch which has to handle the basic glibc group of libraries? > --- > elf/dl-load.c | 34 ++++++++++++++++++++++++++++++++++ > elf/dl-open.c | 31 +++++++++++++++++++++++++++++-- > 2 files changed, 63 insertions(+), 2 deletions(-) > > diff --git a/elf/dl-load.c b/elf/dl-load.c > index a5e3a25462..a3bc85fb0a 100644 > --- a/elf/dl-load.c > +++ b/elf/dl-load.c > @@ -1847,6 +1847,40 @@ _dl_map_object (struct link_map *loader, const char *name, > assert (nsid >= 0); > assert (nsid < GL(dl_nns)); > > +#ifdef SHARED > + /* Only need to do cloning work if `nsid' is not LM_ID_BASE. */ > + if (__glibc_unlikely ((mode & RTLD_SHARED) && (nsid != LM_ID_BASE))) > + { > + /* Search the target namespace, in case the object is already there. > + Note that unlike the search in the next section we do not attempt to > + extract the object's name if it does not yet have one. */ > + for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next) > + { > + if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0)) > + continue; > + > + if (!_dl_name_match_p (name, l)) > + continue; > + > + /* We have a match - stop searching. */ > + break; > + } > + > + if (l) > + { > + if (l->l_clone) > + return l; > + > + _dl_signal_error (EEXIST, name, NULL, > + N_("object cannot be demoted to a clone")); > + } > + > + /* Further searches should be in the base ns: We will clone the > + resulting object in dl_open_worker *after* it is initialised. */ > + nsid = LM_ID_BASE; > + } This should be split into a helper function. I would like _dl_ma_object() to be easier to read. > +#endif > + > /* Look for this name among those already loaded. */ > for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next) > { > diff --git a/elf/dl-open.c b/elf/dl-open.c > index 9dde4acfbc..0c5c75c137 100644 > --- a/elf/dl-open.c > +++ b/elf/dl-open.c > @@ -237,6 +237,10 @@ dl_open_worker (void *a) > /* This object is directly loaded. */ > ++new->l_direct_opencount; > > + /* Clone already existed in the target ns, nothing left to do. */ > + if (__glibc_unlikely (new->l_clone)) > + return; OK. > + > /* It was already open. */ > if (__glibc_unlikely (new->l_searchlist.r_list != NULL)) > { > @@ -252,6 +256,16 @@ dl_open_worker (void *a) > > assert (_dl_debug_initialize (0, args->nsid)->r_state == RT_CONSISTENT); > > + if (__glibc_unlikely (mode & RTLD_SHARED)) > + { > + args->map = new = _dl_clone_object (new, mode, args->nsid); > + ++new->l_direct_opencount; > + > + if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)) > + _dl_debug_printf ("cloning file=%s [%lu]; direct_opencount=%u\n\n", > + new->l_name, new->l_ns, new->l_direct_opencount); > + } OK. > + > return; > } > > @@ -509,6 +523,11 @@ TLS generation counter wrapped! Please report this.")); > /* It failed. */ > return; > > + if (__glibc_unlikely (mode & RTLD_SHARED)) > + { > + args->map = _dl_clone_object (new, mode, args->nsid); > + ++args->map->l_direct_opencount; > + } OK. > #ifndef SHARED > /* We must be the static _dl_open in libc.a. A static program that > has loaded a dynamic object now has competition. */ > @@ -517,8 +536,16 @@ TLS generation counter wrapped! Please report this.")); > > /* Let the user know about the opencount. */ > if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)) > - _dl_debug_printf ("opening file=%s [%lu]; direct_opencount=%u\n\n", > - new->l_name, new->l_ns, new->l_direct_opencount); > + { > + _dl_debug_printf ("opening file=%s [%lu]; direct_opencount=%u\n\n", > + new->l_name, new->l_ns, new->l_direct_opencount); > + > + if (mode & RTLD_SHARED) > + _dl_debug_printf ("cloning file=%s [%lu]; direct_opencount=%u\n\n", > + args->map->l_name, > + args->map->l_ns, > + args->map->l_direct_opencount) > + } OK. > } > > > -- Cheers, Carlos.