Re: [Toybox] [PATCH] Add uclampset(1).

2023-02-26 Thread Rob Landley 
On 12/11/21 13:22, Rob Landley wrote:
>> i wondered that too, but this is what the util-linux one does. (which is
>> presumably just because that's what taskset does. so -- like me with 
>> `default y`
>> at least they're _consistent_.)
> 
> Complicates the code, but simplifies the help text. Hmmm...
> 
> I suppose I could move -a to the end? There's tension between "options in
> alphabetical order so you can find them" and "options in some sort of coherent
> order where they make SENSE".
> 
> You'll note that find.c errs ENTIRELY towards the second, but you've pretty 
> much
> GOTTA collate name/iname path/ipath user/nouser group/nogroup, true/false,
> depth/maxdepth/mindepth... Plus you'll notice it's totally cheating by having
> the "-option  explanation" indentation be WAY different on the left (16 chars
> after the dash) side than on the right side (11 chars). I kind of want to
> collate prune/xdev/empty/quit, but... hmmm, maybe options with no arguments on
> the right and ones with arguments on the left? Except I haven't got an equal
> number of types. (Plus those last two lines just go all the way across because
> there's too much to explain, and newerXY would be right under -newer...)
> 
> Anyway, this kinda polishing thing can eat All The Time if I let it. :)

I have a lot of old open tabs, but just FYI, commit df0eb047c764 at least makes
this slightly less bad.

(One of my TODO items is to reread the mailing list history and my blog to find
old TODO items that fell through the cracks. The "make videos of every command"
todo item _also_ turns into Yet Another Polishing Pass on each one I try...)

Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Add uclampset(1).

2021-12-15 Thread enh via Toybox 
On Sat, Dec 11, 2021 at 11:21 AM Rob Landley  wrote:

> On 12/10/21 1:53 PM, enh wrote:
> > On Fri, Dec 10, 2021 at 6:52 AM Rob Landley  > > wrote:
> >
> > On 12/9/21 7:31 PM, enh via Toybox wrote:
> > > No standard, but part of util-linux.
> >
> > Don't default y in pending.
> >
> > yeah, sorry. i realized that after i'd gone to bed. at least i'm
> consistent ...
> > i *always* make that mistake.
>
> Eh, should be a fairly fast promotion, I just need to get it to _work_
> first. :)
>
> > USE_TASKSET(NEWTOY(uclampset)) doesn't match.
> >
> > d'oh. _that's_ a new mistake though!
>
> I usually do it with "USE_HELLO" so have learned to check. :)
>
> > Does this require root?
> >
> > as you can tell from the previous mistake, i took taskset as a basis for
> this.
> > it seemed plausible to me that if taskset needed STAYROOT (which i
> assume is
> > what you're asking about here), this would too? but i've no idea.
>
> I suspect taskset doesn't either. It needs it to set the mask of processes
> you
> don't own, but you should legitimately be root for that?
>

(sounds plausible to me, but suid isn't a thing on Android anyway, so (a)
it doesn't matter to me, but (b) it also means you shouldn't trust my
intuition on such things :-) )


> (P.S. The kernel says "Invalid argument" for an attempt to set taskset to
> 0.
> That's one of the things I need to be sure to check about this new
> plumbing...)
>
> > >From a UI perspective, do we ever _not_ want -a?
> >
> > i wondered that too, but this is what the util-linux one does. (which is
> > presumably just because that's what taskset does. so -- like me with
> `default y`
> > at least they're _consistent_.)
>
> Complicates the code, but simplifies the help text. Hmmm...
>
> I suppose I could move -a to the end? There's tension between "options in
> alphabetical order so you can find them" and "options in some sort of
> coherent
> order where they make SENSE".
>

yeah, when there's a small enough number that you can grok them in a
glance, alphabetical order is definitely best, but even i agree that the
"theme" grouping is necessary for stuff like find.


> You'll note that find.c errs ENTIRELY towards the second, but you've
> pretty much
> GOTTA collate name/iname path/ipath user/nouser group/nogroup, true/false,
> depth/maxdepth/mindepth... Plus you'll notice it's totally cheating by
> having
> the "-option  explanation" indentation be WAY different on the left (16
> chars
> after the dash) side than on the right side (11 chars). I kind of want to
> collate prune/xdev/empty/quit, but... hmmm, maybe options with no
> arguments on
> the right and ones with arguments on the left? Except I haven't got an
> equal
> number of types. (Plus those last two lines just go all the way across
> because
> there's too much to explain, and newerXY would be right under -newer...)
>
> Anyway, this kinda polishing thing can eat All The Time if I let it. :)
>
> > Sigh. It's HARD to come up with a good terse description of -R.
> >
> > yeah, as you can see, i gave up: "you're going to want to read a man
> page ---
> > here's what you should search for" was all i was going for there.
>
> -R  Reset child processes to default values on fork
>
> > $ grep syscall\.h toys/*/*.c
> > toys/other/chrt.c:#include 
> > toys/other/insmod.c:#include 
> > toys/other/ionice.c:#include 
> > toys/other/nsenter.c:#include 
> > toys/other/pivot_root.c:#include 
> > toys/other/readahead.c:#include 
> > toys/other/rmmod.c:#include 
> > toys/other/taskset.c:#include 
> > toys/pending/modprobe.c:#include 
> > toys/pending/strace.c:#include 
> > toys/pending/uclampset.c:#include 
> >
> > Is there a reason we haven't got sys/syscall.h in toys.h? Sure, it's
> not in
> > posix but it apparently originated in BSD:
> >
> >
> https://www.freebsd.org/cgi/man.cgi?query=syscall=2=html
> > <
> https://www.freebsd.org/cgi/man.cgi?query=syscall=2=html>
> >
> > So the header should at least _exist_ there, and including it not
> break the
> > build?
> >
> > yeah, there's a  on macOS too. (iirc it's the glibc
> 
> > synonym that's less portable.)
>
> I should do that as a separate checkin so you can at least pull to right
> before
> that if it breaks stuff...
>

i'm not _actually_ obliged to follow google's wider "no changes close to
holidays" rules, but i don't think i'll be touching anything but
documentation for the rest of 2021 :-)

anyway, github will show you whether you've broken the mac build thanks to
emolitor's work.


> > Ah. My devuaan kernel is 4.19+fixes, so this goes boing when I run
> it. Luckily,
> > I created mkroot... Hmmm... Except when I tried your version
> (without any of my
> > changes), it still went:
> >
> > Type exit when done.
> > # uclampset
> > uclampset: Need -p PID or CMD [ARG...]
> > # uclampset -p $$
> > sh (48) util_clamp: min: 0 max: 0
> >

Re: [Toybox] [PATCH] Add uclampset(1).

2021-12-11 Thread Rob Landley 
On 12/10/21 1:53 PM, enh wrote:
> On Fri, Dec 10, 2021 at 6:52 AM Rob Landley  > wrote:
> 
> On 12/9/21 7:31 PM, enh via Toybox wrote:
> > No standard, but part of util-linux.
> 
> Don't default y in pending.
> 
> yeah, sorry. i realized that after i'd gone to bed. at least i'm consistent 
> ...
> i *always* make that mistake.

Eh, should be a fairly fast promotion, I just need to get it to _work_ first. :)

> USE_TASKSET(NEWTOY(uclampset)) doesn't match.
> 
> d'oh. _that's_ a new mistake though!

I usually do it with "USE_HELLO" so have learned to check. :)

> Does this require root?
>  
> as you can tell from the previous mistake, i took taskset as a basis for this.
> it seemed plausible to me that if taskset needed STAYROOT (which i assume is
> what you're asking about here), this would too? but i've no idea.

I suspect taskset doesn't either. It needs it to set the mask of processes you
don't own, but you should legitimately be root for that?

(P.S. The kernel says "Invalid argument" for an attempt to set taskset to 0.
That's one of the things I need to be sure to check about this new plumbing...)

> >From a UI perspective, do we ever _not_ want -a?
> 
> i wondered that too, but this is what the util-linux one does. (which is
> presumably just because that's what taskset does. so -- like me with `default 
> y`
> at least they're _consistent_.)

Complicates the code, but simplifies the help text. Hmmm...

I suppose I could move -a to the end? There's tension between "options in
alphabetical order so you can find them" and "options in some sort of coherent
order where they make SENSE".

You'll note that find.c errs ENTIRELY towards the second, but you've pretty much
GOTTA collate name/iname path/ipath user/nouser group/nogroup, true/false,
depth/maxdepth/mindepth... Plus you'll notice it's totally cheating by having
the "-option  explanation" indentation be WAY different on the left (16 chars
after the dash) side than on the right side (11 chars). I kind of want to
collate prune/xdev/empty/quit, but... hmmm, maybe options with no arguments on
the right and ones with arguments on the left? Except I haven't got an equal
number of types. (Plus those last two lines just go all the way across because
there's too much to explain, and newerXY would be right under -newer...)

Anyway, this kinda polishing thing can eat All The Time if I let it. :)

> Sigh. It's HARD to come up with a good terse description of -R.
> 
> yeah, as you can see, i gave up: "you're going to want to read a man page ---
> here's what you should search for" was all i was going for there.

-R  Reset child processes to default values on fork

> $ grep syscall\.h toys/*/*.c
> toys/other/chrt.c:#include 
> toys/other/insmod.c:#include 
> toys/other/ionice.c:#include 
> toys/other/nsenter.c:#include 
> toys/other/pivot_root.c:#include 
> toys/other/readahead.c:#include 
> toys/other/rmmod.c:#include 
> toys/other/taskset.c:#include 
> toys/pending/modprobe.c:#include 
> toys/pending/strace.c:#include 
> toys/pending/uclampset.c:#include 
> 
> Is there a reason we haven't got sys/syscall.h in toys.h? Sure, it's not 
> in
> posix but it apparently originated in BSD:
> 
> https://www.freebsd.org/cgi/man.cgi?query=syscall=2=html
> 
> 
> So the header should at least _exist_ there, and including it not break 
> the
> build?
> 
> yeah, there's a  on macOS too. (iirc it's the glibc 
> synonym that's less portable.)

I should do that as a separate checkin so you can at least pull to right before
that if it breaks stuff...

> Ah. My devuaan kernel is 4.19+fixes, so this goes boing when I run it. 
> Luckily,
> I created mkroot... Hmmm... Except when I tried your version (without any 
> of my
> changes), it still went:
> 
> Type exit when done.
> # uclampset
> uclampset: Need -p PID or CMD [ARG...]
> # uclampset -p $$
> sh (48) util_clamp: min: 0 max: 0
> # uclampset -p $$ -m 42
> uclampset: sched_setattr for pid 48: Not supported
> # uclampset -p $$ -M 42
> uclampset: sched_setattr for pid 48: Not supported
> # cat /proc/version
> Linux version 5.16.0-rc3 (landley@driftwood) (x86_64-linux-musl-gcc (GCC) 
> 9.2.01
> 
> Do I need a kernel config entry or something?
> 
> i think so, yes. i got similar results to you on my laptop, but running on an
> x86-64 [virtual] device everything worked [to the extent that "i got back the
> values i put in, and plausible looking numbers from other stuff"].
> 
> i'm guessing it's these?
> 
> vsoc_x86_64:/# gzip -dc /proc/config.gz | grep UCLAMP
> CONFIG_UCLAMP_TASK=y

They added a config symbol for a new feature of an existing syscall.

Why?

> CONFIG_UCLAMP_BUCKETS_COUNT=20
> CONFIG_UCLAMP_TASK_GROUP=y

I think to test this I should just need the first one... Nope:

  Depends

Re: [Toybox] [PATCH] Add uclampset(1).

2021-12-10 Thread enh via Toybox 
On Fri, Dec 10, 2021 at 6:52 AM Rob Landley  wrote:

> On 12/9/21 7:31 PM, enh via Toybox wrote:
> > No standard, but part of util-linux.
>
> Don't default y in pending.
>

yeah, sorry. i realized that after i'd gone to bed. at least i'm consistent
... i *always* make that mistake.


> USE_TASKSET(NEWTOY(uclampset)) doesn't match.
>

d'oh. _that's_ a new mistake though!


> Does this require root?
>

as you can tell from the previous mistake, i took taskset as a basis for
this. it seemed plausible to me that if taskset needed STAYROOT (which i
assume is what you're asking about here), this would too? but i've no idea.


> From a UI perspective, do we ever _not_ want -a?
>

i wondered that too, but this is what the util-linux one does. (which is
presumably just because that's what taskset does. so -- like me with
`default y` at least they're _consistent_.)


> Sigh. It's HARD to come up with a good terse description of -R.
>

yeah, as you can see, i gave up: "you're going to want to read a man page
--- here's what you should search for" was all i was going for there.


> $ grep syscall\.h toys/*/*.c
> toys/other/chrt.c:#include 
> toys/other/insmod.c:#include 
> toys/other/ionice.c:#include 
> toys/other/nsenter.c:#include 
> toys/other/pivot_root.c:#include 
> toys/other/readahead.c:#include 
> toys/other/rmmod.c:#include 
> toys/other/taskset.c:#include 
> toys/pending/modprobe.c:#include 
> toys/pending/strace.c:#include 
> toys/pending/uclampset.c:#include 
>
> Is there a reason we haven't got sys/syscall.h in toys.h? Sure, it's not in
> posix but it apparently originated in BSD:
>
> https://www.freebsd.org/cgi/man.cgi?query=syscall=2=html
>
> So the header should at least _exist_ there, and including it not break
> the build?
>

yeah, there's a  on macOS too. (iirc it's the glibc
 synonym that's less portable.)


> Ah. My devauan kernel is 4.19+fixes, so this goes boing when I run it.
> Luckily,
> I created mkroot... Hmmm... Except when I tried your version (without any
> of my
> changes), it still went:
>
> Type exit when done.
> # uclampset
> uclampset: Need -p PID or CMD [ARG...]
> # uclampset -p $$
> sh (48) util_clamp: min: 0 max: 0
> # uclampset -p $$ -m 42
> uclampset: sched_setattr for pid 48: Not supported
> # uclampset -p $$ -M 42
> uclampset: sched_setattr for pid 48: Not supported
> # cat /proc/version
> Linux version 5.16.0-rc3 (landley@driftwood) (x86_64-linux-musl-gcc (GCC)
> 9.2.01
>
> Do I need a kernel config entry or something?
>

i think so, yes. i got similar results to you on my laptop, but running on
an x86-64 [virtual] device everything worked [to the extent that "i got
back the values i put in, and plausible looking numbers from other stuff"].

i'm guessing it's these?

vsoc_x86_64:/# gzip -dc /proc/config.gz | grep UCLAMP
CONFIG_UCLAMP_TASK=y
CONFIG_UCLAMP_BUCKETS_COUNT=20
CONFIG_UCLAMP_TASK_GROUP=y

(i don't have /proc/config.gz on my laptop though, so i can't check.)

Rob
>
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] Add uclampset(1).

2021-12-10 Thread Rob Landley 
On 12/9/21 7:31 PM, enh via Toybox wrote:
> No standard, but part of util-linux.

Don't default y in pending.

USE_TASKSET(NEWTOY(uclampset)) doesn't match.

Does this require root?

>From a UI perspective, do we ever _not_ want -a?

Sigh. It's HARD to come up with a good terse description of -R.

$ grep syscall\.h toys/*/*.c
toys/other/chrt.c:#include 
toys/other/insmod.c:#include 
toys/other/ionice.c:#include 
toys/other/nsenter.c:#include 
toys/other/pivot_root.c:#include 
toys/other/readahead.c:#include 
toys/other/rmmod.c:#include 
toys/other/taskset.c:#include 
toys/pending/modprobe.c:#include 
toys/pending/strace.c:#include 
toys/pending/uclampset.c:#include 

Is there a reason we haven't got sys/syscall.h in toys.h? Sure, it's not in
posix but it apparently originated in BSD:

https://www.freebsd.org/cgi/man.cgi?query=syscall=2=html

So the header should at least _exist_ there, and including it not break the 
build?

Ah. My devauan kernel is 4.19+fixes, so this goes boing when I run it. Luckily,
I created mkroot... Hmmm... Except when I tried your version (without any of my
changes), it still went:

Type exit when done.
# uclampset
uclampset: Need -p PID or CMD [ARG...]
# uclampset -p $$
sh (48) util_clamp: min: 0 max: 0
# uclampset -p $$ -m 42
uclampset: sched_setattr for pid 48: Not supported
# uclampset -p $$ -M 42
uclampset: sched_setattr for pid 48: Not supported
# cat /proc/version
Linux version 5.16.0-rc3 (landley@driftwood) (x86_64-linux-musl-gcc (GCC) 9.2.01

Do I need a kernel config entry or something?

Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

[Toybox] [PATCH] Add uclampset(1).

2021-12-09 Thread enh via Toybox 
No standard, but part of util-linux.
---
 toys/pending/uclampset.c | 119 +++
 1 file changed, 119 insertions(+)
 create mode 100644 toys/pending/uclampset.c
From 9ddf2d65915fd1a4fe3f602292bbbae71b9bdb5a Mon Sep 17 00:00:00 2001
From: Elliott Hughes 
Date: Thu, 9 Dec 2021 17:29:03 -0800
Subject: [PATCH] Add uclampset(1).

No standard, but part of util-linux.
---
 toys/pending/uclampset.c | 119 +++
 1 file changed, 119 insertions(+)
 create mode 100644 toys/pending/uclampset.c

diff --git a/toys/pending/uclampset.c b/toys/pending/uclampset.c
new file mode 100644
index ..e59f0d33
--- /dev/null
+++ b/toys/pending/uclampset.c
@@ -0,0 +1,119 @@
+/* uclampset.c - Set/get processes' utilization clamping attributes.
+ *
+ * Copyright 2021 The Android Open Source Project
+ *
+ * No standard. See https://man7.org/linux/man-pages/man1/uclampset.1.html
+
+USE_TASKSET(NEWTOY(uclampset, "p#am#M#R", TOYFLAG_USR|TOYFLAG_BIN|TOYFLAG_STAYROOT))
+
+config UCLAMPSET
+  bool "uclampset"
+  default y
+  help
+usage: uclampset [-p PID] [-m MIN] [-M MAX] [CMD [ARG...]]
+
+Set/get utilization clamping attributes for new or existing processes.
+
+-a	Apply to all the tasks/threads for the given PID
+-m MIN	Set the minimum
+-M MAX	Set the maximum
+-p PID	Apply to PID rather than launching a command
+-R	Set SCHED_FLAG_RESET_ON_FORK
+*/
+
+#define FOR_uclampset
+#include "toys.h"
+
+GLOBALS(
+  long M, m, p;
+
+  int set;
+)
+
+#include 
+// The uclamp flags and fields in struct sched_attr were added to the kernel
+// in 2019, so we can't rely on the headers yet.
+#define SCHED_FLAG_RESET_ON_FORK 0x01
+#define SCHED_FLAG_KEEP_POLICY 0x08
+#define SCHED_FLAG_KEEP_PARAMS 0x10
+#define SCHED_FLAG_UTIL_CLAMP_MIN 0x20
+#define SCHED_FLAG_UTIL_CLAMP_MAX 0x40
+#define sched_getattr(pid, attr) \
+  syscall(__NR_sched_getattr, pid, attr, sizeof(struct sched_attr_v2), 0)
+#define sched_setattr(pid, attr, flags) \
+  syscall(__NR_sched_setattr, pid, attr, flags)
+struct sched_attr_v2 {
+  unsigned size;
+  unsigned sched_policy;
+  unsigned long long sched_flags;
+  int sched_nice;
+  unsigned sched_priority;
+  unsigned long long sched_runtime;
+  unsigned long long sched_deadline;
+  unsigned long long sched_period;
+  // These fields aren't in v1 of the struct.
+  unsigned sched_util_min;
+  unsigned sched_util_max;
+};
+
+static void do_uclampset(pid_t pid)
+{
+  struct sched_attr_v2 *sa = (struct sched_attr_v2 *)toybuf;
+
+  if (TT.set) {
+if (sched_setattr(pid, sa, 0)) perror_exit("sched_setattr for pid %d", pid);
+  } else {
+char path[33], *comm;
+size_t len;
+
+if (sched_getattr(pid, sa)) perror_exit("sched_getattr for pid %d", pid);
+sprintf(path, "/proc/%u/comm", pid);
+comm = xreadfile(path, 0, 0);
+len = strlen(comm);
+if (comm[len-1] == '\n') comm[len-1] = 0;
+printf("%s (%d) util_clamp: min: %u max: %u\n", comm, pid,
+   sa->sched_util_min, sa->sched_util_max);
+free(comm);
+  }
+}
+
+static int task_callback(struct dirtree *new)
+{
+  if (!new->parent) return DIRTREE_RECURSE;
+  if (isdigit(*new->name)) do_uclampset(atoi(new->name));
+
+  return 0;
+}
+
+void uclampset_main(void)
+{
+  struct sched_attr_v2* sa = (struct sched_attr_v2 *)toybuf;
+
+  memset(sa, 0, sizeof(*sa));
+  sa->size = sizeof(*sa);
+  sa->sched_flags = SCHED_FLAG_KEEP_POLICY | SCHED_FLAG_KEEP_PARAMS;
+  if (FLAG(R)) {
+sa->sched_flags |= SCHED_FLAG_RESET_ON_FORK;
+TT.set = 1;
+  }
+  if (FLAG(m)) {
+sa->sched_flags |= SCHED_FLAG_UTIL_CLAMP_MIN;
+sa->sched_util_min = TT.m;
+TT.set = 1;
+  }
+  if (FLAG(M)) {
+sa->sched_flags |= SCHED_FLAG_UTIL_CLAMP_MAX;
+sa->sched_util_max = TT.M;
+TT.set = 1;
+  }
+
+  if (!FLAG(p)) {
+if (toys.optc < 1) error_exit("Need -p PID or CMD [ARG...]");
+do_uclampset(getpid());
+xexec(toys.optargs);
+  } else if (FLAG(a)) {
+char buf[33];
+sprintf(buf, "/proc/%ld/task/", TT.p);
+dirtree_read(buf, task_callback);
+  } else do_uclampset(TT.p);
+}
-- 
2.34.1.173.g76aa8bc2d0-goog

___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net