All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota
@ 2021-08-31  9:00 Joerg Vehlow
  2021-08-31  9:00 ` [LTP] [PATCH 2/2] cfs_bandwidth01: Add misssing needs_kconfigs Joerg Vehlow
  2021-08-31  9:18 ` [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota Richard Palethorpe
  0 siblings, 2 replies; 6+ messages in thread
From: Joerg Vehlow @ 2021-08-31  9:00 UTC (permalink / raw)
  To: ltp

From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

If set_cpu_quota failed, mk_cpu_cgroup did not return
and cg_workers[n] was not set. This lead to a failure during
cleanup, because the worker cgroups were not deleted.

Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
---
 .../sched/cfs-scheduler/cfs_bandwidth01.c       | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
index 9301ee458..e8032d65a 100644
--- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
+++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
@@ -57,17 +57,14 @@ static void set_cpu_quota(const struct tst_cgroup_group *const cg,
 		tst_cgroup_group_name(cg), quota_us, period_us);
 }
 
-static struct tst_cgroup_group *
-mk_cpu_cgroup(const struct tst_cgroup_group *const cg_parent,
+static void mk_cpu_cgroup(struct tst_cgroup_group **cg,
+          const struct tst_cgroup_group *const cg_parent,
 	      const char *const cg_child_name,
 	      const float quota_percent)
 {
-	struct tst_cgroup_group *const cg =
-		tst_cgroup_group_mk(cg_parent, cg_child_name);
+	*cg = tst_cgroup_group_mk(cg_parent, cg_child_name);
 
-	set_cpu_quota(cg, quota_percent);
-
-	return cg;
+	set_cpu_quota(*cg, quota_percent);
 }
 
 static void busy_loop(const unsigned int sleep_ms)
@@ -142,11 +139,11 @@ static void setup(void)
 	cg_level2 = tst_cgroup_group_mk(cg_test, "level2");
 
 	cg_level3a = tst_cgroup_group_mk(cg_level2, "level3a");
-	cg_workers[0] = mk_cpu_cgroup(cg_level3a, "worker1", 30);
-	cg_workers[1] = mk_cpu_cgroup(cg_level3a, "worker2", 20);
+	mk_cpu_cgroup(&cg_workers[0], cg_level3a, "worker1", 30);
+	mk_cpu_cgroup(&cg_workers[1], cg_level3a, "worker2", 20);
 
 	cg_level3b = tst_cgroup_group_mk(cg_level2, "level3b");
-	cg_workers[2] = mk_cpu_cgroup(cg_level3b, "worker3", 30);
+	mk_cpu_cgroup(&cg_workers[2], cg_level3b, "worker3", 30);
 }
 
 static void cleanup(void)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] cfs_bandwidth01: Add misssing needs_kconfigs
  2021-08-31  9:00 [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota Joerg Vehlow
@ 2021-08-31  9:00 ` Joerg Vehlow
  2021-08-31  9:19   ` Richard Palethorpe
  2021-08-31  9:18 ` [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota Richard Palethorpe
  1 sibling, 1 reply; 6+ messages in thread
From: Joerg Vehlow @ 2021-08-31  9:00 UTC (permalink / raw)
  To: ltp

From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

The test requires CONFIG_CFS_BANDWIDTH, otherwise it fails with
cfs_bandwidth01.c:51: TBROK: openat(13</sys/fs/cgroup/cpu,cpuacct/ltp/test-5666/level2/level3a/worker1>, 'cpu.cfs_period_us', O_WRONLY): ENOENT (2)

Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
---
 testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
index e8032d65a..001fb2685 100644
--- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
+++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
@@ -178,6 +178,10 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.needs_checkpoints = 1,
 	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_CFS_BANDWIDTH",
+		NULL
+	},
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "39f23ce07b93"},
 		{"linux-git", "b34cb07dde7c"},
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota
  2021-08-31  9:00 [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota Joerg Vehlow
  2021-08-31  9:00 ` [LTP] [PATCH 2/2] cfs_bandwidth01: Add misssing needs_kconfigs Joerg Vehlow
@ 2021-08-31  9:18 ` Richard Palethorpe
  2021-08-31  9:26   ` Joerg Vehlow
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Palethorpe @ 2021-08-31  9:18 UTC (permalink / raw)
  To: ltp

Hello Joerg,

Joerg Vehlow <lkml@jv-coder.de> writes:

> From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>
> If set_cpu_quota failed, mk_cpu_cgroup did not return
> and cg_workers[n] was not set. This lead to a failure during
> cleanup, because the worker cgroups were not deleted.
>
> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
> ---
>  .../sched/cfs-scheduler/cfs_bandwidth01.c       | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> index 9301ee458..e8032d65a 100644
> --- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> +++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> @@ -57,17 +57,14 @@ static void set_cpu_quota(const struct tst_cgroup_group *const cg,
>  		tst_cgroup_group_name(cg), quota_us, period_us);
>  }
>  
> -static struct tst_cgroup_group *
> -mk_cpu_cgroup(const struct tst_cgroup_group *const cg_parent,
> +static void mk_cpu_cgroup(struct tst_cgroup_group **cg,
> +          const struct tst_cgroup_group *const cg_parent,

Seems like there are spaces instead of a tab at the start.

Otherwise LGTM, thanks.

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2] cfs_bandwidth01: Add misssing needs_kconfigs
  2021-08-31  9:00 ` [LTP] [PATCH 2/2] cfs_bandwidth01: Add misssing needs_kconfigs Joerg Vehlow
@ 2021-08-31  9:19   ` Richard Palethorpe
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Palethorpe @ 2021-08-31  9:19 UTC (permalink / raw)
  To: ltp

Hi,

LGTM, thanks.

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota
  2021-08-31  9:18 ` [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota Richard Palethorpe
@ 2021-08-31  9:26   ` Joerg Vehlow
  2021-08-31 13:24     ` Li Wang
  0 siblings, 1 reply; 6+ messages in thread
From: Joerg Vehlow @ 2021-08-31  9:26 UTC (permalink / raw)
  To: ltp

Hi Richard,

On 8/31/2021 11:18 AM, Richard Palethorpe wrote:
> Hello Joerg,
>
> Joerg Vehlow <lkml@jv-coder.de> writes:
>
>> From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>>
>> If set_cpu_quota failed, mk_cpu_cgroup did not return
>> and cg_workers[n] was not set. This lead to a failure during
>> cleanup, because the worker cgroups were not deleted.
>>
>> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>> ---
>>   .../sched/cfs-scheduler/cfs_bandwidth01.c       | 17 +++++++----------
>>   1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> index 9301ee458..e8032d65a 100644
>> --- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> +++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> @@ -57,17 +57,14 @@ static void set_cpu_quota(const struct tst_cgroup_group *const cg,
>>   		tst_cgroup_group_name(cg), quota_us, period_us);
>>   }
>>   
>> -static struct tst_cgroup_group *
>> -mk_cpu_cgroup(const struct tst_cgroup_group *const cg_parent,
>> +static void mk_cpu_cgroup(struct tst_cgroup_group **cg,
>> +          const struct tst_cgroup_group *const cg_parent,
> Seems like there are spaces instead of a tab at the start.
Right, I wanted to keep the same format you had, but did not realize it 
was one tab and 5 spaces instead of 9 spaces.
I guess this can be fixed during merging.

>
> Otherwise LGTM, thanks.
>
> Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
Thanks

Joerg


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota
  2021-08-31  9:26   ` Joerg Vehlow
@ 2021-08-31 13:24     ` Li Wang
  0 siblings, 0 replies; 6+ messages in thread
From: Li Wang @ 2021-08-31 13:24 UTC (permalink / raw)
  To: ltp

Hi Joerg, Richard,



> >> -static struct tst_cgroup_group *
> >> -mk_cpu_cgroup(const struct tst_cgroup_group *const cg_parent,
> >> +static void mk_cpu_cgroup(struct tst_cgroup_group **cg,
> >> +          const struct tst_cgroup_group *const cg_parent,
> > Seems like there are spaces instead of a tab at the start.
>


> Right, I wanted to keep the same format you had, but did not realize it
> was one tab and 5 spaces instead of 9 spaces.
> I guess this can be fixed during merging.
>

I fixed that and pushed those patches. Thanks!

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210831/33d6120f/attachment-0001.htm>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-31 13:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31  9:00 [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota Joerg Vehlow
2021-08-31  9:00 ` [LTP] [PATCH 2/2] cfs_bandwidth01: Add misssing needs_kconfigs Joerg Vehlow
2021-08-31  9:19   ` Richard Palethorpe
2021-08-31  9:18 ` [LTP] [PATCH 1/2] cfs_bandwidth01: Fix cleanup on failure in set_cpu_quota Richard Palethorpe
2021-08-31  9:26   ` Joerg Vehlow
2021-08-31 13:24     ` Li Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.