linux-kernel - Re: [PATCH v8 1/8] x86/resctrl: Add multiple tasks to the resctrl group at once

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Sep 2023 13:42:46 -0700
From:   Fenghua Yu <fenghua.yu@...el.com>
To:     <babu.moger@....com>, <corbet@....net>,
        <reinette.chatre@...el.com>, <tglx@...utronix.de>,
        <mingo@...hat.com>, <bp@...en8.de>
CC:     <dave.hansen@...ux.intel.com>, <x86@...nel.org>, <hpa@...or.com>,
        <paulmck@...nel.org>, <akpm@...ux-foundation.org>,
        <quic_neeraju@...cinc.com>, <rdunlap@...radead.org>,
        <damien.lemoal@...nsource.wdc.com>, <songmuchun@...edance.com>,
        <peterz@...radead.org>, <jpoimboe@...nel.org>,
        <pbonzini@...hat.com>, <chang.seok.bae@...el.com>,
        <pawan.kumar.gupta@...ux.intel.com>, <jmattson@...gle.com>,
        <daniel.sneddon@...ux.intel.com>, <sandipan.das@....com>,
        <tony.luck@...el.com>, <james.morse@....com>,
        <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <bagasdotme@...il.com>, <eranian@...gle.com>,
        <christophe.leroy@...roup.eu>, <jarkko@...nel.org>,
        <adrian.hunter@...el.com>, <quic_jiles@...cinc.com>,
        <peternewman@...gle.com>
Subject: Re: [PATCH v8 1/8] x86/resctrl: Add multiple tasks to the resctrl
 group at once

Hi, Babu,

On 9/6/23 07:56, Moger, Babu wrote:
>>> @@ -715,7 +714,27 @@ static ssize_t rdtgroup_tasks_write(struct 
>>> kernfs_open_file *of,
>>>           goto unlock;
>>>       }
>>>   -    ret = rdtgroup_move_task(pid, rdtgrp, of);
>>> +    while (buf && buf[0] != '\0' && buf[0] != '\n') {
>>> +        pid_str = strim(strsep(&buf, ","));
>>> +
>>> +        if (kstrtoint(pid_str, 0, &pid)) {
>>> +            rdt_last_cmd_puts("Task list parsing error\n");
>>
>> It would be better to show the failed pid string in the failure report:
>> +            rdt_last_cmd_puts("Task list parsing error pid %s\n", 
>> pid_str);
>>
>> So user will know which pid string causes the failure?
> 
> It was already discussed. Printing the characters during parsing error 
> may not be much useful.

Could you please let me know where printing "pid_str" is discussed?

My understanding is a similar thing is discussed in v3:
https://lore.kernel.org/all/167778866506.1053859.2329229096484796501.stgit@bmoger-ubuntu/

Then v4 has this code without printing pid_str. In v4, there is a 
similar discussion of printing pid, but not pid_str.

But I cannot find a discussion of why "pid_str" is not printed.

If kstritoint(pid_str, 0, &pid) fails, without printing pid_str, how can 
user know which pid string fails? e.g. user tries to move 100 pids and 
the 51st pid parsing fails. It's hard for user to know the 51st pid 
fails without showing pid_str in the error info and then it's hard for 
the user to decide to re-do moving or aborting moving etc.

Thanks.

-Fenghua
then