[PATCH 0/3] cgroup: Misc fixes

[PATCH 0/3] cgroup: Misc fixes

[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.

-- 
Regards,
Andreas

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

[PATCH 1/3] cgroup: Fix typo

[Andreas Herrmann]

Signed-off-by: Andreas Herrmann <aherrmann@suse.com>
---
 cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cgroup.c b/cgroup.c
index 77e31a4d..9ee1b8c8 100644
--- a/cgroup.c
+++ b/cgroup.c
@@ -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",
-- 
2.28.0

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

[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 <aherrmann@suse.com>
---
 cgroup.c | 58 ++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 18 deletions(-)

diff --git a/cgroup.c b/cgroup.c
index 9ee1b8c8..43513733 100644
--- a/cgroup.c
+++ b/cgroup.c
@@ -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);
-- 
2.28.0

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

[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 <aherrmann@suse.com>
---
 cgroup.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/cgroup.c b/cgroup.c
index 43513733..6edb5b14 100644
--- a/cgroup.c
+++ b/cgroup.c
@@ -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)) {
-- 
2.28.0

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

[Sitsofe Wheeler]

On Thu, 27 Aug 2020 at 13:26, Andreas Herrmann <aherrmann@suse.com> wrote:
>
> 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 <aherrmann@suse.com>
> ---
>  cgroup.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/cgroup.c b/cgroup.c
> index 43513733..6edb5b14 100644
> --- a/cgroup.c
> +++ b/cgroup.c
> @@ -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?

> +                       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
>


-- 
Sitsofe | http://sucs.org/~sits/

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

[Andreas Herrmann]

On Fri, Aug 28, 2020 at 06:29:38PM +0100, Sitsofe Wheeler wrote:
> On Thu, 27 Aug 2020 at 13:26, Andreas Herrmann <aherrmann@suse.com> wrote:
> >
> > 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 <aherrmann@suse.com>
> > ---
> >  cgroup.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/cgroup.c b/cgroup.c
> > index 43513733..6edb5b14 100644
> > --- a/cgroup.c
> > +++ b/cgroup.c
> > @@ -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.

> > +                       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
> >
> 
> 
> -- 
> Sitsofe | http://sucs.org/~sits/
> 

-- 
Regards,
Andreas

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

Re: [PATCH 0/3] cgroup: Misc fixes

[Andreas Herrmann]

On Thu, Aug 27, 2020 at 02:07:25PM +0200, Andreas Herrmann wrote:
> 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.

-- 
Regards,
Andreas

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