[libresource-devel] [PATCH] library of interfaces to fetch system resource information

Jonathan Helman jonathan.helman at oracle.com
Thu Mar 1 19:55:30 UTC 2018


Hi Rahul,

I have a few questions and comments below on the resource.h API and 
README. Did not check out the implementation yet.

Jon

On 02/28/2018 10:58 PM, Rahul Yadav wrote:
> 1: System resource Information related to  memory, CPU, stat, networking,
> security etc.
> 2: Currently most of such information is read from /proc and /sys.
> 3: Add basic infrastructure and code for some of memory and network related
> information.
> 4: Update README file with the use cases and design details.
> 5: Added example codes in README.
> 
> Signed-off-by: Rahul Yadav <rahul.x.yadav at oracle.com>
> ---
>   Makefile        |  18 ++++
>   README.md       | 160 ++++++++++++++++++++++++++--
>   resmem.c        | 218 ++++++++++++++++++++++++++++++++++++++
>   resmem.h        |  16 +++
>   resnet.c        | 316 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   resnet.h        |  14 +++
>   resource.c      | 211 +++++++++++++++++++++++++++++++++++++
>   resource.h      | 147 ++++++++++++++++++++++++++
>   resource_impl.h |  47 +++++++++
>   9 files changed, 1137 insertions(+), 10 deletions(-)
>   create mode 100644 Makefile
>   create mode 100644 resmem.c
>   create mode 100644 resmem.h
>   create mode 100644 resnet.c
>   create mode 100644 resnet.h
>   create mode 100644 resource.c
>   create mode 100644 resource.h
>   create mode 100644 resource_impl.h
> 
> diff --git a/Makefile b/Makefile
> new file mode 100644
> index 0000000..2c71328
> --- /dev/null
> +++ b/Makefile
> @@ -0,0 +1,18 @@
> +CC = gcc
> +CFLAGS = -g -Wall -Werror -fPIC
> +DEPS = resource.h res_impl.h resmem.h resnet.h
> +OBJ = resource.o resmem.o resnet.o
> +LIB = libresource.so
> +TEST = test
> +RM = rm -rf
> +CP = cp
> +
> +%.o: %.c $(DEPS)
> +	$(CC) -c -o $@ $< $(CFLAGS)
> +
> +all: $(OBJ)
> +	$(CC) -shared -o $(LIB) $^ $(CFLAGS)
> +
> +.PHONY : clean
> +clean:
> +	$(RM) $(LIB) $(OBJ) $(TEST)
> diff --git a/README.md b/README.md
> index 960468d..97b37df 100644
> --- a/README.md
> +++ b/README.md
> @@ -1,14 +1,154 @@
>   # libresource
> +library of interfaces through which we can get system resource information
> +like memory, CPU, stat, networking, device etc.
> +Currently most of such information is read from /proc and /sys.
>   
> -libresource is a community effort to generate a new library which tools
> -like ps, top, etc can use to get information typically garnered from
> -/proc.  libresource however will attempt to virtualize the data according
> -to the calling task's cgroup information.
> +Use case:
> +1: Ease of use -
> +Currently applications and tools need to read this info mostly from
> +/proc and /sys file-systems. In most of the cases complex string
> +parsing is involved which is needed to be done in application
> +code. With the library interfaces application can get the information
> +directly and all the string parsing, if any, will be done by library.
> +2: Stability -
> +If the format in which the information is provided in /proc or /sys
> +file-system is changed then the application code is changed to align
> +with those changes.

What is the method by which you do this? Is there a chance the library 
breaks when /proc or /sys is updated, until libresource is updated?

  Also if a better way to get information comes in
> +future, like through a syscall or a sysconf then again application code
> +needs to be changed to get the benefit of it. Library will take care of
> +such changes.
> +3: Virtualization -
> +In cases where DB is running in a virtualized environment using
> +cgroup or namespaces, reading from /proc and /sys file-systems
> +might not give correct information as these are not cgroup
> +aware. Library API will take care of this e.g. if a process
> +is running in a cgroup then library should provide information
> +which is local to that cgroup.
>   
> -# contributing
> +Design:
> +Each resource is identified by a resource id. User land application should
> +provide the id while calling the interfaces to fetch the information.
> +Following are resource IDs which are already implemented.
>   
> -We will interact through pull requests and the issue tracker.  I think we
> -should start with pull requests editing the CONTRIBUTORS so we can have a
> -feel whether enough people are able to contribute or interested in following
> -for the project to be successful.  We can also start pull requests against
> -the specs/api.md file to discuss what that should look like.
> +RES_MEM_ACTIVE
> +RES_MEM_INACTIVE
> +RES_MEM_AVAILABLE
> +RES_MEM_FREE
> +RES_MEM_TOTAL
> +RES_MEM_PAGESIZE
> +RES_MEM_SWAPFREE
> +RES_MEM_SWAPTOTAL
> +RES_KERN_COMPILE_TIME
> +RES_KERN_RELEASE
> +RES_NET_ALLIFSTAT
> +RES_NET_IFSTAT
> +RES_MEM_INFOALL
> +
> +Interfaces:
> +1:
> +int res_read(int res_id, void *out, void *hint, int pid, int flags); > +
> +This is to read a resource information. A valid resource id should be provided
> +in res_id, out should be properly allocated on the basis of size of resource
> +information, hint should be given where needed. Currently pid and flags are
> +not used, they are for future extensions.
> +
> +2:
> +
> +If an application wants to read multiple resource information in one call, it
> +can call res_*_blk APIs to do so.
> +
> +2.1 structs
> +Following struct holds information about one resource in such case.
> +typedef struct res_unit {
> +        int status;
> +        int res_id;
> +        void *hint;
> +        union r_data data;
> +} res_unit_t;
> +
> +An array of these form res_blk_t structure as below.
> +typedef struct res_blk {
> +        int res_count;
> +        res_unit_t *res_unit[0];
> +} res_blk_t;
> +
> +res_blk_t strcut is used in all res_*_blk interfaces.

s/strcut/struct

> +
> +2.2 functions
> +res_blk_t *res_build_blk(int *res_ids, int res_count);
> +
> +This allocates memory for resources and initiates them properly. res_ids
> +holds and array of valid resource ids and res_count holds number of
> +resource ids. It also initiates struct fields properly.
> +
> +int res_read_blk(res_blk_t *resblk, int pid, int flags);
> +Reading bulk resource information. Memory must be properly allocated and
> +all fields should be properly filled to return error free resource
> +information. res_build_blk call is suggested to allocate build res_blk_t
> +structure.

It seems that flags, since for future extensions, would be per res_unit 
(like the hint). Could you explain why flags is per res_blk?

> +
> +/* Free allocated memory from res_build_blk */
> +extern void res_destroy_blk(res_blk_t *resblk);
> +
> +Example
> +1:
> +Reading total memory
> +size_t stemp = 0;
> +res_read(RES_MEM_TOTAL,&stemp,NULL, 0, 0);
> +printf("MEMTOTAL is: %zu\n", stemp);
> +
> +
> +2:
> +reading network interface related statistics for interface named "lo"
> +res_net_ifstat_t ifstat;
> +res_read(RES_NET_IFSTAT,&ifstat, (void *)"lo",0, 0);
> +printf("status for %s: %llu %llu\n", ifstat.ifname,
> +	ifstat.rx_bytes,
> +	ifstat.rx_packets
> +	);
> +3:
> +Reading multiple resouce information in one call.

s/resouce/resource

> +
> +    res_blk_t *b = NULL;
> +    int a[NUM] = {RES_MEM_PAGESIZE,
> +                RES_MEM_TOTAL,
> +                RES_MEM_AVAILABLE,
> +                RES_MEM_INFOALL,
> +                RES_KERN_RELEASE,
> +                RES_NET_IFSTAT,
> +                RES_NET_ALLIFSTAT,
> +                RES_KERN_COMPILE_TIME
> +                };
> +    b = res_build_blk(a, NUM);
> +    b->res_unit[5]->hint = (void *)"lo";
> +
> +    res_read_blk(b, 0, 0);
> +
> +    printf("pagesize %ld bytes,\n memtotal %ld kb,\n memavailable %ld kb,\n"
> +            " mefree %ld kb,\n release %s,\n compile time %s\n",

s/mefree/memfree

> +            b->res_unit[0]->data.sz,
> +            b->res_unit[1]->data.sz,
> +            b->res_unit[2]->data.sz,
> +            ((res_mem_infoall_t *)(b->res_unit[3]->data.ptr))->memfree,
> +            b->res_unit[4]->data.str,
> +            b->res_unit[7]->data.str
> +    );
> +
> +    res_net_ifstat_t *ip = (res_net_ifstat_t *)b->res_unit[5]->data.ptr;
> +    printf("stat for interface %s: %llu %llu\n", ip->ifname,
> +        ip->rx_bytes,
> +        ip->rx_packets
> +        );
> +
> +    int k = (int)(long long)b->res_unit[6]->hint;
> +    res_net_ifstat_t *ipp = (res_net_ifstat_t *)b->res_unit[6]->data.ptr;
> +    for (int j=0; j< k; j++) {
> +        printf("stat for interface %s: %llu %llu\n", ipp[j].ifname,
> +            ipp[j].rx_bytes,
> +            ipp[j].rx_packets
> +        );
> +    }
> +
> +    free(ipp);

What is ipp?

> +    res_destroy_blk(b);


> <snip>


> diff --git a/resource.h b/resource.h
> new file mode 100644
> index 0000000..963877d
> --- /dev/null
> +++ b/resource.h
> @@ -0,0 +1,147 @@
> +#ifndef	_RESOURCE_H
> +#define	_RESOURCE_H
> +
> +#include <stdio.h>

What is this file included for here?

> +#include <net/if.h>
> +
> +/* libresource version */
> +#define LIBRESOURCE_API_VERSION 1
> +
> +/* Possible status for libresource information returned */
> +/* libresource information was fetched correctly */
> +#define RES_STATUS_FILLED 0
> +/* There was some error in fetching libresource information. In most cases
> + * errno will be set.
> + */
> +#define RES_STATUS_EMPTY -1
> +/* Resource information is not supported yet, or Invalid resource
> + * information.

Seems that notsupported should be differentiated from invalid resource info.

> + */
> +#define RES_STATUS_NOTSUPPORTED	-2
> +/* If partial information was read for a libresource information. For example
> + * a string was read partially.
> + */
> +#define RES_STATUS_TRUNCATED -3

How do you know which information is valid? Is this if you want multiple 
units in a block and only some of the units are filled?

> +
> +
> +/* Maximum size of a resource information data type which can be returned
> + * without explicitly allocating memory for it. If resource information
> + * size is larger than this this a pointer to allocated memory will be
> + * returned.
> + */
> +#define RES_UNIT_OUT_SIZE	256
> +
> +/* This union is used to return resource information of various types */
> +union r_data {
> +	int i;
> +	size_t sz;
> +	long l;
> +	char str[RES_UNIT_OUT_SIZE];
> +	void *ptr;
> +};

Hmm, this is a confusing idea, IMO. Maybe everything is in void *ptr? 
And then this union could be collapsed.

> +
> +/* In case of res_read_blk, each resource information will be represented by
> + * following structure.
> + */
> +typedef struct res_unit {
> +	int status;
> +	int res_id;
> +	void *hint;
> +	union r_data data;
> +} res_unit_t;
> +
> +/* In case of bulk read (res_read_blk), this structure will hold all required
> + * information needed to do so.
> + */
> +typedef struct res_blk {
> +	int res_count;
> +	res_unit_t *res_unit[0];
> +} res_blk_t;
> +
> +/* Resource information is divided in broad categories and each
> + * category is assigned a number range for its resource information
> + * Memory related			(RES_MEM_*)		1024-
> + * Network related			(RES_NET_*)		2048-
> + * General kernel related		(RES_KERN_*)		3072-
> + * This is done to facilitate any future optimization which can be made
> + * on the basis of resource information (hashing etc ?)

Should these be u64 instead of int then?

> + */
> +#define MEM_MIN				1024
> +#define RES_MEM_HUGEPAGEALL		1025
> +#define RES_MEM_HUGEPAGESIZE		1026
> +#define RES_MEM_INACTIVE		1027
> +#define RES_MEM_INFOALL			1028
> +#define RES_MEM_AVAILABLE		1029
> +#define RES_MEM_FREE			1030
> +#define RES_MEM_TOTAL			1031
> +#define RES_MEM_PAGESIZE		1032
> +#define RES_MEM_SHMALL			1033
> +#define RES_MEM_SHMINFOALL		1034
> +#define RES_MEM_SHMMAX			1035
> +#define RES_MEM_SHMMNI			1036

What is MNI?

> +#define RES_MEM_SWAPFREE		1037
> +#define RES_MEM_SWAPTOTAL		1038
> +#define RES_MEM_ACTIVE			1039
> +#define MEM_MAX				1040
> +
> +#define NET_MIN				2048
> +#define RES_NET_IFSTAT			2049
> +#define RES_NET_ALLIFSTAT		2050
> +#define NET_MAX				2051
> +
> +#define KERN_MIN			3072
> +#define RES_KERN_COMPILE_TIME		3073
> +#define RES_KERN_RELEASE		3074
> +#define KERN_MAX			3075
> +
> +/* Structure to return RES_MEM_INFOALL resource information */
> +typedef struct res_mem_infoall {
> +	size_t memfree;
> +	size_t memtotal;
> +	size_t memavailable;
> +	size_t active;
> +	size_t inactive;
> +	size_t swaptotal;
> +	size_t swapfree;
> +} res_mem_infoall_t;
> +
> +/* Structure to return RES_MEM_ALLIFSTAT resource information */
> +typedef struct res_net_ifstat {
> +	char ifname[IFNAMSIZ];
> +	unsigned long long rx_bytes;
> +	unsigned long long rx_packets;
> +	unsigned long rx_errors;
> +	unsigned long rx_dropped;
> +	unsigned long rx_fifo_err;
> +	unsigned long rx_frame_err;
> +	unsigned long rx_compressed;
> +	unsigned long rx_multicast;
> +	unsigned long long tx_bytes;
> +	unsigned long long tx_packets;
> +	unsigned long tx_errors;
> +	unsigned long tx_dropped;
> +	unsigned long tx_fifo_err;
> +	unsigned long tx_collisions;
> +	unsigned long tx_carrier_err;
> +	unsigned long tx_compressed;
> +} res_net_ifstat_t;

Maybe this struct name should have the word all in it to be consistent 
with struct res_mem_infoall?

> +
> +/* Allocating memory and building a res_blk structure to return bulk
> + * resource information.
> + */
> +extern res_blk_t *res_build_blk(int *res_ids, int res_count);
> +
> +/* Reading bulk resource information. Memory must be properly allocated and
> + * all fields should be properly filled to return error free resource
> + * information. res_build_blk call is suggested to allocate build res_blk_t

s/allocate build/allocate and build

> + * structure.
> + */
> +extern int res_read_blk(res_blk_t *resblk, int pid, int flags);
> +
> +/* Free allocated memory from res_build_blk */
> +extern void res_destroy_blk(res_blk_t *resblk);
> +
> +/* Read a resource information. Memory for out should be properly allocated */
> +extern int res_read(int res_id, void *out, void *hint, int pid, int flags);
> +
> +#endif /* RESOURCE_H */
> 


More information about the libresource-devel mailing list