Message-ID: <95fca7d1-d473-e9e2-b6c8-c4ae3d44d2df@intel.com>
Date:   Wed, 15 Feb 2023 13:43:28 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.7.2
Subject: Re: [PATCH] x86/resctrl: Only show tasks' pids in current pid
 namespace
Content-Language: en-US
To:     Shawn Wang <shawnwang@linux.alibaba.com>, <fenghua.yu@intel.com>
CC:     <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
        <dave.hansen@linux.intel.com>, <james.morse@arm.com>,
        <x86@kernel.org>, <hpa@zytor.com>, <linux-kernel@vger.kernel.org>
References: <20230116071246.97717-1-shawnwang@linux.alibaba.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230116071246.97717-1-shawnwang@linux.alibaba.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a0RqajVZVUovVVY1cm5OSTRQRFJMeVd2d3EzOUJZZkxqeFN5cSttUUFYQzV4?=
 =?utf-8?B?d2ltU2w5WDNDS00vV25HNG1NOVhpNmR3YkxLWFdNMktHVjJ4YzRrMVByQXR3?=
 =?utf-8?B?aUxCbWtrenJqbnFpQVhrOXp0SEpoWVJ4UTVSNlh6djdHQnZFTExzTG9JUk5R?=
 =?utf-8?B?SDVLc0JQbE8wSUZWNC9FbzJPQllNbTVueTIxUVMyZzdhMDFBTVJyTjZSWms3?=
 =?utf-8?B?Mm1JdkJKcmpyYVNpSWFmZVltaWVyZHNQRTMycW5EZ2xSZmJ4VVo3NWl4elNM?=
 =?utf-8?B?aER4V2pNVkw3dzVGMU80b2NGUERoRDNhanErL2FpdThSbmQ2S0xEYWp5TTB0?=
 =?utf-8?B?Qjh5Ri9PUmlLeFJUL0ZlbmUyUGRyaHJISS8wd3J1ck91NTZXOTE2UDlFRDZm?=
 =?utf-8?B?VFRJS0kxK1dhSXU3OFF6NDV3NER1bW1kUnFaMHlualV5dm1kNnJLTzRYeFJ3?=
 =?utf-8?B?RHAyT2lkcXRwcHFuSDRCbkdxNWlmRlMySDNFeHRNRXhmdkVsT0wvb09LMVNH?=
 =?utf-8?B?a2NWR1I1LzZaeHZpTzZjWmtmc1ljcFVBMjB3cmQ1dlFtZXZTYWw4ckkzMUE2?=
 =?utf-8?B?RlpXMXA1UTMxQTllbjA2eVU3eXlGTnRaS3FReEV1YlN2cVdhK1NXRU1pSmEx?=
 =?utf-8?B?dENMSkc2aU5kZDRqWjcvWkJKUlI5NkVNcXFiZUZySFgvek5KMWtRZDJMb3JC?=
 =?utf-8?B?UGFVR1JvcDBBNEh4VmovQXlNcWRiMkdIcmhzbGNGOTBFbHA2ekJVdXpFZ1Zp?=
 =?utf-8?B?TFhUNUpSQ29TakJIdFAxdVZjUU9BUDNIYnNnbi91YkR6TUN6OE9XbkdDazVi?=
 =?utf-8?B?M3dIdXVGVjh4NmR3ZFBJclBrM1R5WGtneGVKbkI0UkRxQWpWejdVbm1oVVpF?=
 =?utf-8?B?YTZHS0g1OU11VXBjV3JjNTNUQWIrQUVzOFR2V2x1aDBxSis4NE1URWlkOWJn?=
 =?utf-8?B?MzNXaUVDVVZVd3lSNlVuc1B1TS90UkNyS3pLYi9MMTVOMTJnOXlnNDA2K2FP?=
 =?utf-8?B?MmR5eVJBd2xmaUFxekVJV2I1MkJmUEg4b2F5aXhzTU1uUEora0p4d1grempN?=
 =?utf-8?B?aEVMRHViSlYyMHRLc0g0QVNyQVpRcG9ZaVNRaVdkRlZ1Tm1Eb3lPN1QxRHAz?=
 =?utf-8?B?ZHMyMmFLbWNDTWNSVnNEQlNKMlFnNnJCSlNVWUFpRFpzbnAyRXNBTHZIRVdM?=
 =?utf-8?B?eXA0RmtXdDlLMEpTT2ZtU3lKbzBPblhWZ0VxcVYvcjlDd09NRmJvdFhQRUhL?=
 =?utf-8?B?UlBlVnZYSk5XYjhIL0gvbkQ0NHdFanVwNDVkMnFyMXdQZ0ErQ2pRL281SDhh?=
 =?utf-8?B?ZHRaZnloZkJKWEJQUGMvUFdWZWV5T2xFeWFCVDF4bXBiOEdHc20zdXJLbTlV?=
 =?utf-8?B?N1JDdmI3bjZjbmdqQnRUN3lsL1NjUmF3NU9ocVgxOFkyL1RXc2Q0VFNncy9R?=
 =?utf-8?B?Wm1YR0ZiK2R0alFEQzlWTytGWUIvc1BMRVpQajk0VDNZVHlPcFVjOFlIWlBE?=
 =?utf-8?B?WlFocTJ2cE1jbFpBN05ITTNzM0dRYnp5TFgwb1AyUTdIeURseFViSEViZ2d4?=
 =?utf-8?B?Q0o1K2JCa0R4U21tQmFZR1JmTkNrOEVNaFFRSkRuLzlLbm1SOXRLdGNTYk96?=
 =?utf-8?B?R2hvWWZhY1pHUUtuOGRvdHgzRm04bFp0NVZYRWt2OHJhQXVVWXZFL2QyWFVV?=
 =?utf-8?B?QjJuK3p3YlFJNlVqR1ZxNS8yQUNmTlo5SkxYRDBWM3FlaVJjQU5XV3B3UmJO?=
 =?utf-8?B?V3VhYjM1MHRBMlVadCtqR2I2SDRoRDdQR1VhYVZ0Z1ZCaVZCY0lGK1VDYUZ4?=
 =?utf-8?B?MFBZcTNSUk1YVlpxbktXbC8zRlM2bEZCTnVKeUpKVjZIWUd6S3YwMGpod0w3?=
 =?utf-8?B?aVR4QUQwQU1aNmI0dEdmZFd2aVJWVkpPU3BUVWZnOUFrdTVIVVVmb0RzVWJI?=
 =?utf-8?B?emcxSTlsS0NHNkxoenZ5cUQyMnlQQ0NTd2ZHNEVDZWd6ZUNDTkRtNTZhcmNt?=
 =?utf-8?B?eUVKOU5oc0Rnc2tjUmlmTnNOSkhOZWNEdU12MllDOWFFM21KblpzKytzSHVP?=
 =?utf-8?B?RGNsR0lYOWFUQUVHaC90ZVFIaTVjQ0dYdzQxdkxwdCs3b3U3QTNzclg3aEFn?=
 =?utf-8?B?OENnbjJpSjJNWm9ETzNzVi9MSnBOcmZzeUo2aXhRbHE1V1pkbkdleWRGWjdw?=
 =?utf-8?B?d0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 910096b2-7c3a-4488-4458-08db0f9e3092
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2023 21:47:10.0010
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ExYlVy4skyHp7dN3SDKdnX0KuUXyPC4ifBGreIwroKmIxvlJkMCZl1tS8WbVY0ZGEHHYNP1Wh+k1XTzzbCW9VaEq7yBLEJE+wb5N3h6m+Jc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7270
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Shawn,

On 1/15/2023 11:12 PM, Shawn Wang wrote:
> When writing a task id to the "tasks" file in an rdtgroup,
> rdtgroup_tasks_write() treats the pid as a number in the current pid
> namespace. But when reading the "tasks" file, rdtgroup_tasks_show() shows
> the list of global pids from the init namespace. If current pid namespace
> is not the init namespace, pids in "tasks" will be confusing and incorrect.
> 
> To be more robust, let the "tasks" file only show pids in the current pid
> namespace.
> 

Is it possible to elaborate more on the use case that this is aiming to
address? It is unexpected to me that resource management is approached from
within a container. My expectation is that the resource management and monitoring
is done from the host. 

> Signed-off-by: Shawn Wang <shawnwang@linux.alibaba.com>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 5993da21d822..9e97ae24c159 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -718,11 +718,15 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
>  static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
>  {
>  	struct task_struct *p, *t;
> +	pid_t pid;
>  
>  	rcu_read_lock();
>  	for_each_process_thread(p, t) {
> -		if (is_closid_match(t, r) || is_rmid_match(t, r))
> -			seq_printf(s, "%d\n", t->pid);
> +		if (is_closid_match(t, r) || is_rmid_match(t, r)) {
> +			pid = task_pid_vnr(t);
> +			if (pid)
> +				seq_printf(s, "%d\n", pid);
> +		}
>  	}
>  	rcu_read_unlock();
>  }

This looks like it would solve the stated problem. Does it slow down
reading a tasks file in a measurable way?

Reinette