Hi Fenghua,
On 9/6/2023 3:42 PM, Fenghua Yu wrote:
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 bad. Should have read your comments more carefully.
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.
That is correct. Will add following print statement o print the pid_str.
rdt_last_cmd_printf("Task list parsing error pid %s\n", pid_str);
Thanks
Babu
