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

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
>>
>