On Fri, May 8, 2020 at 7:46 AM Tom Hromatka <tom.hroma...@oracle.com> wrote:
> This commit adds cgroup v2 support to cgroup_init(),
> cg_test_mounted_fs(), and cgroup_get_cgroup(). With these
> changes, cgget and cgset now work on a cgroup v1 mount, a
> cgroup v2 mount, or a cgroup v2 unified mount hierarchy.
>
>
There is a lot of stuff happening in this patch.
Could you please split the patch slightly?
- prepwork introducing needed fields (Version field comes to mind straight
up)
- pre reqs needed for v2 support
- v2 support
I *think* the patch looks fine, but my 8am brain is slow :), the patch
split will make it easier to parse the review :)
> Signed-off-by: Tom Hromatka <tom.hroma...@oracle.com>
> ---
> src/api.c | 142 ++++++++++++++++++++++++++++++++++-----
> src/libcgroup-internal.h | 6 ++
> 2 files changed, 132 insertions(+), 16 deletions(-)
>
> diff --git a/src/api.c b/src/api.c
> index 3845352f07cf..0c05f851391a 100644
> --- a/src/api.c
> +++ b/src/api.c
> @@ -63,6 +63,12 @@ static __thread char errtext[MAXLEN];
> /* Task command name length */
> #define TASK_COMM_LEN 16
>
> +/* cgroup v2 controllers file */
> +#define CGV2_CONTROLLERS_FILE "cgroup.controllers"
> +
> +/* maximum line length when reading the cgroup.controllers file */
> +#define LL_MAX 100
> +
> /* Check if cgroup_init has been called or not. */
> static int cgroup_initialized;
>
> @@ -1086,6 +1092,7 @@ static int cgroup_process_v1_mnt(char
> *controllers[], struct mntent *ent,
> cg_mount_table[*mnt_tbl_idx].mount.path[FILENAME_MAX-1] =
> '\0';
> cg_mount_table[*mnt_tbl_idx].mount.next = NULL;
> + cg_mount_table[*mnt_tbl_idx].version = CGROUP_V1;
>
> cgroup_dbg("Found cgroup option %s, count %d\n",
> ent->mnt_opts, *mnt_tbl_idx);
> (*mnt_tbl_idx)++;
> @@ -1147,6 +1154,96 @@ out:
> return ret;
> }
>
> +/**
> + * Process a cgroup v2 mount and add it to cg_mount_table if it's not a
> + * duplicate.
> + *
> + * @param ent File system description of cgroup mount being processed
> + * @param mnt_tbl_idx cg_mount_table index
> + */
> +static int cgroup_process_v2_mnt(struct mntent *ent, int *mnt_tbl_idx)
> +{
> + char cgroup_controllers_path[FILENAME_MAX];
> + char *ret_c = NULL, line[LL_MAX], *stok_buff = NULL;
> + int ret = 0, i, duplicate;
> + FILE *fp = NULL;
> +
> + /* determine what v2 controllers are available on this mount */
> + snprintf(cgroup_controllers_path, FILENAME_MAX, "%s/%s",
> ent->mnt_dir,
> + CGV2_CONTROLLERS_FILE);
> +
> + fp = fopen(cgroup_controllers_path, "re");
> + if (!fp) {
> + ret = ECGOTHER;
> + goto out;
> + }
> +
> + ret_c = fgets(line, LL_MAX, fp);
> + if (ret_c == NULL) {
> + ret = ECGEOF;
> + goto out;
> + }
> +
> + /* remove the trailing newline */
> + ret_c[strlen(ret_c) - 1] = '\0';
> +
> + /*
> + * cgroup.controllers returns a list of available controllers in
> + * the following format:
> + * cpuset cpu io memory pids rdma
> + */
> + stok_buff = strtok(ret_c, " ");
> + while (stok_buff) {
> + /* do not have duplicates in mount table */
> + duplicate = 0;
> +
> + for (i = 0; i < *mnt_tbl_idx; i++) {
> + if (strncmp(cg_mount_table[i].name, stok_buff,
> + FILENAME_MAX) == 0) {
> + duplicate = 1;
> + break;
> + }
> + }
> +
> + if (duplicate) {
> + cgroup_dbg("controller %s is already mounted on
> %s\n",
> + stok_buff, cg_mount_table[i].mount.path);
> +
> + ret = cg_add_duplicate_mount(&cg_mount_table[i],
> + ent->mnt_dir);
> + if (ret)
> + goto out;
> +
> + /* advance to the next controller */
> + stok_buff = strtok(NULL, " ");
> + continue;
> + }
> +
> + /* this controller is not in the mount table. add it */
> + strncpy(cg_mount_table[*mnt_tbl_idx].name,
> + stok_buff, FILENAME_MAX);
> + cg_mount_table[*mnt_tbl_idx].name[FILENAME_MAX-1] = '\0';
> + strncpy(cg_mount_table[*mnt_tbl_idx].mount.path,
> + ent->mnt_dir, FILENAME_MAX);
> + cg_mount_table[*mnt_tbl_idx].mount.path[FILENAME_MAX-1] =
> + '\0';
> + cg_mount_table[*mnt_tbl_idx].mount.next = NULL;
> + cg_mount_table[*mnt_tbl_idx].version = CGROUP_V2;
> + cgroup_dbg("Found cgroup option %s, count %d\n",
> + stok_buff, *mnt_tbl_idx);
> + (*mnt_tbl_idx)++;
> +
> + /* advance to the next controller */
> + stok_buff = strtok(NULL, " ");
> + }
> +
> +out:
> + if (fp)
> + fclose(fp);
> +
> + return ret;
> +}
> +
> /**
> * cgroup_init(), initializes the MOUNT_POINT.
> *
> @@ -1255,6 +1352,16 @@ int cgroup_init(void)
> if (ret)
> goto unlock_exit;
> }
> + else if (strcmp(ent->mnt_type, "cgroup2") == 0) {
> + ret = cgroup_process_v2_mnt(ent, &found_mnt);
> + if (ret == ECGEOF)
> + /* The controllers file was empty. Ignore
> and
> + * move on.
> + */
> + continue;
> + else if (ret)
> + goto unlock_exit;
> + }
> }
>
> free(temp_ent);
> @@ -1314,7 +1421,8 @@ static int cg_test_mounted_fs(void)
> goto done;
> }
>
> - while (strcmp(ent->mnt_type, "cgroup") != 0) {
> + while (strcmp(ent->mnt_type, "cgroup") != 0 &&
> + strcmp(ent->mnt_type, "cgroup2") != 0) {
> ent = getmntent_r(proc_mount, temp_ent, mntent_buff,
> sizeof(mntent_buff));
> if (ent == NULL) {
> @@ -2687,25 +2795,27 @@ int cgroup_get_cgroup(struct cgroup *cgroup)
> * Get the uid and gid information
> */
>
> - ret = asprintf(&control_path, "%s/tasks", path);
> + if (cg_mount_table[i].version == CGROUP_V1) {
> + ret = asprintf(&control_path, "%s/tasks", path);
>
> - if (ret < 0) {
> - last_errno = errno;
> - error = ECGOTHER;
> - goto unlock_error;
> - }
> + if (ret < 0) {
> + last_errno = errno;
> + error = ECGOTHER;
> + goto unlock_error;
> + }
>
> - if (stat(control_path, &stat_buffer)) {
> - last_errno = errno;
> - free(control_path);
> - error = ECGOTHER;
> - goto unlock_error;
> - }
> + if (stat(control_path, &stat_buffer)) {
> + last_errno = errno;
> + free(control_path);
> + error = ECGOTHER;
> + goto unlock_error;
> + }
>
> - cgroup->tasks_uid = stat_buffer.st_uid;
> - cgroup->tasks_gid = stat_buffer.st_gid;
> + cgroup->tasks_uid = stat_buffer.st_uid;
> + cgroup->tasks_gid = stat_buffer.st_gid;
>
> - free(control_path);
> + free(control_path);
> + }
>
> cgc = cgroup_add_controller(cgroup,
> cg_mount_table[i].name);
> diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
> index e31df512fcbe..dc292c661842 100644
> --- a/src/libcgroup-internal.h
> +++ b/src/libcgroup-internal.h
> @@ -112,6 +112,11 @@ struct cg_mount_point {
> struct cg_mount_point *next;
> };
>
> +enum cg_version_t {
> + CGROUP_V1,
> + CGROUP_V2,
> +};
> +
> struct cg_mount_table_s {
> /** Controller name. */
> char name[FILENAME_MAX];
> @@ -120,6 +125,7 @@ struct cg_mount_table_s {
> */
> struct cg_mount_point mount;
> int index;
> + enum cg_version_t version;
> };
>
> struct cgroup_rules_data {
> --
> 2.25.3
>
>
>
> _______________________________________________
> Libcg-devel mailing list
> Libcg-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/libcg-devel
>
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
