2021-05-05 11:22:16

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH 0/6] sched,delayacct: Some cleanups

Hi,

Due to:

https://lkml.kernel.org/r/[email protected]

and general principle, delayacct really shouldn't be using ktime (pvclock also
really shouldn't be doing what it does, but that's another story). This lead me
to looking at the SCHED_INFO, SCHEDSTATS, DELAYACCT (and PSI) accounting hell.

The rest of the patches are an attempt at simplifying all that a little. All
that crud is enabled by default for distros which is leading to a death by a
thousand cuts.

The last patch is an attempt at default disabling DELAYACCT, because I don't
think anybody actually uses that much, but what do I know, there were no ill
effects on my testbox. Perhaps we should mirror
/proc/sys/kernel/sched_schedstats and provide a delayacct sysctl for runtime
frobbing.


2021-05-05 23:23:35

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Wed, May 05, 2021 at 12:59:40PM +0200, Peter Zijlstra wrote:
> Hi,
>
> Due to:
>
> https://lkml.kernel.org/r/[email protected]
>
> and general principle, delayacct really shouldn't be using ktime (pvclock also
> really shouldn't be doing what it does, but that's another story). This lead me
> to looking at the SCHED_INFO, SCHEDSTATS, DELAYACCT (and PSI) accounting hell.
>
> The rest of the patches are an attempt at simplifying all that a little. All
> that crud is enabled by default for distros which is leading to a death by a
> thousand cuts.
>
> The last patch is an attempt at default disabling DELAYACCT, because I don't
> think anybody actually uses that much, but what do I know, there were no ill
> effects on my testbox. Perhaps we should mirror
> /proc/sys/kernel/sched_schedstats and provide a delayacct sysctl for runtime
> frobbing.
>

There are tools like iotop that use delayacct to display information. When the
code was checked in, we did run SPEC* back in the day 2006 to find overheads,
nothing significant showed. Do we have any date on the overhead your seeing?

I'll look at the patches

Balbir Singh.

2021-05-06 09:33:23

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Thu, May 06, 2021 at 08:29:40AM +1000, Balbir Singh wrote:
> On Wed, May 05, 2021 at 12:59:40PM +0200, Peter Zijlstra wrote:
> > Hi,
> >
> > Due to:
> >
> > https://lkml.kernel.org/r/[email protected]
> >
> > and general principle, delayacct really shouldn't be using ktime (pvclock also
> > really shouldn't be doing what it does, but that's another story). This lead me
> > to looking at the SCHED_INFO, SCHEDSTATS, DELAYACCT (and PSI) accounting hell.
> >
> > The rest of the patches are an attempt at simplifying all that a little. All
> > that crud is enabled by default for distros which is leading to a death by a
> > thousand cuts.
> >
> > The last patch is an attempt at default disabling DELAYACCT, because I don't
> > think anybody actually uses that much, but what do I know, there were no ill
> > effects on my testbox. Perhaps we should mirror
> > /proc/sys/kernel/sched_schedstats and provide a delayacct sysctl for runtime
> > frobbing.
> >
>
> There are tools like iotop that use delayacct to display information.

Right, but how many actual people use that? Does that justify saddling
the whole sodding world with the overhead?

> When the
> code was checked in, we did run SPEC* back in the day 2006 to find overheads,
> nothing significant showed. Do we have any date on the overhead your seeing?

I've not looked, but having it disabled saves that per-task allocation
and that spinlock in delayacct_end() for iowait wakeups and a bunch of
cache misses ofcourse.

I doubt SPEC is a benchmark that tickles those paths much if at all.

The thing is; we can't just keep growing more and more stats, that'll
kill us quite dead.

2021-05-07 09:54:15

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Wed, May 05 2021 at 12:59, Peter Zijlstra wrote:
> Due to:
>
> https://lkml.kernel.org/r/[email protected]
>
> and general principle, delayacct really shouldn't be using ktime (pvclock also
> really shouldn't be doing what it does, but that's another story). This lead me
> to looking at the SCHED_INFO, SCHEDSTATS, DELAYACCT (and PSI) accounting hell.
>
> The rest of the patches are an attempt at simplifying all that a little. All
> that crud is enabled by default for distros which is leading to a death by a
> thousand cuts.
>
> The last patch is an attempt at default disabling DELAYACCT, because I don't
> think anybody actually uses that much, but what do I know, there were no ill
> effects on my testbox. Perhaps we should mirror
> /proc/sys/kernel/sched_schedstats and provide a delayacct sysctl for runtime
> frobbing.

Reviewed-by: Thomas Gleixner <[email protected]>

2021-05-07 16:41:48

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Thu, May 06, 2021 at 11:13:52AM +0200, Peter Zijlstra wrote:
> On Thu, May 06, 2021 at 08:29:40AM +1000, Balbir Singh wrote:
> > On Wed, May 05, 2021 at 12:59:40PM +0200, Peter Zijlstra wrote:
> > > Hi,
> > >
> > > Due to:
> > >
> > > https://lkml.kernel.org/r/[email protected]
> > >
> > > and general principle, delayacct really shouldn't be using ktime (pvclock also
> > > really shouldn't be doing what it does, but that's another story). This lead me
> > > to looking at the SCHED_INFO, SCHEDSTATS, DELAYACCT (and PSI) accounting hell.
> > >
> > > The rest of the patches are an attempt at simplifying all that a little. All
> > > that crud is enabled by default for distros which is leading to a death by a
> > > thousand cuts.
> > >
> > > The last patch is an attempt at default disabling DELAYACCT, because I don't
> > > think anybody actually uses that much, but what do I know, there were no ill
> > > effects on my testbox. Perhaps we should mirror
> > > /proc/sys/kernel/sched_schedstats and provide a delayacct sysctl for runtime
> > > frobbing.
> > >
> >
> > There are tools like iotop that use delayacct to display information.
>
> Right, but how many actual people use that? Does that justify saddling
> the whole sodding world with the overhead?
>

Not sure I have that data.

> > When the
> > code was checked in, we did run SPEC* back in the day 2006 to find overheads,
> > nothing significant showed. Do we have any date on the overhead your seeing?
>
> I've not looked, but having it disabled saves that per-task allocation
> and that spinlock in delayacct_end() for iowait wakeups and a bunch of
> cache misses ofcourse.
>
> I doubt SPEC is a benchmark that tickles those paths much if at all.
>
> The thing is; we can't just keep growing more and more stats, that'll
> kill us quite dead.


I don't disagree, we've had these around for a while and I know of users
that use these stats to find potential starvation. I am OK with default
disabled. I suspect distros will have the final say.

Balbir Singh.

2021-05-10 07:10:25

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups


* Peter Zijlstra <[email protected]> wrote:

> Hi,
>
> Due to:
>
> https://lkml.kernel.org/r/[email protected]
>
> and general principle, delayacct really shouldn't be using ktime (pvclock also
> really shouldn't be doing what it does, but that's another story). This lead me
> to looking at the SCHED_INFO, SCHEDSTATS, DELAYACCT (and PSI) accounting hell.
>
> The rest of the patches are an attempt at simplifying all that a little. All
> that crud is enabled by default for distros which is leading to a death by a
> thousand cuts.
>
> The last patch is an attempt at default disabling DELAYACCT, because I don't
> think anybody actually uses that much, but what do I know, there were no ill
> effects on my testbox. Perhaps we should mirror
> /proc/sys/kernel/sched_schedstats and provide a delayacct sysctl for runtime
> frobbing.

Reviewed-by: Ingo Molnar <[email protected]>

(This includes the #6 RFC patch.)

Thanks,

Ingo

2021-05-10 16:01:52

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH 7/6] delayacct: Add sysctl to enable at runtime


Just like sched_schedstats, allow runtime enabling (and disabling) of
delayacct. This is useful if one forgot to add the delayacct boot time
option.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
Documentation/accounting/delay-accounting.rst | 6 ++--
kernel/delayacct.c | 36 ++++++++++++++++++++++++--
kernel/sysctl.c | 12 ++++++++
3 files changed, 50 insertions(+), 4 deletions(-)

--- a/Documentation/accounting/delay-accounting.rst
+++ b/Documentation/accounting/delay-accounting.rst
@@ -74,8 +74,10 @@ Delay accounting is disabled by default

delayacct

-to the kernel boot options. The rest of the instructions
-below assume this has been done.
+to the kernel boot options. The rest of the instructions below assume this has
+been done. Alternatively, use sysctl kernel.sched_delayacct to switch the state
+at runtime. Note however that only tasks started after enabling it will have
+delayacct information.

After the system has booted up, use a utility
similar to getdelays.c to access the delays
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -18,6 +18,17 @@ DEFINE_STATIC_KEY_FALSE(delayacct_key);
int delayacct_on __read_mostly; /* Delay accounting turned on/off */
struct kmem_cache *delayacct_cache;

+static void set_delayacct(bool enabled)
+{
+ if (enabled) {
+ static_branch_enable(&delayacct_key);
+ delayacct_on = 1;
+ } else {
+ delayacct_on = 0;
+ static_branch_disable(&delayacct_key);
+ }
+}
+
static int __init delayacct_setup_enable(char *str)
{
delayacct_on = 1;
@@ -29,9 +40,30 @@ void delayacct_init(void)
{
delayacct_cache = KMEM_CACHE(task_delay_info, SLAB_PANIC|SLAB_ACCOUNT);
delayacct_tsk_init(&init_task);
- if (delayacct_on)
- static_branch_enable(&delayacct_key);
+ set_delayacct(delayacct_on);
+}
+
+#ifdef CONFIG_PROC_SYSCTL
+int sysctl_delayacct(struct ctl_table *table, int write, void *buffer,
+ size_t *lenp, loff_t *ppos)
+{
+ int state = delayacct_on;
+ struct ctl_table t;
+ int err;
+
+ if (write && !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ t = *table;
+ t.data = &state;
+ err = proc_dointvec_minmax(&t, write, buffer, lenp, ppos);
+ if (err < 0)
+ return err;
+ if (write)
+ set_delayacct(state);
+ return err;
}
+#endif

void __delayacct_tsk_init(struct task_struct *tsk)
{
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -71,6 +71,7 @@
#include <linux/coredump.h>
#include <linux/latencytop.h>
#include <linux/pid.h>
+#include <linux/delayacct.h>

#include "../lib/kstrtox.h"

@@ -1727,6 +1728,17 @@ static struct ctl_table kern_table[] = {
.extra2 = SYSCTL_ONE,
},
#endif /* CONFIG_SCHEDSTATS */
+#ifdef CONFIG_TASK_DELAY_ACCT
+ {
+ .procname = "sched_delayacct",
+ .data = NULL,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = sysctl_delayacct,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+#endif /* CONFIG_TASK_DELAY_ACCT */
#ifdef CONFIG_NUMA_BALANCING
{
.procname = "numa_balancing",

Subject: [tip: sched/core] delayacct: Add sysctl to enable at runtime

The following commit has been merged into the sched/core branch of tip:

Commit-ID: 0cd7c741f01de13dc1eecf22557593b3514639bb
Gitweb: https://git.kernel.org/tip/0cd7c741f01de13dc1eecf22557593b3514639bb
Author: Peter Zijlstra <[email protected]>
AuthorDate: Mon, 10 May 2021 14:01:00 +02:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Wed, 12 May 2021 11:43:25 +02:00

delayacct: Add sysctl to enable at runtime

Just like sched_schedstats, allow runtime enabling (and disabling) of
delayacct. This is useful if one forgot to add the delayacct boot time
option.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
Documentation/accounting/delay-accounting.rst | 6 ++-
include/linux/delayacct.h | 4 ++-
kernel/delayacct.c | 36 +++++++++++++++++-
kernel/sysctl.c | 12 ++++++-
4 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/Documentation/accounting/delay-accounting.rst b/Documentation/accounting/delay-accounting.rst
index f20b282..1b8b46d 100644
--- a/Documentation/accounting/delay-accounting.rst
+++ b/Documentation/accounting/delay-accounting.rst
@@ -74,8 +74,10 @@ To enable, add::

delayacct

-to the kernel boot options. The rest of the instructions
-below assume this has been done.
+to the kernel boot options. The rest of the instructions below assume this has
+been done. Alternatively, use sysctl kernel.task_delayacct to switch the state
+at runtime. Note however that only tasks started after enabling it will have
+delayacct information.

After the system has booted up, use a utility
similar to getdelays.c to access the delays
diff --git a/include/linux/delayacct.h b/include/linux/delayacct.h
index 225c8e0..af7e6eb 100644
--- a/include/linux/delayacct.h
+++ b/include/linux/delayacct.h
@@ -65,6 +65,10 @@ DECLARE_STATIC_KEY_FALSE(delayacct_key);
extern int delayacct_on; /* Delay accounting turned on/off */
extern struct kmem_cache *delayacct_cache;
extern void delayacct_init(void);
+
+extern int sysctl_delayacct(struct ctl_table *table, int write, void *buffer,
+ size_t *lenp, loff_t *ppos);
+
extern void __delayacct_tsk_init(struct task_struct *);
extern void __delayacct_tsk_exit(struct task_struct *);
extern void __delayacct_blkio_start(void);
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 3f08690..51530d5 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -18,6 +18,17 @@ DEFINE_STATIC_KEY_FALSE(delayacct_key);
int delayacct_on __read_mostly; /* Delay accounting turned on/off */
struct kmem_cache *delayacct_cache;

+static void set_delayacct(bool enabled)
+{
+ if (enabled) {
+ static_branch_enable(&delayacct_key);
+ delayacct_on = 1;
+ } else {
+ delayacct_on = 0;
+ static_branch_disable(&delayacct_key);
+ }
+}
+
static int __init delayacct_setup_enable(char *str)
{
delayacct_on = 1;
@@ -29,9 +40,30 @@ void delayacct_init(void)
{
delayacct_cache = KMEM_CACHE(task_delay_info, SLAB_PANIC|SLAB_ACCOUNT);
delayacct_tsk_init(&init_task);
- if (delayacct_on)
- static_branch_enable(&delayacct_key);
+ set_delayacct(delayacct_on);
+}
+
+#ifdef CONFIG_PROC_SYSCTL
+int sysctl_delayacct(struct ctl_table *table, int write, void *buffer,
+ size_t *lenp, loff_t *ppos)
+{
+ int state = delayacct_on;
+ struct ctl_table t;
+ int err;
+
+ if (write && !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ t = *table;
+ t.data = &state;
+ err = proc_dointvec_minmax(&t, write, buffer, lenp, ppos);
+ if (err < 0)
+ return err;
+ if (write)
+ set_delayacct(state);
+ return err;
}
+#endif

void __delayacct_tsk_init(struct task_struct *tsk)
{
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 14edf84..0afbfc8 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -71,6 +71,7 @@
#include <linux/coredump.h>
#include <linux/latencytop.h>
#include <linux/pid.h>
+#include <linux/delayacct.h>

#include "../lib/kstrtox.h"

@@ -1727,6 +1728,17 @@ static struct ctl_table kern_table[] = {
.extra2 = SYSCTL_ONE,
},
#endif /* CONFIG_SCHEDSTATS */
+#ifdef CONFIG_TASK_DELAY_ACCT
+ {
+ .procname = "task_delayacct",
+ .data = NULL,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = sysctl_delayacct,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+#endif /* CONFIG_TASK_DELAY_ACCT */
#ifdef CONFIG_NUMA_BALANCING
{
.procname = "numa_balancing",

2021-05-12 11:35:20

by Mel Gorman

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Fri, May 07, 2021 at 10:38:10PM +1000, Balbir Singh wrote:
> On Thu, May 06, 2021 at 11:13:52AM +0200, Peter Zijlstra wrote:
> > On Thu, May 06, 2021 at 08:29:40AM +1000, Balbir Singh wrote:
> > > On Wed, May 05, 2021 at 12:59:40PM +0200, Peter Zijlstra wrote:
> > > > Hi,
> > > >
> > > > Due to:
> > > >
> > > > https://lkml.kernel.org/r/[email protected]
> > > >
> > > > and general principle, delayacct really shouldn't be using ktime (pvclock also
> > > > really shouldn't be doing what it does, but that's another story). This lead me
> > > > to looking at the SCHED_INFO, SCHEDSTATS, DELAYACCT (and PSI) accounting hell.
> > > >
> > > > The rest of the patches are an attempt at simplifying all that a little. All
> > > > that crud is enabled by default for distros which is leading to a death by a
> > > > thousand cuts.
> > > >
> > > > The last patch is an attempt at default disabling DELAYACCT, because I don't
> > > > think anybody actually uses that much, but what do I know, there were no ill
> > > > effects on my testbox. Perhaps we should mirror
> > > > /proc/sys/kernel/sched_schedstats and provide a delayacct sysctl for runtime
> > > > frobbing.
> > > >
> > >
> > > There are tools like iotop that use delayacct to display information.
> >
> > Right, but how many actual people use that? Does that justify saddling
> > the whole sodding world with the overhead?
> >
>
> Not sure I have that data.
>

It's used occasionally as a debugging tool when looking at IO problems
in general. Like sched_schedstats, it seems unnecesary to incur overhead
unless someone is debugging.

Minimally disabling had a small positive impact -- tbench 0-8% gain
depending on thread count and machine, positive impact in general to
specjbb2005, neutral on hackbench and most of the other workloads checked.

> > > When the
> > > code was checked in, we did run SPEC* back in the day 2006 to find overheads,
> > > nothing significant showed. Do we have any date on the overhead your seeing?
> >
> > I've not looked, but having it disabled saves that per-task allocation
> > and that spinlock in delayacct_end() for iowait wakeups and a bunch of
> > cache misses ofcourse.
> >
> > I doubt SPEC is a benchmark that tickles those paths much if at all.
> >
> > The thing is; we can't just keep growing more and more stats, that'll
> > kill us quite dead.
>
>
> I don't disagree, we've had these around for a while and I know of users
> that use these stats to find potential starvation. I am OK with default
> disabled. I suspect distros will have the final say.
>

I think default disabled should be fine. At worst when dealing with a bug
there would be a need to either reboot or enable at runtime with patch
7 included and add that instruction to the bug report when requesting
iotop data. At worst, a distro could revert the patch if iotop generated
too many bug reports or patch iotop in the distro package.

Alternatively, I've added Paul Wise to the cc who is the latest
committer to iotop. Maybe he knows who could add/commit a check for
sysctl.sched_delayacct and if it exists then check if it's 1 and display
an error suggesting corrective action (add delayacct to the kernel command
line or sysctl sched.sched_delayacct=1). iotop appears to be in maintenance
mode but gets occasional commits even if it has not had a new version
since 2013 so maybe it could get a 0.7 tag if such a check was added.

--
Mel Gorman
SUSE Labs

2021-05-12 11:42:00

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Wed, May 12, 2021 at 12:34:19PM +0100, Mel Gorman wrote:
> > I don't disagree, we've had these around for a while and I know of users
> > that use these stats to find potential starvation. I am OK with default
> > disabled. I suspect distros will have the final say.
> >
>
> I think default disabled should be fine. At worst when dealing with a bug
> there would be a need to either reboot or enable at runtime with patch
> 7 included and add that instruction to the bug report when requesting
> iotop data. At worst, a distro could revert the patch if iotop generated
> too many bug reports or patch iotop in the distro package.
>
> Alternatively, I've added Paul Wise to the cc who is the latest
> committer to iotop. Maybe he knows who could add/commit a check for
> sysctl.sched_delayacct and if it exists then check if it's 1 and display
> an error suggesting corrective action (add delayacct to the kernel command
> line or sysctl sched.sched_delayacct=1). iotop appears to be in maintenance
> mode but gets occasional commits even if it has not had a new version
> since 2013 so maybe it could get a 0.7 tag if such a check was added.

In the final commit I made the knob be known as task_delayacct. I
figured that was a slightly better name.

But yes, a check in iotop, if that really is the sole user of all this,
would be very nice.

Thanks!

2021-05-12 11:42:37

by Mel Gorman

[permalink] [raw]
Subject: Re: [PATCH 7/6] delayacct: Add sysctl to enable at runtime

On Mon, May 10, 2021 at 02:05:13PM +0200, Peter Zijlstra wrote:
>
> Just like sched_schedstats, allow runtime enabling (and disabling) of
> delayacct. This is useful if one forgot to add the delayacct boot time
> option.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> Documentation/accounting/delay-accounting.rst | 6 ++--
> kernel/delayacct.c | 36 ++++++++++++++++++++++++--
> kernel/sysctl.c | 12 ++++++++
> 3 files changed, 50 insertions(+), 4 deletions(-)
>

Update sysctl/kernel.rst?

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 1d56a6b73a4e..5d9193bd8d27 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -1087,6 +1087,13 @@ Model available). If your platform happens to meet the
requirements for EAS but you do not want to use it, change
this value to 0.

+sched_delayacct
+===============
+
+Enables/disables task delay accounting (see
+:doc:`accounting/delay-accounting.rst`). Enabling this feature incurs
+a small amount of overhead in the scheduler but is useful for debugging
+and performance tuning. It is required by some tools such as iotop.

sched_schedstats
================

--
Mel Gorman
SUSE Labs

2021-05-12 12:52:57

by Paul Wise

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Wed, 2021-05-12 at 12:34 +0100, Mel Gorman wrote:

> Alternatively, I've added Paul Wise to the cc who is the latest
> committer to iotop.  Maybe he knows who could add/commit a check for
> sysctl.sched_delayacct and if it exists then check if it's 1 and display
> an error suggesting corrective action (add delayacct to the kernel command
> line or sysctl sched.sched_delayacct=1). iotop appears to be in maintenance
> mode but gets occasional commits even if it has not had a new version
> since 2013 so maybe it could get a 0.7 tag if such a check was added.

I am able to commit to the iotop repository but I don't have permission
from the author to make releases nor do I have access to the website.

I am happy to apply any patches anyone has for iotop and upload the
result to Debian, or I'll be willing to write patches to cope with
changes in Linux behaviour, if given a succinct explanation of what
changes are needed in iotop, once the Linux changes are fully merged.

As well as the Python iotop implementation, there is one written in C
with more features, so please also file an issue or pull request there.
Please note that I don't have commit access to that repository though.

https://github.com/Tomas-M/iotop

--
bye,
pabs

https://bonedaddy.net/pabs3/


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part

2021-05-12 13:21:52

by Mel Gorman

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Wed, May 12, 2021 at 08:23:51PM +0800, Paul Wise wrote:
> On Wed, 2021-05-12 at 12:34 +0100, Mel Gorman wrote:
>
> > Alternatively, I've added Paul Wise to the cc who is the latest
> > committer to iotop.? Maybe he knows who could add/commit a check for
> > sysctl.sched_delayacct and if it exists then check if it's 1 and display
> > an error suggesting corrective action (add delayacct to the kernel command
> > line or sysctl sched.sched_delayacct=1). iotop appears to be in maintenance
> > mode but gets occasional commits even if it has not had a new version
> > since 2013 so maybe it could get a 0.7 tag if such a check was added.
>
> I am able to commit to the iotop repository but I don't have permission
> from the author to make releases nor do I have access to the website.
>
> I am happy to apply any patches anyone has for iotop and upload the
> result to Debian, or I'll be willing to write patches to cope with
> changes in Linux behaviour, if given a succinct explanation of what
> changes are needed in iotop, once the Linux changes are fully merged.
>

If you send me the same patch, I can do submit a request to the devel
package for openSUSE. I don't have commit access but I would be surprised
if the package maintainer didn't accept the request. Obviously, I'll
build+boot a kernel that includes the final version of this series in
case of any naming changes or other oddities.

> As well as the Python iotop implementation, there is one written in C
> with more features, so please also file an issue or pull request there.
> Please note that I don't have commit access to that repository though.
>

Good thinking. I'll open a bug on github when I've tested your iotop
patch so that the bug report is more coherent.

Thanks for the quick response.

--
Mel Gorman
SUSE Labs


Attachments:
(No filename) (1.80 kB)
signature.asc (499.00 B)
Digital signature
Download all attachments

2021-05-13 01:34:13

by Paul Wise

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Wed, 2021-05-12 at 14:00 +0100, Mel Gorman wrote:

> If you send me the same patch, I can do submit a request to the devel
> package for openSUSE. I don't have commit access but I would be surprised
> if the package maintainer didn't accept the request. Obviously, I'll
> build+boot a kernel that includes the final version of this series in
> case of any naming changes or other oddities.

At this point I'm not clear exactly what needs to be done and whether
or not the details have been nailed down enough that it is time to
commit the change to the iotop-py and iotop-c git repositories.

I recommend upgrading the openSUSE iotop package to the latest git
commit rather than just applying the latest patch on top.

Alternatively, once the patch is applied I can probably overstep my
permissions and add a tag to the iotop-py git repository, in case folks
are happy to pull from the git repository instead of the website.

> Good thinking. I'll open a bug on github when I've tested your iotop
> patch so that the bug report is more coherent.

OK, sounds good.

PS: does Linux have a facility for userspace processes to convert
syscall names to numbers for the currently running Linux kernel? I
noticed that iotop-py just hard-codes the syscall numbers for
ioprio_set and ioprio_get on common arches, missing newer arches.

--
bye,
pabs

https://bonedaddy.net/pabs3/


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part
Subject: [tip: sched/core] delayacct: Document task_delayacct sysctl

The following commit has been merged into the sched/core branch of tip:

Commit-ID: fcb501704554eebfd27e3220b0540997fd2b24a8
Gitweb: https://git.kernel.org/tip/fcb501704554eebfd27e3220b0540997fd2b24a8
Author: Mel Gorman <[email protected]>
AuthorDate: Wed, 12 May 2021 12:40:35 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Tue, 18 May 2021 12:53:53 +02:00

delayacct: Document task_delayacct sysctl

Update sysctl/kernel.rst.

Signed-off-by: Mel Gorman <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
Documentation/admin-guide/sysctl/kernel.rst | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 1d56a6b..ebd2f99 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -1087,6 +1087,13 @@ Model available). If your platform happens to meet the
requirements for EAS but you do not want to use it, change
this value to 0.

+task_delayacct
+===============
+
+Enables/disables task delay accounting (see
+:doc:`accounting/delay-accounting.rst`). Enabling this feature incurs
+a small amount of overhead in the scheduler but is useful for debugging
+and performance tuning. It is required by some tools such as iotop.

sched_schedstats
================

2021-06-25 00:59:44

by Paul Wise

[permalink] [raw]
Subject: Re: [PATCH 0/6] sched,delayacct: Some cleanups

On Wed, 2021-05-12 at 12:34 +0100, Mel Gorman wrote:

> Alternatively, I've added Paul Wise to the cc who is the latest
> committer to iotop.  Maybe he knows who could add/commit a check for
> sysctl.sched_delayacct and if it exists then check if it's 1 and display
> an error suggesting corrective action (add delayacct to the kernel command
> line or sysctl sched.sched_delayacct=1). iotop appears to be in maintenance
> mode but gets occasional commits even if it has not had a new version
> since 2013 so maybe it could get a 0.7 tag if such a check was added.

Did the proposed changes get merged?

If so, please let me know the details of what needs to happen in iotop
and iotop-c to cope with the changes in the Linux kernel.

--
bye,
pabs

https://bonedaddy.net/pabs3/


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part