[DTrace-devel] [PATCH v2 10/20] libproc: add Pinode_to_map()
Kris Van Hees
kris.van.hees at oracle.com
Thu Sep 22 20:01:39 UTC 2022
On Wed, Sep 07, 2022 at 01:59:58PM +0100, Nick Alcock via DTrace-devel wrote:
> This lets you hand in a (dev, ino) pair and get back the prmap_file_t
> entry that corresponds to it. This is useful when dealing with uprobes,
> which work on an *inode* basis, not a process basis or even a mapping
> basis.
I think this could use a bit more explanation. For one, it fails to mention
that it actually takes a ps_prochandle, dev, and ino and returns a prmap_file_t
for that. I.e. this is functionality that operates within a given process.
The last sentence is therefore also a bit confusing because you mentin how this
is useful for uprones that do not work on a process basis, but the function
requires a process handle.
So, I think a better commit description is in order.
Also, the implementation *seems* more extensive than is really needed for this
patch series, but I assume that is with a view on the future?
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
> include/sys/sol_procfs.h | 8 ++--
> libproc/Pcontrol.h | 1 +
> libproc/Psymtab.c | 85 ++++++++++++++++++++++++++++++++++++----
> libproc/libproc.h | 5 ++-
> 4 files changed, 86 insertions(+), 13 deletions(-)
>
> diff --git a/include/sys/sol_procfs.h b/include/sys/sol_procfs.h
> index 2f7c6a4eafe5..7c5b0fce07d6 100644
> --- a/include/sys/sol_procfs.h
> +++ b/include/sys/sol_procfs.h
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2016, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> @@ -17,17 +17,17 @@ extern "C" {
> #include <sys/types.h>
> #include <sys/dtrace_types.h>
> #include <sys/procfs_isa.h>
> -#include <dt_list.h>
> #include <link.h>
>
> /*
> * The prmap_file points to all mappings corresponding to a single file, sorted
> * by address. prmap_files are hashed by name (including the terminating \0,
> - * so anonymous maps are all hashed together).
> + * so anonymous maps are all hashed together) and by dev/inode number.
> */
> struct prmap;
> typedef struct prmap_file {
> - struct prmap_file *prf_next; /* next in hash chain */
> + struct prmap_file *prf_next; /* next in filename hash chain */
> + struct prmap_file *prf_inum_next; /* next in inode hash chain */
Perhaps rename prf_next to prf_name_next, consistent with prf_inum_next?
> char *prf_mapname; /* name in /proc/<pid>/maps */
> struct prmap **prf_mappings; /* sorted by address */
> size_t prf_num_mappings; /* number of mappings */
> diff --git a/libproc/Pcontrol.h b/libproc/Pcontrol.h
> index 9c3340d04d54..77d71d98abf0 100644
> --- a/libproc/Pcontrol.h
> +++ b/libproc/Pcontrol.h
> @@ -247,6 +247,7 @@ struct ps_prochandle {
> map_info_t *mappings; /* process mappings, sorted by address */
> size_t num_mappings; /* number of mappings */
> prmap_file_t **map_files; /* hash of mappings by filename */
> + prmap_file_t **map_inum; /* hash of mappings by (dev, inode) */
> uint_t num_files; /* number of file elements in file_list */
> dt_list_t file_list; /* list of mapped files w/ symbol table info */
> auxv_t *auxv; /* the process's aux vector */
> diff --git a/libproc/Psymtab.c b/libproc/Psymtab.c
> index aca3382bdde5..ef383e3e19ef 100644
> --- a/libproc/Psymtab.c
> +++ b/libproc/Psymtab.c
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2009, 2020, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> @@ -70,6 +70,39 @@ string_hash(const char *key)
> return h;
> }
>
> +/*
> + * This is Pelle Evensen's NASAM mixer. Since we are only mixing in one value
> + * we don't need anything but a mixing step.
> + */
> +
> +#define ror(x,y) ((x) >> (y) | (x) << (64 - (y)))
> +
> +static uint64_t
> +nasam_mix(uint64_t h)
> +{
> +
> + h ^= ror(h, 25) ^ ror(h, 47);
> + h *= 0x9E6C63D0676A9A99UL;
> + h ^= h >> 23 ^ h >> 51;
> + h *= 0x9E6D62D06F6A9A9BUL;
> + h ^= h >> 23 ^ h >> 51;
> +
> + return h;
> +}
> +#undef ror
> +
> +static ino_t
> +dev_inum_hash(dev_t dev, ino_t inum)
> +{
> + uint64_t h;
> +
> + h = nasam_mix(inum);
> + h += dev;
> + h = nasam_mix(h);
> +
> + return (ino_t) h;
> +}
> +
> /*
> * Allocation function for a new file_info_t.
> */
> @@ -182,6 +215,7 @@ mapping_purge(struct ps_prochandle *P)
> }
>
> memset(P->map_files, 0, sizeof(struct prmap_file *) * MAP_HASH_BUCKETS);
> + memset(P->map_inum, 0, sizeof(struct prmap_file *) * MAP_HASH_BUCKETS);
>
> for (i = 0, fptr = dt_list_next(&P->file_list);
> i < P->num_files; i++, fptr = dt_list_next(fptr)) {
> @@ -208,6 +242,11 @@ Psym_init(struct ps_prochandle *P)
> _dprintf("Out of memory initializing map_files hash\n");
> return -ENOMEM;
> }
> + P->map_inum = calloc(MAP_HASH_BUCKETS, sizeof(struct prmap_file_t *));
> + if (!P->map_inum) {
> + _dprintf("Out of memory initializing map_inum hash\n");
> + return -ENOMEM;
> + }
> return 0;
> }
>
> @@ -219,6 +258,8 @@ Psym_free(struct ps_prochandle *P)
> {
> free(P->map_files);
> P->map_files = NULL;
> + free(P->map_inum);
> + P->map_inum = NULL;
> }
>
> /*
> @@ -238,6 +279,25 @@ static prmap_file_t *Pprmap_file_by_name(struct ps_prochandle *P,
> return NULL;
> }
>
> +/*
> + * Given a process handle, find a corresponding prmap_file by (dev_t, inode
> + * number), or NULL if none.
> + */
> +static prmap_file_t *Pprmap_file_by_dev_inum(struct ps_prochandle *P,
> + dev_t dev, ino_t inum)
> +{
> + uint_t h = dev_inum_hash(dev, inum) % MAP_HASH_BUCKETS;
> +
> + prmap_file_t *prf;
> +
> + for (prf = P->map_inum[h]; prf != NULL; prf = prf->prf_inum_next)
> + if (prf->first_segment->pr_inum == inum &&
> + prf->first_segment->pr_dev == dev)
> + return prf;
> +
> + return NULL;
> +}
Why is this not simply called Pinode_to_map()? Since Pinode_to_map() is the
sole caller for Pprmap_file_by_dev_inum() *and* since it really just calls this
function, why not name this Pinode_to_map() and be done?
> +
> /*
> * Call-back function for librtld_db to iterate through all of its shared
> * libraries and determine their load object names and lmids.
> @@ -552,9 +612,13 @@ Pupdate_maps(struct ps_prochandle *P)
> goto err;
> pmptr = mptr->map_pmap;
> memset(pmptr, 0, sizeof(struct prmap));
> + pmptr->pr_dev = makedev(major, minor);
> + pmptr->pr_inum = inode;
>
> if ((prf = Pprmap_file_by_name(P, fn)) == NULL) {
> - uint_t h = string_hash(fn) % MAP_HASH_BUCKETS;
> + uint_t fn_h = string_hash(fn) % MAP_HASH_BUCKETS;
> + uint_t inum_h = dev_inum_hash(pmptr->pr_dev, pmptr->pr_inum) %
> + MAP_HASH_BUCKETS;
>
> if ((prf = malloc(sizeof(struct prmap_file))) == NULL) {
> free(mptr->map_pmap);
> @@ -563,10 +627,11 @@ Pupdate_maps(struct ps_prochandle *P)
>
> memset(prf, 0, sizeof(struct prmap_file));
> prf->prf_mapname = fn;
> - prf->prf_next = P->map_files[h];
> - P->map_files[h] = prf;
> - }
> - else {
> + prf->prf_next = P->map_files[fn_h];
> + P->map_files[fn_h] = prf;
> + prf->prf_inum_next = P->map_inum[inum_h];
> + P->map_inum[inum_h] = prf;
> + } else {
> free(fn);
> fn = NULL;
> }
> @@ -644,8 +709,6 @@ Pupdate_maps(struct ps_prochandle *P)
> (strcmp(prf->prf_mapname, exefile) == 0))
> P->map_exec = P->num_mappings;
> }
> - pmptr->pr_dev = makedev(major, minor);
> - pmptr->pr_inum = inode;
> pmptr->pr_file = prf;
>
> /*
> @@ -855,6 +918,12 @@ Pname_to_map(struct ps_prochandle *P, const char *name)
> return Plmid_to_map(P, PR_LMID_EVERY, name);
> }
>
> +const prmap_file_t *
> +Pinode_to_map(struct ps_prochandle *P, dev_t dev, ino_t inum)
> +{
> + return Pprmap_file_by_dev_inum(P, dev, inum);
> +}
> +
> /*
> * We wouldn't need these if qsort(3C) took an argument for the callback...
> */
> diff --git a/libproc/libproc.h b/libproc/libproc.h
> index 1f8683e25b31..420437ea841e 100644
> --- a/libproc/libproc.h
> +++ b/libproc/libproc.h
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2009, 2020, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> @@ -32,6 +32,7 @@
> #include <sys/socket.h>
> #include <sys/utsname.h>
> #include <sys/time.h>
> +#include <sys/types.h>
> #include <sys/user.h>
>
> #include <sys/compiler.h>
> @@ -242,6 +243,8 @@ extern const prmap_t *Paddr_to_map(struct ps_prochandle *, uintptr_t);
> extern const prmap_t *Pname_to_map(struct ps_prochandle *, const char *);
> extern const prmap_t *Plmid_to_map(struct ps_prochandle *,
> Lmid_t, const char *);
> +extern const prmap_file_t *Pinode_to_map(struct ps_prochandle *,
> + dev_t, ino_t);
>
> extern char *Pobjname(struct ps_prochandle *, uintptr_t, char *, size_t);
> extern int Plmid(struct ps_prochandle *, uintptr_t, Lmid_t *);
> --
> 2.37.1.265.g363c192786.dirty
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list