From: "Moger, Babu" <bmoger@amd.com>
To: Fenghua Yu <fenghua.yu@intel.com>,
Babu Moger <babu.moger@amd.com>,
reinette.chatre@intel.com
Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
corbet@lwn.net, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, eranian@google.com,
peternewman@google.com
Subject: Re: [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once
Date: Fri, 17 Feb 2023 09:05:55 -0600 [thread overview]
Message-ID: <090d80c5-47ad-c152-27cc-81019aa5daef@amd.com> (raw)
In-Reply-To: <3898cf3c-42e8-1fa0-395f-318bceda313d@intel.com>
Hi Fenghua,
On 2/16/2023 4:27 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 2/2/23 13:46, Babu Moger wrote:
>> The resctrl task assignment for MONITOR or CONTROL group needs to be
>> done one at a time. For example:
>>
>> $mount -t resctrl resctrl /sys/fs/resctrl/
>> $mkdir /sys/fs/resctrl/clos1
>> $echo 123 > /sys/fs/resctrl/clos1/tasks
>> $echo 456 > /sys/fs/resctrl/clos1/tasks
>> $echo 789 > /sys/fs/resctrl/clos1/tasks
>>
>> This is not user-friendly when dealing with hundreds of tasks.
>
> Maybe add something like "poor performance due to syscall overhead...".
Ok. Sure
>
>>
>> Improve the user experience by supporting the multiple task assignment
>> in one command with the tasks separated by commas. For example:
>>
>> $echo 123,456,789 > /sys/fs/resctrl/clos1/tasks
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> Documentation/x86/resctrl.rst | 9 +++++++--
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 24 +++++++++++++++++++++++-
>> 2 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/x86/resctrl.rst
>> b/Documentation/x86/resctrl.rst
>> index 058257dc56c8..58b76fc75cb7 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -292,13 +292,18 @@ All groups contain the following files:
>> "tasks":
>> Reading this file shows the list of all tasks that belong to
>> this group. Writing a task id to the file will add a task to the
>> - group. If the group is a CTRL_MON group the task is removed from
>> + group. Multiple tasks can be assigned together in one command by
>> + inputting the tasks separated by commas. Tasks will be assigned
>> + sequentially in the order it is provided. Failure while assigning
>> + the tasks will be aborted immediately and tasks next in the
>> + sequence will not be assigned. Users may need to retry them again.
>
> May need to add "tasks before the failure are assigned...".
Sure.
>
> To retry movement, user needs to know which pid fails. So it's better
> to add "last_command_status shows the failure pid and user can parse
> it to retry assignment starting from the failure pid".
Sure.
>
>> +
>> + If the group is a CTRL_MON group the task is removed from
>> whichever previous CTRL_MON group owned the task and also from
>> any MON group that owned the task. If the group is a MON group,
>> then the task must already belong to the CTRL_MON parent of this
>> group. The task is removed from any previous MON group.
>> -
>> "cpus":
>> Reading this file shows a bitmask of the logical CPUs owned by
>> this group. Writing a mask to this file will add and remove
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index e2c1599d1b37..13b7c5f3a27c 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -683,16 +683,34 @@ static ssize_t rdtgroup_tasks_write(struct
>> kernfs_open_file *of,
>> char *buf, size_t nbytes, loff_t off)
>> {
>> struct rdtgroup *rdtgrp;
>> + char *pid_str;
>> int ret = 0;
>> pid_t pid;
>> - if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0)
>> + /* Valid input requires a trailing newline */
>> + if (nbytes == 0 || buf[nbytes - 1] != '\n')
>> return -EINVAL;
>> +
>> + buf[nbytes - 1] = '\0';
>> +
>> rdtgrp = rdtgroup_kn_lock_live(of->kn);
>> if (!rdtgrp) {
>> rdtgroup_kn_unlock(of->kn);
>> return -ENOENT;
>> }
>> +
>> +next:
>> + if (!buf || buf[0] == '\0')
>> + goto unlock;
>> +
>> + pid_str = strim(strsep(&buf, ","));
>> +
>> + if (kstrtoint(pid_str, 0, &pid) || pid < 0) {
>> + rdt_last_cmd_puts("Invalid pid value\n");
>
> Better to add pid_str in failure info. Then user knows where the
> failure pid happens and can re-do the movement starting from the
> failed pid.
ok.
>
>> + ret = -EINVAL;
>> + goto unlock;
>> + }
>> +
>> rdt_last_cmd_clear();
>> if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
>> @@ -703,6 +721,10 @@ static ssize_t rdtgroup_tasks_write(struct
>> kernfs_open_file *of,
>> }
>> ret = rdtgroup_move_task(pid, rdtgrp, of);
>> + if (ret)
>
> May need to report "Failed at %d\n", pid;
Ok. Dont want to overwrite the last command status. I may need to update
it with pid. Will check on this.
thanks
Babu
>
>> + goto unlock;
>> + else
>> + goto next;
>> unlock:
>> rdtgroup_kn_unlock(of->kn);
>>
>>
>
> Thanks.
>
> -Fenghua
next prev parent reply other threads:[~2023-02-17 15:06 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
2023-02-02 21:46 ` [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger
2023-02-16 22:27 ` Fenghua Yu
2023-02-17 15:05 ` Moger, Babu [this message]
2023-02-02 21:47 ` [RFC v2 PATCH 2/7] x86/resctrl: Remove few unnecessary rftype flags Babu Moger
2023-02-02 21:47 ` [RFC v2 PATCH 3/7] x86/resctrl: Rename rftype flags for consistency Babu Moger
2023-02-02 21:47 ` [RFC v2 PATCH 4/7] x86/resctrl: Re-arrange RFTYPE flags based on hierarchy Babu Moger
2023-02-02 21:47 ` [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option Babu Moger
2023-02-17 1:42 ` Fenghua Yu
2023-02-17 17:29 ` Moger, Babu
2023-02-02 21:47 ` [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger
2023-02-17 1:46 ` Fenghua Yu
2023-02-17 17:39 ` Moger, Babu
2023-02-02 21:47 ` [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option Babu Moger
2023-02-17 1:50 ` Fenghua Yu
2023-02-17 17:49 ` Moger, Babu
2023-02-16 18:05 ` [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Moger, Babu
2023-02-16 22:13 ` Reinette Chatre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=090d80c5-47ad-c152-27cc-81019aa5daef@amd.com \
--to=bmoger@amd.com \
--cc=babu.moger@amd.com \
--cc=bp@alien8.de \
--cc=corbet@lwn.net \
--cc=dave.hansen@linux.intel.com \
--cc=eranian@google.com \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peternewman@google.com \
--cc=reinette.chatre@intel.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).