These three patches are about PSI.
patch 1: Fixed PSI statistics error caused by unzeroed memory
in struct psi_group.
patch 2 and patch 3 are to clean up some unused functions
and parameters.
Hao Jia (3):
sched/psi: Zero the memory of struct psi_group
sched/psi: Remove unused parameter nbytes of psi_trigger_create()
sched/psi: Remove redundant cgroup_psi() when !CONFIG_CGROUPS
include/linux/cgroup.h | 5 -----
include/linux/psi.h | 2 +-
kernel/cgroup/cgroup.c | 2 +-
kernel/sched/psi.c | 10 +++-------
4 files changed, 5 insertions(+), 14 deletions(-)
--
2.32.0
cgroup_psi() is only called under CONFIG_CGROUPS.
We don't need cgroup_psi() when !CONFIG_CGROUPS,
so we can remove it in this case.
Signed-off-by: Hao Jia <[email protected]>
---
include/linux/cgroup.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index ed53bfe7c46c..ac5d0515680e 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -734,11 +734,6 @@ static inline struct cgroup *cgroup_parent(struct cgroup *cgrp)
return NULL;
}
-static inline struct psi_group *cgroup_psi(struct cgroup *cgrp)
-{
- return NULL;
-}
-
static inline bool cgroup_psi_enabled(void)
{
return false;
--
2.32.0
psi_trigger_create()'s 'nbytes' parameter is not used, so we can remove it.
Signed-off-by: Hao Jia <[email protected]>
---
include/linux/psi.h | 2 +-
kernel/cgroup/cgroup.c | 2 +-
kernel/sched/psi.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/psi.h b/include/linux/psi.h
index 89784763d19e..dd74411ac21d 100644
--- a/include/linux/psi.h
+++ b/include/linux/psi.h
@@ -27,7 +27,7 @@ void psi_memstall_leave(unsigned long *flags);
int psi_show(struct seq_file *s, struct psi_group *group, enum psi_res res);
struct psi_trigger *psi_trigger_create(struct psi_group *group,
- char *buf, size_t nbytes, enum psi_res res);
+ char *buf, enum psi_res res);
void psi_trigger_destroy(struct psi_trigger *t);
__poll_t psi_trigger_poll(void **trigger_ptr, struct file *file,
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index ffaccd6373f1..df7df5843b4f 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3698,7 +3698,7 @@ static ssize_t cgroup_pressure_write(struct kernfs_open_file *of, char *buf,
}
psi = cgroup_ino(cgrp) == 1 ? &psi_system : cgrp->psi;
- new = psi_trigger_create(psi, buf, nbytes, res);
+ new = psi_trigger_create(psi, buf, res);
if (IS_ERR(new)) {
cgroup_put(cgrp);
return PTR_ERR(new);
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index 5ee615a59fe1..ecb4b4ff4ce0 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -1087,7 +1087,7 @@ int psi_show(struct seq_file *m, struct psi_group *group, enum psi_res res)
}
struct psi_trigger *psi_trigger_create(struct psi_group *group,
- char *buf, size_t nbytes, enum psi_res res)
+ char *buf, enum psi_res res)
{
struct psi_trigger *t;
enum psi_states state;
@@ -1316,7 +1316,7 @@ static ssize_t psi_write(struct file *file, const char __user *user_buf,
return -EBUSY;
}
- new = psi_trigger_create(&psi_system, buf, nbytes, res);
+ new = psi_trigger_create(&psi_system, buf, res);
if (IS_ERR(new)) {
mutex_unlock(&seq->lock);
return PTR_ERR(new);
--
2.32.0
After commit 5f69a6577bc3 ("psi: dont alloc memory for psi by default"),
the memory used by struct psi_group is no longer allocated and zeroed
in cgroup_create().
Since the memory of struct psi_group is not zeroed, the data in this
memory is random, which will lead to inaccurate psi statistics when
creating a new cgroup.
So we use kzlloc() to allocate and zero the struct psi_group and
remove the redundant zeroing in group_init().
Steps to reproduce:
1. Use cgroup v2 and enable CONFIG_PSI
2. Create a new cgroup, and query psi statistics
mkdir /sys/fs/cgroup/test
cat /sys/fs/cgroup/test/cpu.pressure
some avg10=0.00 avg60=0.00 avg300=47927752200.00 total=12884901
full avg10=561815124.00 avg60=125835394188.00 avg300=1077090462000.00 total=10273561772
cat /sys/fs/cgroup/test/io.pressure
some avg10=1040093132823.95 avg60=1203770351379.21 avg300=3862252669559.46 total=4294967296
full avg10=921884564601.39 avg60=0.00 avg300=1984507298.35 total=442381631
cat /sys/fs/cgroup/test/memory.pressure
some avg10=232476085778.11 avg60=0.00 avg300=0.00 total=0
full avg10=0.00 avg60=0.00 avg300=2585658472280.57 total=12884901
Fixes: commit 5f69a6577bc3 ("psi: dont alloc memory for psi by default")
Signed-off-by: Hao Jia <[email protected]>
---
kernel/sched/psi.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index ec66b40bdd40..5ee615a59fe1 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -190,12 +190,8 @@ static void group_init(struct psi_group *group)
/* Init trigger-related members */
mutex_init(&group->trigger_lock);
INIT_LIST_HEAD(&group->triggers);
- memset(group->nr_triggers, 0, sizeof(group->nr_triggers));
- group->poll_states = 0;
group->poll_min_period = U32_MAX;
- memset(group->polling_total, 0, sizeof(group->polling_total));
group->polling_next_update = ULLONG_MAX;
- group->polling_until = 0;
init_waitqueue_head(&group->poll_wait);
timer_setup(&group->poll_timer, poll_timer_fn, 0);
rcu_assign_pointer(group->poll_task, NULL);
@@ -957,7 +953,7 @@ int psi_cgroup_alloc(struct cgroup *cgroup)
if (static_branch_likely(&psi_disabled))
return 0;
- cgroup->psi = kmalloc(sizeof(struct psi_group), GFP_KERNEL);
+ cgroup->psi = kzalloc(sizeof(struct psi_group), GFP_KERNEL);
if (!cgroup->psi)
return -ENOMEM;
--
2.32.0
* Hao Jia <[email protected]> wrote:
> These three patches are about PSI.
> patch 1: Fixed PSI statistics error caused by unzeroed memory
> in struct psi_group.
> patch 2 and patch 3 are to clean up some unused functions
> and parameters.
>
> Hao Jia (3):
> sched/psi: Zero the memory of struct psi_group
> sched/psi: Remove unused parameter nbytes of psi_trigger_create()
> sched/psi: Remove redundant cgroup_psi() when !CONFIG_CGROUPS
>
> include/linux/cgroup.h | 5 -----
> include/linux/psi.h | 2 +-
> kernel/cgroup/cgroup.c | 2 +-
> kernel/sched/psi.c | 10 +++-------
> 4 files changed, 5 insertions(+), 14 deletions(-)
LGTM, and I suspect the scheduler fix wants to go upstream via the tree
that introduced the bug, the cgroup tree?
For the series:
Reviewed-by: Ingo Molnar <[email protected]>
Thanks,
Ingo
(cc'ing Johannes)
On Sat, Aug 06, 2022 at 08:05:07PM +0800, Hao Jia wrote:
> These three patches are about PSI.
> patch 1: Fixed PSI statistics error caused by unzeroed memory
> in struct psi_group.
> patch 2 and patch 3 are to clean up some unused functions
> and parameters.
>
> Hao Jia (3):
> sched/psi: Zero the memory of struct psi_group
> sched/psi: Remove unused parameter nbytes of psi_trigger_create()
> sched/psi: Remove redundant cgroup_psi() when !CONFIG_CGROUPS
>
> include/linux/cgroup.h | 5 -----
> include/linux/psi.h | 2 +-
> kernel/cgroup/cgroup.c | 2 +-
> kernel/sched/psi.c | 10 +++-------
> 4 files changed, 5 insertions(+), 14 deletions(-)
Johannes, care to review these patches?
Thanks.
--
tejun
On Sat, Aug 06, 2022 at 08:05:10PM +0800, Hao Jia wrote:
> cgroup_psi() is only called under CONFIG_CGROUPS.
> We don't need cgroup_psi() when !CONFIG_CGROUPS,
> so we can remove it in this case.
>
> Signed-off-by: Hao Jia <[email protected]>
Acked-by: Johannes Weiner <[email protected]>
On Sat, Aug 06, 2022 at 08:05:08PM +0800, Hao Jia wrote:
> After commit 5f69a6577bc3 ("psi: dont alloc memory for psi by default"),
> the memory used by struct psi_group is no longer allocated and zeroed
> in cgroup_create().
>
> Since the memory of struct psi_group is not zeroed, the data in this
> memory is random, which will lead to inaccurate psi statistics when
> creating a new cgroup.
>
> So we use kzlloc() to allocate and zero the struct psi_group and
> remove the redundant zeroing in group_init().
>
> Steps to reproduce:
> 1. Use cgroup v2 and enable CONFIG_PSI
> 2. Create a new cgroup, and query psi statistics
> mkdir /sys/fs/cgroup/test
> cat /sys/fs/cgroup/test/cpu.pressure
> some avg10=0.00 avg60=0.00 avg300=47927752200.00 total=12884901
> full avg10=561815124.00 avg60=125835394188.00 avg300=1077090462000.00 total=10273561772
>
> cat /sys/fs/cgroup/test/io.pressure
> some avg10=1040093132823.95 avg60=1203770351379.21 avg300=3862252669559.46 total=4294967296
> full avg10=921884564601.39 avg60=0.00 avg300=1984507298.35 total=442381631
>
> cat /sys/fs/cgroup/test/memory.pressure
> some avg10=232476085778.11 avg60=0.00 avg300=0.00 total=0
> full avg10=0.00 avg60=0.00 avg300=2585658472280.57 total=12884901
>
> Fixes: commit 5f69a6577bc3 ("psi: dont alloc memory for psi by default")
>
> Signed-off-by: Hao Jia <[email protected]>
Yikes! Yes, we relied on the embedding cgroup being kzalloc'd, or, in
the case of psi_system, on the psi_group being in NULLed static mem.
The partial zeroing in group_init() obscured that. Thanks for the fix.
Cc: [email protected] # 5.19
Acked-by: Johannes Weiner <[email protected]>
On Sat, Aug 06, 2022 at 08:05:09PM +0800, Hao Jia wrote:
> psi_trigger_create()'s 'nbytes' parameter is not used, so we can remove it.
>
> Signed-off-by: Hao Jia <[email protected]>
Right, it relies on \0-termination which the callers psi_write() and
kernfs_fop_write_iter() guarantee.
Acked-by: Johannes Weiner <[email protected]>
On Tue, Aug 09, 2022 at 07:37:21AM -1000, Tejun Heo wrote:
> (cc'ing Johannes)
>
> On Sat, Aug 06, 2022 at 08:05:07PM +0800, Hao Jia wrote:
> > These three patches are about PSI.
> > patch 1: Fixed PSI statistics error caused by unzeroed memory
> > in struct psi_group.
> > patch 2 and patch 3 are to clean up some unused functions
> > and parameters.
> >
> > Hao Jia (3):
> > sched/psi: Zero the memory of struct psi_group
> > sched/psi: Remove unused parameter nbytes of psi_trigger_create()
> > sched/psi: Remove redundant cgroup_psi() when !CONFIG_CGROUPS
> >
> > include/linux/cgroup.h | 5 -----
> > include/linux/psi.h | 2 +-
> > kernel/cgroup/cgroup.c | 2 +-
> > kernel/sched/psi.c | 10 +++-------
> > 4 files changed, 5 insertions(+), 14 deletions(-)
>
> Johannes, care to review these patches?
They look good to me.
Thanks!
On Sat, Aug 06, 2022 at 08:05:07PM +0800, Hao Jia wrote:
> These three patches are about PSI.
> patch 1: Fixed PSI statistics error caused by unzeroed memory
> in struct psi_group.
> patch 2 and patch 3 are to clean up some unused functions
> and parameters.
Applied 1-3 to cgroup/for-6.0-fixes.
Thanks.
--
tejun