Skip to contentSkip to conversation

cgroup: Misc fixes

PATCH In review Andreas Herrmann 2 participants 7 messages Aug 27, 2020 – Aug 31, 2020

lore
Share & tools
lore

Andreas Herrmann

author
#0 lore raw

cover cgroup: Misc fixes

Hi,

I've tried fio's cgroup options with a recent kernel and found some issues with it -- following some patches to fix them.

Signature · 6 lines
Regards,
Andreas

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer (HRB 36809, AG Nürnberg)
author
#1 lore raw

PATCH 1/3 cgroup: Fix typo

Signed-off-by Andreas Herrmann <[email protected]>
---
 cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
cgroup.c +1 -1
index 77e31a4d..9ee1b8c8 100644
@@ -191,7 +191,7 @@ int cgroup_setup(struct thread_data *td, struct flist_head *clist, struct cgroup
 
 	if (td->o.cgroup_weight) {
 		if ((*mnt)->cgroup2) {
-			log_err("fio: cgroup weit doesn't work with cgroup2\n");
+			log_err("fio: cgroup weight doesn't work with cgroup2\n");
 			goto err;
 		}
 		if (write_int_to_file(td, root, "blkio.weight",
Signature · 1 line
2.28.0
author
#2 lore raw

PATCH 2/3 cgroup: Fix handling when cgroup blkio and cgroup-v2 are mounted

In reply to Andreas Herrmann

On systems that have a mixed setup of cgroup and cgroup-v2, blkio controller might be active. I think in this case there is no point in using cgroup-v2. (One needs to boot with cgroup_no_v1=blkio to be able to use cgroup-v2 IO controller.)

Signed-off-by Andreas Herrmann <[email protected]>
---
 cgroup.c | 58 ++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 18 deletions(-)
cgroup.c +40 -18
index 9ee1b8c8..43513733 100644
@@ -20,11 +20,10 @@ struct cgroup_member {
 
 static struct cgroup_mnt *find_cgroup_mnt(struct thread_data *td)
 {
-	struct cgroup_mnt *cgroup_mnt = NULL;
+	struct cgroup_mnt *cgroup_mnt, *cg1_mnt = NULL, *cg2_mnt = NULL;
 	struct mntent *mnt, dummy;
 	char buf[256] = {0};
 	FILE *f;
-	bool cgroup2 = false;
 
 	f = setmntent("/proc/mounts", "r");
 	if (!f) {

@@ -34,27 +33,50 @@ static struct cgroup_mnt *find_cgroup_mnt(struct thread_data *td)
 
 	while ((mnt = getmntent_r(f, &dummy, buf, sizeof(buf))) != NULL) {
 		if (!strcmp(mnt->mnt_type, "cgroup") &&
-		    strstr(mnt->mnt_opts, "blkio"))
-			break;
-		if (!strcmp(mnt->mnt_type, "cgroup2")) {
-			cgroup2 = true;
+			strstr(mnt->mnt_opts, "blkio")) {
+			cg1_mnt = smalloc(sizeof(*cg1_mnt));
+			if (cg1_mnt) {
+				cg1_mnt->path = smalloc_strdup(mnt->mnt_dir);
+				if (!cg1_mnt->path) {
+					sfree(cg1_mnt);
+					cg1_mnt = NULL;
+					log_err("fio: could not allocate memory\n");
+				}
+			}
+			/*
+			 * blkio controller has precedence
+			 */
 			break;
+		} if (!strcmp(mnt->mnt_type, "cgroup2")) {
+			if (cg2_mnt)
+				continue;
+			cg2_mnt = smalloc(sizeof(*cg2_mnt));
+			if (cg2_mnt) {
+				cg2_mnt->path = smalloc_strdup(mnt->mnt_dir);
+				if (!cg2_mnt->path) {
+					sfree(cg2_mnt);
+					cg2_mnt = NULL;
+					log_err("fio: could not allocate memory\n");
+				} else
+					cg2_mnt->cgroup2 = true;
+			}
+			/*
+			 * Check all mount points, there can be a mix
+			 * of cgroup-v1 and cgroup-v2
+			 */
+			continue;
 		}
 	}
 
-	if (mnt) {
-		cgroup_mnt = smalloc(sizeof(*cgroup_mnt));
-		if (cgroup_mnt) {
-			cgroup_mnt->path = smalloc_strdup(mnt->mnt_dir);
-			if (!cgroup_mnt->path) {
-				sfree(cgroup_mnt);
-				log_err("fio: could not allocate memory\n");
-			} else {
-				cgroup_mnt->cgroup2 = cgroup2;
-			}
-		}
+	cgroup_mnt = NULL;
+	if (cg1_mnt) {
+		cgroup_mnt = cg1_mnt;
+		if (cg2_mnt)
+			sfree(cg2_mnt);
+	} else if (cg2_mnt) {
+		cgroup_mnt = cg2_mnt;
 	} else {
-		log_err("fio: cgroup blkio does not appear to be mounted\n");
+		log_err("fio: cgroup blkio or cgroup2 do not appear to be mounted\n");
 	}
 
 	endmntent(f);
Signature · 1 line
2.28.0
author
#3 lore raw

PATCH 3/3 cgroup: Try to use blkio.bfq.weight if blkio.weight is not available

In reply to Andreas Herrmann

Newer kernels just provide blkio.bfq.weight for blkio contoller. Try to use this attribute when legacy file is not available.

Signed-off-by Andreas Herrmann <[email protected]>
---
 cgroup.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
cgroup.c +8 -2
index 43513733..6edb5b14 100644
@@ -218,8 +218,14 @@ int cgroup_setup(struct thread_data *td, struct flist_head *clist, struct cgroup
 		}
 		if (write_int_to_file(td, root, "blkio.weight",
 					td->o.cgroup_weight,
-					"cgroup open weight"))
-			goto err;
+					"cgroup open blkio.weight")) {
+			td_clear_error(td);
+			log_err("Trying to use blkio.bfq.weight\n");
+			if (write_int_to_file(td, root, "blkio.bfq.weight",
+						td->o.cgroup_weight,
+						"cgroup open blkio.bfq.weight"))
+				goto err;
+		}
 	}
 
 	if (!cgroup_write_pid(td, root, (*mnt)->cgroup2)) {
Signature · 1 line
2.28.0
#4 lore raw

Re: [PATCH 3/3] cgroup: Try to use blkio.bfq.weight if blkio.weight is not available

Quoted 14 lines · Andreas Herrmann

Newer kernels just provide blkio.bfq.weight for blkio contoller. Try to use this attribute when legacy file is not available.

Signed-off-by Andreas Herrmann <[email protected]>
---
 cgroup.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
cgroup.c +0 -0
index 43513733..6edb5b14 100644

cgroup.c int cgroup_setup(struct thread_data *td, struct flist_head *clist, struct cgroup · line 223

Quoted 8 lines
cgroup.c +3 -2
@@ -218,8 +218,14 @@ int cgroup_setup(struct thread_data *td, struct flist_head *clist, struct cgroup
                }
                if (write_int_to_file(td, root, "blkio.weight",
                                        td->o.cgroup_weight,
-                                       "cgroup open weight"))
-                       goto err;
+                                       "cgroup open blkio.weight")) {
+                       td_clear_error(td);
+                       log_err("Trying to use blkio.bfq.weight\n");

It doesn't feel right to print an error just because you tried a fallback. Maybe try both and print an error if neither works?

Quoted 11 lines
cgroup.c +5 -1
+                       if (write_int_to_file(td, root, "blkio.bfq.weight",
+                                               td->o.cgroup_weight,
+                                               "cgroup open blkio.bfq.weight"))
+                               goto err;
+               }
        }
 
        if (!cgroup_write_pid(td, root, (*mnt)->cgroup2)) {
--

2.28.0

Signature · 1 line
Sitsofe | http://sucs.org/~sits/
author
#5 lore raw

Re: [PATCH 3/3] cgroup: Try to use blkio.bfq.weight if blkio.weight is not available

Quoted 10 lines · Sitsofe Wheeler
Quoted 9 lines · Andreas Herrmann

Newer kernels just provide blkio.bfq.weight for blkio contoller. Try to use this attribute when legacy file is not available.

Signed-off-by Andreas Herrmann <[email protected]>
---
 cgroup.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
Quoted 16 lines

cgroup.c int cgroup_setup(struct thread_data *td, struct flist_head *clist, struct cgroup · line 223

Quoted 13 lines
cgroup.c +3 -2
index 43513733..6edb5b14 100644
@@ -218,8 +218,14 @@ int cgroup_setup(struct thread_data *td, struct flist_head *clist, struct cgroup
                }
                if (write_int_to_file(td, root, "blkio.weight",
                                        td->o.cgroup_weight,
-                                       "cgroup open weight"))
-                       goto err;
+                                       "cgroup open blkio.weight")) {
+                       td_clear_error(td);
+                       log_err("Trying to use blkio.bfq.weight\n");

It doesn't feel right to print an error just because you tried a fallback. Maybe try both and print an error if neither works?

Ok.
Most likely it's better to merge this patch into patch 2 of my second submission.

Thanks.

Quoted 16 lines
Quoted 11 lines
cgroup.c +5 -1
+                       if (write_int_to_file(td, root, "blkio.bfq.weight",
+                                               td->o.cgroup_weight,
+                                               "cgroup open blkio.bfq.weight"))
+                               goto err;
+               }
        }
 
        if (!cgroup_write_pid(td, root, (*mnt)->cgroup2)) {
--

2.28.0

Signature · 1 line
Sitsofe | http://sucs.org/~sits/
Signature · 6 lines
Regards,
Andreas

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer (HRB 36809, AG Nürnberg)
author
#6 lore raw
In reply to Andreas Herrmann
Quoted 4 lines · Andreas Herrmann

Hi,

I've tried fio's cgroup options with a recent kernel and found some issues with it -- following some patches to fix them.

Now I think patches #1 and #3 are superfluous or should be folded into patches of my 2nd submission. I'll rework my patches.

Signature · 6 lines
Regards,
Andreas

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer (HRB 36809, AG Nürnberg)