[PATCH v2 1/2] sched_setattr: Break EINVAL into separate blocks

[PATCH v2 1/2] sched_setattr: Break EINVAL into separate blocks

[Brian Norris]

The current EINVAL description has a complex boolean expression, and I'm
planning to add one more condition to it. Let's separate it into
different EINVAL descriptions to make it easier to read. Tweak the
punctuation a bit too while I'm at it.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2:
 * split out from patch 2 and reworked

 man/man2/sched_setattr.2 | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/man/man2/sched_setattr.2 b/man/man2/sched_setattr.2
index 5d7061bd6e3a..0c866a786748 100644
--- a/man/man2/sched_setattr.2
+++ b/man/man2/sched_setattr.2
@@ -349,16 +349,21 @@ .SH ERRORS
 .TP
 .B EINVAL
 .I attr.sched_policy
-is not one of the recognized policies;
+is not one of the recognized policies.
+.TP
+.B EINVAL
 .I attr.sched_flags
 contains a flag other than
-.BR SCHED_FLAG_RESET_ON_FORK ;
-or
+.BR SCHED_FLAG_RESET_ON_FORK .
+.TP
+.B EINVAL
 .I attr.sched_priority
-is invalid; or
+is invalid.
+.TP
+.B EINVAL
 .I attr.sched_policy
 is
-.B SCHED_DEADLINE
+.BR SCHED_DEADLINE ,
 and the deadline scheduling parameters in
 .I attr
 are invalid.
-- 
2.45.2.505.gda0bf45e8d-goog

[PATCH v2 2/2] sched_setattr.2: Document sched_util_{min,max}

[Brian Norris]

Utilization attributes were added in Linux v5.3 via commit a509a7cd7974
("sched/uclamp: Extend sched_setattr() to support utilization
clamping"). Borrow some documentation from there, with a bit of
editorial trimming and augmentation.

The "reset" (-1 / UINT32_MAX) behavior was added in Linux 5.11 via
commit 480a6ca2dc6e ("sched/uclamp: Allow to reset a task uclamp
constraint value").

Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2:
 * address various style, linter review comments

 man/man2/sched_setattr.2 | 67 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/man/man2/sched_setattr.2 b/man/man2/sched_setattr.2
index 0c866a786748..c3b2cc5be6f1 100644
--- a/man/man2/sched_setattr.2
+++ b/man/man2/sched_setattr.2
@@ -94,10 +94,14 @@ .SS sched_setattr()
                               SCHED_BATCH) */
     u32 sched_priority;    /* Static priority (SCHED_FIFO,
                               SCHED_RR) */
-    /* Remaining fields are for SCHED_DEADLINE */
+    /* For SCHED_DEADLINE */
     u64 sched_runtime;
     u64 sched_deadline;
     u64 sched_period;
+\&
+    /* Utilization hints */
+    u32 sched_util_min;
+    u32 sched_util_max;
 };
 .EE
 .in
@@ -186,6 +190,23 @@ .SS sched_setattr()
 On the other hand, if the process-directed signal is delivered to
 a thread inside the process other than the one that had a run-time overrun,
 the application has no way of knowing which thread overran.
+.TP
+.B SCHED_FLAG_UTIL_CLAMP_MIN
+.TQ
+.BR SCHED_FLAG_UTIL_CLAMP_MAX " (both since Linux 5.3)"
+These flags indicate that the
+.I
+sched_util_min
+or
+.I
+sched_util_max
+fields, respectively, are present,
+representing the expected minimum and maximum utilization of the thread.
+.IP
+The utilization attributes provide the scheduler with boundaries
+within which it should schedule the thread,
+potentially informing its decisions
+regarding task placement and frequency selection.
 .RE
 .TP
 .I sched_nice
@@ -228,6 +249,33 @@ .SS sched_setattr()
 .I sched_period
 This field specifies the "Period" parameter for deadline scheduling.
 The value is expressed in nanoseconds.
+.TP
+.IR sched_util_min ", " sched_util_max " (since Linux 5.3)"
+These fields specify the expected minimum and maximum utilization, respectively.
+They are ignored unless their corresponding
+.I SCHED_FLAG_UTIL_CLAMP_MIN
+or
+.I SCHED_FLAG_UTIL_CLAMP_MAX
+are set in
+.IR sched_flags .
+.IP
+Utilization is a value in the range [0, 1024], representing the percentage of
+CPU time used by a task when running at the maximum frequency on the highest
+capacity CPU of the system.
+This is a fixed point representation, where 1024 corresponds to 100%, and 0
+corresponds to 0%.
+For example, a 20% utilization task is a task running for 2ms every 10ms at
+maximum frequency and is represented by a utilization value of
+0.2 * 1024 = 205.
+.IP
+A task with a minimum utilization value larger than 0 is more likely scheduled
+on a CPU with a capacity big enough to fit the specified value.
+A task with a maximum utilization value smaller than 1024 is more likely
+scheduled on a CPU with no more capacity than the specified value.
+.IP
+A task utilization boundary can be reset by setting its field to
+.B UINT32_MAX
+(new in Linux 5.11).
 .P
 The
 .I flags
@@ -368,6 +416,23 @@ .SH ERRORS
 .I attr
 are invalid.
 .TP
+.B EINVAL
+.I attr.sched_flags
+contains
+.B SCHED_FLAG_UTIL_CLAMP_MIN
+or
+.BR SCHED_FLAG_UTIL_CLAMP_MAX ,
+and
+.I attr.sched_util_min
+or
+.I attr.sched_util_max
+are out of bounds.
+.TP
+.B EOPNOTSUPP
+SCHED_FLAG_UTIL_CLAMP was provided, but the kernel was not built with
+.B CONFIG_UCLAMP_TASK
+support.
+.TP
 .B EPERM
 The caller does not have appropriate privileges.
 .TP
-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH v2 2/2] sched_setattr.2: Document sched_util_{min,max}

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 5105 bytes --]

Hi Brian,

On Wed, Jun 12, 2024 at 01:44:53PM GMT, Brian Norris wrote:
> Utilization attributes were added in Linux v5.3 via commit a509a7cd7974
> ("sched/uclamp: Extend sched_setattr() to support utilization
> clamping"). Borrow some documentation from there, with a bit of
> editorial trimming and augmentation.
> 
> The "reset" (-1 / UINT32_MAX) behavior was added in Linux 5.11 via
> commit 480a6ca2dc6e ("sched/uclamp: Allow to reset a task uclamp
> constraint value").
> 
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Patrick Bellasi <patrick.bellasi@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> v2:
>  * address various style, linter review comments
> 
>  man/man2/sched_setattr.2 | 67 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/man/man2/sched_setattr.2 b/man/man2/sched_setattr.2
> index 0c866a786748..c3b2cc5be6f1 100644
> --- a/man/man2/sched_setattr.2
> +++ b/man/man2/sched_setattr.2
> @@ -94,10 +94,14 @@ .SS sched_setattr()
>                                SCHED_BATCH) */
>      u32 sched_priority;    /* Static priority (SCHED_FIFO,
>                                SCHED_RR) */
> -    /* Remaining fields are for SCHED_DEADLINE */
> +    /* For SCHED_DEADLINE */
>      u64 sched_runtime;
>      u64 sched_deadline;
>      u64 sched_period;
> +\&
> +    /* Utilization hints */
> +    u32 sched_util_min;
> +    u32 sched_util_max;
>  };
>  .EE
>  .in
> @@ -186,6 +190,23 @@ .SS sched_setattr()
>  On the other hand, if the process-directed signal is delivered to
>  a thread inside the process other than the one that had a run-time overrun,
>  the application has no way of knowing which thread overran.
> +.TP
> +.B SCHED_FLAG_UTIL_CLAMP_MIN
> +.TQ
> +.BR SCHED_FLAG_UTIL_CLAMP_MAX " (both since Linux 5.3)"
> +These flags indicate that the
> +.I
> +sched_util_min
> +or
> +.I
> +sched_util_max
> +fields, respectively, are present,
> +representing the expected minimum and maximum utilization of the thread.
> +.IP
> +The utilization attributes provide the scheduler with boundaries
> +within which it should schedule the thread,
> +potentially informing its decisions
> +regarding task placement and frequency selection.
>  .RE
>  .TP
>  .I sched_nice
> @@ -228,6 +249,33 @@ .SS sched_setattr()
>  .I sched_period
>  This field specifies the "Period" parameter for deadline scheduling.
>  The value is expressed in nanoseconds.
> +.TP
> +.IR sched_util_min ", " sched_util_max " (since Linux 5.3)"

This should use .TP and .TQ as above.

> +These fields specify the expected minimum and maximum utilization, respectively.
> +They are ignored unless their corresponding
> +.I SCHED_FLAG_UTIL_CLAMP_MIN
> +or
> +.I SCHED_FLAG_UTIL_CLAMP_MAX
> +are set in

With 'or', here corresponds 'is', not 'are'.  For 'are' we'd need 'and',
to get a plural.

> +.IR sched_flags .
> +.IP
> +Utilization is a value in the range [0, 1024], representing the percentage of

Break after ','.

> +CPU time used by a task when running at the maximum frequency on the highest
> +capacity CPU of the system.
> +This is a fixed point representation, where 1024 corresponds to 100%, and 0

Break after ','.

> +corresponds to 0%.
> +For example, a 20% utilization task is a task running for 2ms every 10ms at

Break after ',', and after 'is'.

> +maximum frequency and is represented by a utilization value of
> +0.2 * 1024 = 205.

This should go in italics.

$ MANWIDTH=72 man man-pages | sed -n '/Expressions/,/^$/p'
     Expressions, if not written on a separate indented line, should be
     specified in italics.  Again, the use of nonbreaking spaces may be
     appropriate if the expression is inlined with normal text.


> +.IP
> +A task with a minimum utilization value larger than 0 is more likely scheduled
> +on a CPU with a capacity big enough to fit the specified value.
> +A task with a maximum utilization value smaller than 1024 is more likely
> +scheduled on a CPU with no more capacity than the specified value.

Please break both sentences at consistent points, since they have
symmetric wording.

> +.IP
> +A task utilization boundary can be reset by setting its field to
> +.B UINT32_MAX
> +(new in Linux 5.11).

s/new in/since/

>  .P
>  The
>  .I flags
> @@ -368,6 +416,23 @@ .SH ERRORS
>  .I attr
>  are invalid.
>  .TP
> +.B EINVAL
> +.I attr.sched_flags
> +contains
> +.B SCHED_FLAG_UTIL_CLAMP_MIN
> +or
> +.BR SCHED_FLAG_UTIL_CLAMP_MAX ,
> +and
> +.I attr.sched_util_min
> +or
> +.I attr.sched_util_max
> +are out of bounds.
> +.TP
> +.B EOPNOTSUPP
> +SCHED_FLAG_UTIL_CLAMP was provided, but the kernel was not built with

Missing bold.

Have a lovely day!
Alex

> +.B CONFIG_UCLAMP_TASK
> +support.
> +.TP
>  .B EPERM
>  The caller does not have appropriate privileges.
>  .TP
> -- 
> 2.45.2.505.gda0bf45e8d-goog
> 
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/2] sched_setattr.2: Document sched_util_{min,max}

[Brian Norris]

On Thu, Jun 13, 2024 at 12:15:55AM +0200, Alejandro Colomar wrote:
> On Wed, Jun 12, 2024 at 01:44:53PM GMT, Brian Norris wrote:
> > +maximum frequency and is represented by a utilization value of
> > +0.2 * 1024 = 205.
> 
> This should go in italics.
> 
> $ MANWIDTH=72 man man-pages | sed -n '/Expressions/,/^$/p'
>      Expressions, if not written on a separate indented line, should be
>      specified in italics.  Again, the use of nonbreaking spaces may be
>      appropriate if the expression is inlined with normal text.

Regarding the nonbreaking spaces part: I'm not too practiced with groff,
but the wording around the \[ti] character is confusing. AFAICT, I
actually need to spell my expression like the following?

0.2\~*\~1024\~=\~205

At least, with trial and error, that's what appears to render correctly,
whereas \[ti] renders as a literal tilde. Is man-pages.7 [1] incorrect?

Brian

[1] from `man man-pages`:
      "In this case, it may be worth using nonbreaking spaces (\[ti]) at
      suitable places in the command."

Re: [PATCH v2 2/2] sched_setattr.2: Document sched_util_{min,max}

[G. Branden Robinson]

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

Hi Brian,

At 2024-06-13T13:29:44-0700, Brian Norris wrote:
> Regarding the nonbreaking spaces part: I'm not too practiced with
> groff, but the wording around the \[ti] character is confusing.
> AFAICT, I actually need to spell my expression like the following?
> 
> 0.2\~*\~1024\~=\~205

Yes, that puts nonbreaking (but adjustable) spaces between the numeric
literals and operators.

> At least, with trial and error, that's what appears to render correctly,
> whereas \[ti] renders as a literal tilde.

Yes.

> Is man-pages.7 [1] incorrect?
[...]
> [1] from `man man-pages`:
>       "In this case, it may be worth using nonbreaking spaces (\[ti]) at
>       suitable places in the command."

It looks that way.

I suspect that, in the man-pages.7 source, something much like...

nonbreaking spaces
.RB ( \[rs]\[ti] )
at suitable places

...was meant.

Regards,
Branden

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/2] sched_setattr.2: Document sched_util_{min,max}

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]

Hi Brian,

On Thu, Jun 13, 2024 at 01:29:44PM GMT, Brian Norris wrote:
> On Thu, Jun 13, 2024 at 12:15:55AM +0200, Alejandro Colomar wrote:
> > On Wed, Jun 12, 2024 at 01:44:53PM GMT, Brian Norris wrote:
> > > +maximum frequency and is represented by a utilization value of
> > > +0.2 * 1024 = 205.
> > 
> > This should go in italics.
> > 
> > $ MANWIDTH=72 man man-pages | sed -n '/Expressions/,/^$/p'
> >      Expressions, if not written on a separate indented line, should be
> >      specified in italics.  Again, the use of nonbreaking spaces may be
> >      appropriate if the expression is inlined with normal text.
> 
> Regarding the nonbreaking spaces part: I'm not too practiced with groff,
> but the wording around the \[ti] character is confusing. AFAICT, I
> actually need to spell my expression like the following?
> 
> 0.2\~*\~1024\~=\~205
> 
> At least, with trial and error, that's what appears to render correctly,
> whereas \[ti] renders as a literal tilde. Is man-pages.7 [1] incorrect?
> 
> Brian
> 
> [1] from `man man-pages`:
>       "In this case, it may be worth using nonbreaking spaces (\[ti]) at
>       suitable places in the command."

Yup; that's a bug.  I introduced it by accident in

commit 36f73ba37945c7ff4aa2d8383f831519a38e3f27
Author: Alejandro Colomar <alx@kernel.org>
Date:   Sun Feb 5 22:59:22 2023 +0100

    man-pages.7: Recommend using \[..] instead of \(.. escapes
    
    They are more readable.
    
    Signed-off-by: Alejandro Colomar <alx@kernel.org>


Which did:

-(\e\(ti) at suitable places in the command.
+(\e[ti]) at suitable places in the command.

but it should have been (\e\[ti]).  The intention was to render as (\~).
Would you mind sending a fix as part of your patch set?

Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/2] sched_setattr: Break EINVAL into separate blocks

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

Hi Brian,

On Wed, Jun 12, 2024 at 01:44:52PM GMT, Brian Norris wrote:
> The current EINVAL description has a complex boolean expression, and I'm
> planning to add one more condition to it. Let's separate it into
> different EINVAL descriptions to make it easier to read. Tweak the
> punctuation a bit too while I'm at it.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Patch applied.  Thanks!

Have a lovely night!
Alex

> ---
> v2:
>  * split out from patch 2 and reworked
> 
>  man/man2/sched_setattr.2 | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/man/man2/sched_setattr.2 b/man/man2/sched_setattr.2
> index 5d7061bd6e3a..0c866a786748 100644
> --- a/man/man2/sched_setattr.2
> +++ b/man/man2/sched_setattr.2
> @@ -349,16 +349,21 @@ .SH ERRORS
>  .TP
>  .B EINVAL
>  .I attr.sched_policy
> -is not one of the recognized policies;
> +is not one of the recognized policies.
> +.TP
> +.B EINVAL
>  .I attr.sched_flags
>  contains a flag other than
> -.BR SCHED_FLAG_RESET_ON_FORK ;
> -or
> +.BR SCHED_FLAG_RESET_ON_FORK .
> +.TP
> +.B EINVAL
>  .I attr.sched_priority
> -is invalid; or
> +is invalid.
> +.TP
> +.B EINVAL
>  .I attr.sched_policy
>  is
> -.B SCHED_DEADLINE
> +.BR SCHED_DEADLINE ,
>  and the deadline scheduling parameters in
>  .I attr
>  are invalid.
> -- 
> 2.45.2.505.gda0bf45e8d-goog
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]