[PATCH v2] chrt: Make priority optional for policies that don't use it

[PATCH v2] chrt: Make priority optional for policies that don't use it

[Madadi Vineeth Reddy]

Currently, chrt requires a priority argument even for scheduling
policies like SCHED_OTHER and SCHED_BATCH, which ignore it.

This change relaxes that requirement. Now, priority is only expected
for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
is set internally and no argument is required on the command line.

This simplifies usage when modifying runtime parameters like
--sched-runtime for non-realtime tasks.

For example, to change the EEVDF tunable base_slice, one currently
needs to run:
chrt -v -o -T 1000000 -p 0 $PID

Passing '0' after -p is not intutive and not required as priority is
not applicable to SCHED_OTHER tasks. Now with this patch, one can do:
chrt -v -o -T 1000000 -p $PID

Passing '0' still works ensuring ABI doesn't break.

Signed-off-by: Madadi Vineeth Reddy <vineethr@linux.ibm.com>
---
Changes in v2:
- Updated the man page to reflect the optional priority behavior (Karel Zak)
- Renamed variable to 'need_prio' (Karel Zak)

Signed-off-by: Madadi Vineeth Reddy <vineethr@linux.ibm.com>
---
 schedutils/chrt.1.adoc | 12 ++++++++----
 schedutils/chrt.c      | 16 +++++++++++-----
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/schedutils/chrt.1.adoc b/schedutils/chrt.1.adoc
index 77add535d..4f419b5f6 100644
--- a/schedutils/chrt.1.adoc
+++ b/schedutils/chrt.1.adoc
@@ -58,16 +58,16 @@ Set scheduling policy to *SCHED_FIFO* (first in-first out).
 Set scheduling policy to *SCHED_RR* (round-robin scheduling). When no policy is defined, the *SCHED_RR* is used as the default.
 
 *-b*, *--batch*::
-Set scheduling policy to *SCHED_BATCH* (scheduling batch processes). Linux-specific, supported since 2.6.16. The priority argument has to be set to zero.
+Set scheduling policy to *SCHED_BATCH* (scheduling batch processes). Linux-specific, supported since 2.6.16. Since util-linux v2.42, the priority argument is optional; if specified, it must be set to zero.
 
 *-i*, *--idle*::
-Set scheduling policy to *SCHED_IDLE* (scheduling very low priority jobs). Linux-specific, supported since 2.6.23. The priority argument has to be set to zero.
+Set scheduling policy to *SCHED_IDLE* (scheduling very low priority jobs). Linux-specific, supported since 2.6.23. Since util-linux v2.42, the priority argument is optional; if specified, it must be set to zero.
 
 *-d*, *--deadline*::
-Set scheduling policy to *SCHED_DEADLINE* (sporadic task model deadline scheduling). Linux-specific, supported since 3.14. The priority argument has to be set to zero. See also *--sched-runtime*, *--sched-deadline* and *--sched-period*. The relation between the options required by the kernel is runtime <= deadline <= period. *chrt* copies _period_ to _deadline_ if *--sched-deadline* is not specified and _deadline_ to _runtime_ if *--sched-runtime* is not specified. It means that at least *--sched-period* has to be specified. See *sched*(7) for more details.
+Set scheduling policy to *SCHED_DEADLINE* (sporadic task model deadline scheduling). Linux-specific, supported since 3.14. Since util-linux v2.42, the priority argument is optional; if specified, it must be set to zero. See also *--sched-runtime*, *--sched-deadline* and *--sched-period*. The relation between the options required by the kernel is runtime <= deadline <= period. *chrt* copies _period_ to _deadline_ if *--sched-deadline* is not specified and _deadline_ to _runtime_ if *--sched-runtime* is not specified. It means that at least *--sched-period* has to be specified. See *sched*(7) for more details.
 
 *-d*, *--ext*::
-Set scheduling policy to *SCHED_EXT* (BPF program-defined scheduling). Linux-specific, supported since 6.12. The priority argument has to be set to zero.
+Set scheduling policy to *SCHED_EXT* (BPF program-defined scheduling). Linux-specific, supported since 6.12. Since util-linux v2.42, the priority argument is optional; if specified, it must be set to zero.
 
 == SCHEDULING OPTIONS
 
@@ -132,6 +132,10 @@ Reset priorities to default for a process{colon}::
 ____
 *chrt -o -p 0* _PID_
 ____
+Set a custom slice of 1 ms for a SCHED_OTHER task (priority is optional for policies other than SCHED_FIFO and SCHED_RR){colon}::
+____
+*chrt -o -T 1000000 -p* _PID_
+____
 See *sched*(7) for a detailed discussion of the different scheduler classes and how they interact.
 
 == PERMISSIONS
diff --git a/schedutils/chrt.c b/schedutils/chrt.c
index cf99935dc..eb1717acc 100644
--- a/schedutils/chrt.c
+++ b/schedutils/chrt.c
@@ -495,20 +495,26 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (((ctl->pid > -1) && argc - optind < 1) ||
-	    ((ctl->pid == -1) && argc - optind < 2)) {
+	bool need_prio = (ctl->policy == SCHED_FIFO || ctl->policy == SCHED_RR);
+
+	if (((ctl->pid > -1) && argc - optind < (need_prio ? 1 : 0)) ||
+	    ((ctl->pid == -1) && argc - optind < (need_prio ? 2 : 1))) {
 		warnx(_("bad usage"));
 		errtryhelp(EXIT_FAILURE);
 }
 
-	if ((ctl->pid > -1) && (ctl->verbose || argc - optind == 1)) {
+	if ((ctl->pid > -1) && (ctl->verbose || argc - optind == (need_prio ? 1 : 0))) {
 		show_sched_info(ctl);
-		if (argc - optind == 1)
+		if (argc - optind == (need_prio ? 1 : 0))
 			return EXIT_SUCCESS;
 	}
 
 	errno = 0;
-	ctl->priority = strtos32_or_err(argv[optind], _("invalid priority argument"));
+
+	if (need_prio || argc - optind == 2)
+		ctl->priority = strtos32_or_err(argv[optind], _("invalid priority argument"));
+	else
+		ctl->priority = 0;
 
 	if (ctl->runtime && !supports_runtime_param(ctl->policy))
 		errx(EXIT_FAILURE, _("--sched-runtime option is supported for %s"),
-- 
2.49.0

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Benno Schulenberg]

[-- Attachment #1.1: Type: text/plain, Size: 974 bytes --]


Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
> Currently, chrt requires a priority argument even for scheduling
> policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
> 
> This change relaxes that requirement. Now, priority is only expected
> for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
> is set internally and no argument is required on the command line.

Doesn't this alter the "show-the-current-policy-and-priority" behavior
when no priority is given?  Currently `./chrt --help` says (trimmed):

   Set policy:
    chrt [options] --pid <priority> <pid>

   Get policy:
    chrt [options] -p <pid>


Without the proposed change, running `chrt --other --pid $$` says:

   pid 1427's current scheduling policy: SCHED_OTHER
   pid 1427's current scheduling priority: 0

After the change, that same command outputs nothing.  Maybe that is
fine, but it would require some adjustment of the docs.


Benno


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Karel Zak]

On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
> 
> Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
> > Currently, chrt requires a priority argument even for scheduling
> > policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
> > 
> > This change relaxes that requirement. Now, priority is only expected
> > for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
> > is set internally and no argument is required on the command line.
> 
> Doesn't this alter the "show-the-current-policy-and-priority" behavior
> when no priority is given?  Currently `./chrt --help` says (trimmed):
 
Very good point. The priority policy (--{other,...}) should be
required to ensure that the user wants to alter the setting rather
than print the current situation. Madadi, what do you think?

>   Set policy:
>    chrt [options] --pid <priority> <pid>
> 
>   Get policy:
>    chrt [options] -p <pid>

I really don't like the use of "-p." We should use "--pid" everywhere  
(in --help, man page, and examples).

> Without the proposed change, running `chrt --other --pid $$` says:
> 
>   pid 1427's current scheduling policy: SCHED_OTHER
>   pid 1427's current scheduling priority: 0
> 
> After the change, that same command outputs nothing.  Maybe that is
> fine, but it would require some adjustment of the docs.

This is bug.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Karel Zak]

On Wed, Jun 18, 2025 at 11:18:18AM +0200, Karel Zak wrote:
> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
> > 
> > Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
> > > Currently, chrt requires a priority argument even for scheduling
> > > policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
> > > 
> > > This change relaxes that requirement. Now, priority is only expected
> > > for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
> > > is set internally and no argument is required on the command line.
> > 
> > Doesn't this alter the "show-the-current-policy-and-priority" behavior
> > when no priority is given?  Currently `./chrt --help` says (trimmed):
>  
> Very good point. The priority policy (--{other,...}) should be
> required to ensure that the user wants to alter the setting rather
> than print the current situation. Madadi, what do you think?

Ah, I now read Benno's note more carefully. The code just silently ignores  
policy when priority is not specified.

$ chrt --fifo --pid $$  
pid 994013's current scheduling policy: SCHED_OTHER  
pid 994013's current scheduling priority: 0

This is ugly. The question is how important it is to support this for  
backward compatibility. I'd assume that users use "chrt --pid $$" to get  
the current setting.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Madadi Vineeth Reddy]

Hi Benno, Karel

On 18/06/25 14:55, Karel Zak wrote:
> On Wed, Jun 18, 2025 at 11:18:18AM +0200, Karel Zak wrote:
>> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
>>>
>>> Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
>>>> Currently, chrt requires a priority argument even for scheduling
>>>> policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
>>>>
>>>> This change relaxes that requirement. Now, priority is only expected
>>>> for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
>>>> is set internally and no argument is required on the command line.
>>>
>>> Doesn't this alter the "show-the-current-policy-and-priority" behavior
>>> when no priority is given?  Currently `./chrt --help` says (trimmed):
>>  
>> Very good point. The priority policy (--{other,...}) should be
>> required to ensure that the user wants to alter the setting rather
>> than print the current situation. Madadi, what do you think?
> 
> Ah, I now read Benno's note more carefully. The code just silently ignores  
> policy when priority is not specified.
> 
> $ chrt --fifo --pid $$  
> pid 994013's current scheduling policy: SCHED_OTHER  
> pid 994013's current scheduling priority: 0
> 
> This is ugly. The question is how important it is to support this for  
> backward compatibility. I'd assume that users use "chrt --pid $$" to get  
> the current setting.
> 

chrt --pid 20570
pid 20570's current scheduling policy: SCHED_OTHER
pid 20570's current scheduling priority: 0
pid 20570's current runtime parameter: 2800000

After this patch also, we still get the current setting. Can you give it
a try with the patch applied? Let me know if I am missing something.

Thanks for taking a look.

Thanks,
Madadi Vineeth Reddy

>     Karel
> 

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Madadi Vineeth Reddy]

On 18/06/25 15:04, Madadi Vineeth Reddy wrote:
> Hi Benno, Karel
> 
> On 18/06/25 14:55, Karel Zak wrote:
>> On Wed, Jun 18, 2025 at 11:18:18AM +0200, Karel Zak wrote:
>>> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
>>>>
>>>> Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
>>>>> Currently, chrt requires a priority argument even for scheduling
>>>>> policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
>>>>>
>>>>> This change relaxes that requirement. Now, priority is only expected
>>>>> for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
>>>>> is set internally and no argument is required on the command line.
>>>>
>>>> Doesn't this alter the "show-the-current-policy-and-priority" behavior
>>>> when no priority is given?  Currently `./chrt --help` says (trimmed):
>>>  
>>> Very good point. The priority policy (--{other,...}) should be
>>> required to ensure that the user wants to alter the setting rather
>>> than print the current situation. Madadi, what do you think?
>>
>> Ah, I now read Benno's note more carefully. The code just silently ignores  
>> policy when priority is not specified.
>>
>> $ chrt --fifo --pid $$  
>> pid 994013's current scheduling policy: SCHED_OTHER  
>> pid 994013's current scheduling priority: 0
>>
>> This is ugly. The question is how important it is to support this for  
>> backward compatibility. I'd assume that users use "chrt --pid $$" to get  
>> the current setting.
>>
> 
> chrt --pid 20570
> pid 20570's current scheduling policy: SCHED_OTHER
> pid 20570's current scheduling priority: 0
> pid 20570's current runtime parameter: 2800000
> 
> After this patch also, we still get the current setting. Can you give it
> a try with the patch applied? Let me know if I am missing something.
> 

Ah, I found my mistake. With this patch, getting the current settings only
works for SCHED_FIFO and SCHED_RR, but not for the other policies, because
we don’t print them here:

+	if ((ctl->pid > -1) && (ctl->verbose || argc - optind == (need_prio ? 1 : 0))) {
 		show_sched_info(ctl);

The condition argc - optind == (need_prio ? 1 : 0) becomes true only for
SCHED_FIFO and SCHED_RR, which causes the info to be printed only for those.

I’ll send a new version with the fix.

Thanks,
Madadi Vineeth Reddy

> Thanks for taking a look.
> 
> Thanks,
> Madadi Vineeth Reddy
> 
>>     Karel
>>
> 

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Karel Zak]

On Wed, Jun 18, 2025 at 03:04:40PM +0530, Madadi Vineeth Reddy wrote:
> Hi Benno, Karel
> 
> On 18/06/25 14:55, Karel Zak wrote:
> > On Wed, Jun 18, 2025 at 11:18:18AM +0200, Karel Zak wrote:
> >> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
> >>>
> >>> Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
> >>>> Currently, chrt requires a priority argument even for scheduling
> >>>> policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
> >>>>
> >>>> This change relaxes that requirement. Now, priority is only expected
> >>>> for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
> >>>> is set internally and no argument is required on the command line.
> >>>
> >>> Doesn't this alter the "show-the-current-policy-and-priority" behavior
> >>> when no priority is given?  Currently `./chrt --help` says (trimmed):
> >>  
> >> Very good point. The priority policy (--{other,...}) should be
> >> required to ensure that the user wants to alter the setting rather
> >> than print the current situation. Madadi, what do you think?
> > 
> > Ah, I now read Benno's note more carefully. The code just silently ignores  
> > policy when priority is not specified.
> > 
> > $ chrt --fifo --pid $$  
> > pid 994013's current scheduling policy: SCHED_OTHER  
> > pid 994013's current scheduling priority: 0
> > 
> > This is ugly. The question is how important it is to support this for  
> > backward compatibility. I'd assume that users use "chrt --pid $$" to get  
> > the current setting.
> > 
> 
> chrt --pid 20570
> pid 20570's current scheduling policy: SCHED_OTHER
> pid 20570's current scheduling priority: 0
> pid 20570's current runtime parameter: 2800000
> 
> After this patch also, we still get the current setting. Can you give it
> a try with the patch applied? Let me know if I am missing something.

Sorry for the mess. I have finally tried it (sorry for doing it the first time).

Benno's note is that "chrt --other --pid $$" changed behavior. The old version 
always just read the current setting if no priority was specified, and now it 
alters scheduling and prints nothing. I guess this is the expected behavior now.

I have found that the old version silently ignores the policy option if 
priority is not provided, and it always just displays the current setting. This 
is ugly, and with your change, it's more ugly :), because it depends on the 
specified policy.

  chrt --pid $$               # display current setting

  chrt --other --pid $$       # alter current setting
  chrt --fifo --pid $$        # display current setting (--fifo ignored)
  chrt --fifo --pid 10 $$     # alter current setting

I think with --{other,fifo,...} the command line should always be interpreted 
as a request to alter the current setting, independently of whether priority is 
specified.

  chrt --fifo --pid $$

needs to end with the error message "chrt: priority value for the policy 
required" rather than display the current setting.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Madadi Vineeth Reddy]

On 18/06/25 16:15, Karel Zak wrote:
> On Wed, Jun 18, 2025 at 03:04:40PM +0530, Madadi Vineeth Reddy wrote:
>> Hi Benno, Karel
>>
>> On 18/06/25 14:55, Karel Zak wrote:
>>> On Wed, Jun 18, 2025 at 11:18:18AM +0200, Karel Zak wrote:
>>>> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
>>>>>
>>>>> Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
>>>>>> Currently, chrt requires a priority argument even for scheduling
>>>>>> policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
>>>>>>
>>>>>> This change relaxes that requirement. Now, priority is only expected
>>>>>> for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
>>>>>> is set internally and no argument is required on the command line.
>>>>>
>>>>> Doesn't this alter the "show-the-current-policy-and-priority" behavior
>>>>> when no priority is given?  Currently `./chrt --help` says (trimmed):
>>>>  
>>>> Very good point. The priority policy (--{other,...}) should be
>>>> required to ensure that the user wants to alter the setting rather
>>>> than print the current situation. Madadi, what do you think?
>>>
>>> Ah, I now read Benno's note more carefully. The code just silently ignores  
>>> policy when priority is not specified.
>>>
>>> $ chrt --fifo --pid $$  
>>> pid 994013's current scheduling policy: SCHED_OTHER  
>>> pid 994013's current scheduling priority: 0
>>>
>>> This is ugly. The question is how important it is to support this for  
>>> backward compatibility. I'd assume that users use "chrt --pid $$" to get  
>>> the current setting.
>>>
>>
>> chrt --pid 20570
>> pid 20570's current scheduling policy: SCHED_OTHER
>> pid 20570's current scheduling priority: 0
>> pid 20570's current runtime parameter: 2800000
>>
>> After this patch also, we still get the current setting. Can you give it
>> a try with the patch applied? Let me know if I am missing something.
> 
> Sorry for the mess. I have finally tried it (sorry for doing it the first time).
> 
> Benno's note is that "chrt --other --pid $$" changed behavior. The old version 
> always just read the current setting if no priority was specified, and now it 
> alters scheduling and prints nothing. I guess this is the expected behavior now.
> 
> I have found that the old version silently ignores the policy option if 
> priority is not provided, and it always just displays the current setting. This 
> is ugly, and with your change, it's more ugly :), because it depends on the 
> specified policy.
> 
>   chrt --pid $$               # display current setting
> 
>   chrt --other --pid $$       # alter current setting
>   chrt --fifo --pid $$        # display current setting (--fifo ignored)
>   chrt --fifo --pid 10 $$     # alter current setting
> 
> I think with --{other,fifo,...} the command line should always be interpreted 
> as a request to alter the current setting, independently of whether priority is 
> specified.
> 
>   chrt --fifo --pid $$
> 
> needs to end with the error message "chrt: priority value for the policy 
> required" rather than display the current setting.

Agreed. So, IIUC, when using SCHED_FIFO or SCHED_RR, the priority should be
mandatory, and omitting it should result in an error. To simply print the
current settings, no scheduling policy should be specified, meaning
chrt --pid $$ should be the correct way to query the settings.

However, wouldn't this break existing behavior for users who may already be
using a command like chrt --other --pid $$ to display current settings?

As a first step, would it make sense to Keep the current behavior for
querying unchanged and just make priority optional for policies that
don't use it?

Maybe altering the existing behavior can be handled in a separate patch
with its own discussion.

What do you suggest?

Thanks,
Madadi Vineeth Reddy

> 
>     Karel
> 

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Madadi Vineeth Reddy]

On 18/06/25 14:48, Karel Zak wrote:
> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
>>
>> Op 17-06-2025 om 20:24 schreef Madadi Vineeth Reddy:
>>> Currently, chrt requires a priority argument even for scheduling
>>> policies like SCHED_OTHER and SCHED_BATCH, which ignore it.
>>>
>>> This change relaxes that requirement. Now, priority is only expected
>>> for SCHED_FIFO and SCHED_RR. For other policies, a default value of 0
>>> is set internally and no argument is required on the command line.
>>
>> Doesn't this alter the "show-the-current-policy-and-priority" behavior
>> when no priority is given?  Currently `./chrt --help` says (trimmed):
>  
> Very good point. The priority policy (--{other,...}) should be
> required to ensure that the user wants to alter the setting rather
> than print the current situation. Madadi, what do you think?
> 
>>   Set policy:
>>    chrt [options] --pid <priority> <pid>
>>
>>   Get policy:
>>    chrt [options] -p <pid>
> 
> I really don't like the use of "-p." We should use "--pid" everywhere  
> (in --help, man page, and examples).

I can spin off a separate patch to consistently use --pid instead of -p
in the help text, man page, and examples.

Thanks,
Madadi Vineeth Reddy

> 
>> Without the proposed change, running `chrt --other --pid $$` says:
>>
>>   pid 1427's current scheduling policy: SCHED_OTHER
>>   pid 1427's current scheduling priority: 0
>>
>> After the change, that same command outputs nothing.  Maybe that is
>> fine, but it would require some adjustment of the docs.
> 
> This is bug.
> 
>     Karel
> 

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Benno Schulenberg]

[-- Attachment #1.1: Type: text/plain, Size: 728 bytes --]


Op 18-06-2025 om 11:51 schreef Madadi Vineeth Reddy:
> On 18/06/25 14:48, Karel Zak wrote:
>> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg wrote:
>>>
>>>    Get policy:
>>>     chrt [options] -p <pid>
>>
>> I really don't like the use of "-p." We should use "--pid" everywhere
>> (in --help, man page, and examples).
> 
> I can spin off a separate patch to consistently use --pid instead of -p
> in the help text, man page, and examples.

It should also drop the "[options]" part from the above:

     Get policy:
       chrt --pid <pid>

As Karel says, specifying an option like --other or --fifo
should always be understood as a modification request, not
as an info request.


Benno


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Madadi Vineeth Reddy]

On 18/06/25 17:59, Benno Schulenberg wrote:
> 
> Op 18-06-2025 om 11:51 schreef Madadi Vineeth Reddy:
>> On 18/06/25 14:48, Karel Zak wrote:
>>> On Wed, Jun 18, 2025 at 10:18:29AM +0200, Benno Schulenberg
>>> wrote:
>>>> 
>>>> Get policy: chrt [options] -p <pid>
>>> 
>>> I really don't like the use of "-p." We should use "--pid"
>>> everywhere (in --help, man page, and examples).
>> 
>> I can spin off a separate patch to consistently use --pid instead
>> of -p in the help text, man page, and examples.
> 
> It should also drop the "[options]" part from the above:

Agreed.

> 
> Get policy: chrt --pid <pid>
> 
> As Karel says, specifying an option like --other or --fifo should
> always be understood as a modification request, not as an info
> request.

Yes, my only concern is ABI shouldn't break for existing users
who might be giving options and retrieving current settings.

So, my thought is just to make priority argument optional for
policies that don't use it and keep the existing behavior as is.

I can post another patch after this discussion that removes
"[options]" for querying current settings and the issues due
to changing existing behavior could be discussed there.

Thoughts?

Thanks,
Madadi Vineeth Reddy

> 
> 
> Benno
> 

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Benno Schulenberg]

[-- Attachment #1.1: Type: text/plain, Size: 922 bytes --]


Op 18-06-2025 om 18:41 schreef Madadi Vineeth Reddy:
> So, my thought is just to make priority argument optional for
> policies that don't use it and keep the existing behavior as is.

That's not possible.  Currently, doing something like

   ./chrt --idle --pid $$

just reports the current policy and priority, and does
not change the policy (because the priority is missing).
After your patch, doing the same command reports nothing
but _does_ change the policy.  You could still make it
report the current settings, but that would give the
wrong idea, because it would be the _old_ settings.

So, as Karel said, silently ignoring the policy option
when no priority was given was ugly.  Too ugly to keep
it that way.  As Karel implied: break compatibility and
do the sensible thing: whenever a policy option is given,
act upon it -- and when a needed priority is missing,
error out.


Benno


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2] chrt: Make priority optional for policies that don't use it

[Madadi Vineeth Reddy]

On 19/06/25 19:45, Benno Schulenberg wrote:
> 
> Op 18-06-2025 om 18:41 schreef Madadi Vineeth Reddy:
>> So, my thought is just to make priority argument optional for
>> policies that don't use it and keep the existing behavior as is.
> 
> That's not possible.  Currently, doing something like
> 
>   ./chrt --idle --pid $$
> 
> just reports the current policy and priority, and does
> not change the policy (because the priority is missing).
> After your patch, doing the same command reports nothing
> but _does_ change the policy.  You could still make it
> report the current settings, but that would give the
> wrong idea, because it would be the _old_ settings.
> 
> So, as Karel said, silently ignoring the policy option
> when no priority was given was ugly.  Too ugly to keep
> it that way.  As Karel implied: break compatibility and
> do the sensible thing: whenever a policy option is given,
> act upon it -- and when a needed priority is missing,
> error out.
> 

Sure, that makes sense. I will send a patch with these
changes accordingly.

Thanks,
Madadi Vineeth Reddy

> 
> Benno
>