Message-ID: <2d4c10c7-6406-7458-4f52-4260b415874e@amd.com>
Date:   Tue, 14 Mar 2023 16:33:03 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [RFC 01/10] drm: Track clients by tgid and not tid
Content-Language: en-US
To:     Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc:     cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
        Tejun Heo <tj@kernel.org>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Zefan Li <lizefan.x@bytedance.com>,
        Dave Airlie <airlied@redhat.com>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Rob Clark <robdclark@chromium.org>,
        =?UTF-8?Q?St=c3=a9phane_Marchesin?= <marcheu@chromium.org>,
        "T . J . Mercier" <tjmercier@google.com>, Kenny.Ho@amd.com,
        Brian Welty <brian.welty@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
        Zack Rusin <zackr@vmware.com>,
        linux-graphics-maintainer@vmware.com,
        Alex Deucher <alexander.deucher@amd.com>
References: <20230314141904.1210824-1-tvrtko.ursulin@linux.intel.com>
 <20230314141904.1210824-2-tvrtko.ursulin@linux.intel.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230314141904.1210824-2-tvrtko.ursulin@linux.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YVR1NW85clp1LzJqUGVCcWpZamt6OXNWejFMbGVkYjdySFViTFdGK0N5Ty90?=
 =?utf-8?B?a055dGNNaVpHQ1FFa0xicTR3dENUeGRMSm5sWmY1ckprZUg1M1B4aUU5L1Rr?=
 =?utf-8?B?YmoyN3FLSzFrT0wyKzFCU3B5WkxVYU82U3JPNUhVZFc1S2tmMnZ2V3RqTDBR?=
 =?utf-8?B?Z0w2Y1VNLzFqWnlLY253bnF0ZW9vR2lOTG80M253OGdTWDdIQU5FWHpVWEUr?=
 =?utf-8?B?Z1dhSUYyVytDZDNLWmhpTDhvakRWWjhmRVc4SHdZOGQ2QzdCMy9yMC9nMjJx?=
 =?utf-8?B?SVZTK01NNzgrVzZvVEhmS0c3Z2NGZnphNWtibXI3Z0tYYzVzTjVrYTVHWlRu?=
 =?utf-8?B?a1NGR24zeU9WaEVCRHdqNkRuWTJXd1F3SFlkWlhER0NBRHZoNE9CVlJDL0gx?=
 =?utf-8?B?Y0htUlVlZk5IeVp6NGphNGV3SHp3eVo1eHllU3U4WkFIR3ZWY2l2bGRXbThk?=
 =?utf-8?B?cnlpYm9MRFowNDlKYllxVlE0K012MmxhUHVmNFp2dnNMWDNxOU1NUTNUSFhK?=
 =?utf-8?B?Q3I2N2xDbVREaTk2MTVvTGIvK1B6K0dCWm14MGw3VURINUpHdnRPQnE1WUZL?=
 =?utf-8?B?YWpQL212dzhlT3czd05BeEllRkJhM05ZMWEvMUZLSFpFSmN5ckhFaVhSa3Rl?=
 =?utf-8?B?MjRobElvd1d3U2JLbmp2cTc2YjVXcXpUOWtpQW1vakwwNnAwNFFPVkN2RUFB?=
 =?utf-8?B?WkRCSHUyc09hV2dqcVg1ZUtVckNka3NhRWpnNnR1WU5zWmhNeklBeDZodnc4?=
 =?utf-8?B?SExXcU1iazkvNnJzRFJXa0Rwb09Yc2FicHplZy8wT3hpSTV2b0ZZYndRY2dp?=
 =?utf-8?B?SGJTOTRocXdoeElZaG8xb01UTkE0cjJVTDdZeHZwTENlS1NNOURoUTdyRks3?=
 =?utf-8?B?UEpEV3QwTm9hRjZrblhNVzRyN1F0enBWazduendMV0VkQVFmcitkOHEwUThx?=
 =?utf-8?B?Rld1OWhFYXJhVENBV1B1TUtWVW1wSGFmbGhJWWNXTzBiLzdmQlM2WXcvRGk1?=
 =?utf-8?B?Qzk4cWpoN2RUT2ZoMGZ1UVFoc1FmMGZEMGJ3QUJmd1pQS3hKS0MzMWk0QUov?=
 =?utf-8?B?QUhOZjY5Y2VpWWFSSjd6MFVyU2JZVUh5SmxoVDdMSzQ2RStIQmFkWGx2eHAy?=
 =?utf-8?B?SFJrcmZEVGRaRUt3ZlQxTVlLZXIzMVBFcklmbDdMNk1nQlJuWVhNQWdVRlVW?=
 =?utf-8?B?QWpzMVBQKzhZd05BS2RyVC8zc1RtdmwxMXlCZUFQMTFxNStpaE5BUnBrTUZ5?=
 =?utf-8?B?cHJ2RFVwOE0zV29Yc3JteG0vb21jd3U1dFlxeFh0VzVvRzNxeUQ5OGppbW44?=
 =?utf-8?B?WGVJNkxPSjY2aWMwUFN6d041eUpWS0JZZWlhN0JGMTNpdlU1QWFrSHM0dzBD?=
 =?utf-8?B?cno0b0cwdEQza3dsSHJlTlNLcExkaHdRelJkOFNVaVpMSTBpSHI3YTUwYVFy?=
 =?utf-8?B?MmdVcEFISStKdFdBREFKakZxNDZnWmp0UWc1TmtyL1NtWGdHRlF5aHlsenJT?=
 =?utf-8?B?QkhYOWdhRjA1V3RUSTlPRERKNWh4Nm1FcHlHa045YzhXcmFpSmFNcE9KL1dF?=
 =?utf-8?B?UmJqQTl2VkFISFdwcGF1Q1ovdGJwcU5tZ3l2Z08wN2NZV2dVTWhhR2tzbWRw?=
 =?utf-8?B?V3VMaU1YR0NIOTNQUWdPNDZZSzlKcUxpV3ErMjNWdHZMRlJqdVlhME1zU1JC?=
 =?utf-8?B?eDVjMER3eVZlbUJ6cU5DOWkyQyt0Y2RWWlBtcGZOWjdVR1NNQnF6WXA1c0xF?=
 =?utf-8?B?WTgyc1ZJY0VZSlhFbWdJQVN5eHJNM2E3SE9rN2RkNFZGQlQ3ZzR0bGJ2ZzF3?=
 =?utf-8?B?RytZNnk1NDgvZWt6Qysxczl0RmxtVzRBR3A1RDlKNXN3cUtHMmxOeEFNWm8r?=
 =?utf-8?B?L3VEeGJ0dmJaYnVUdnBQU2xibTVmazdiOVVpVGNLVS9XeWtHbU0vYWpFYjds?=
 =?utf-8?B?ejZPVCtYRzhjQm1iMmdmM29LS1JKbXhZWnlVNlRzRUdxQkcwNVM5NDd2eWhp?=
 =?utf-8?B?ajB4T08yWFlvNFhvU09mUnF3bVN6TWRyVWhoaE53RklrcXRFZU54YTE4RGdn?=
 =?utf-8?B?aUxkSElrcWRNS1oveEdFNXdDN0Z6cU5PeEU3SGVkVDZaQXRTUCtOcW1LcGdK?=
 =?utf-8?B?WU1sNUxsUS9zdGdVTitkSFBOWm5XWXJ5ajNYcW1oa0FJWWtqc1ZkRnc1clY1?=
 =?utf-8?Q?Bxe0YMjGfrTTig2sBoFLTexLgke5yvz/PT+VMs2LAY2c?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: db5a78d4-2da1-49ad-5983-08db24a16b04
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2023 15:33:10.8401
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /6yZO23EDwmO7yosF4U+c17SuMPqOeU/zDnBidR/hq+hbOdsnem/mYSQION+Xo0M
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5106
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 14.03.23 um 15:18 schrieb Tvrtko Ursulin:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Thread group id (aka pid from userspace point of view) is a more
> interesting thing to show as an owner of a DRM fd, so track and show that
> instead of the thread id.
>
> In the next patch we will make the owner updated post file descriptor
> handover, which will also be tgid based to avoid ping-pong when multiple
> threads access the fd.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Zack Rusin <zackr@vmware.com>
> Cc: linux-graphics-maintainer@vmware.com
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Reviewed-by: Zack Rusin <zackr@vmware.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

Should we push the already reviewed cleanups like this one to 
drm-misc-next? That makes sense even without the rest of the 
functionality and reduce the amount of patches re-send.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
>   drivers/gpu/drm/drm_debugfs.c           | 4 ++--
>   drivers/gpu/drm/drm_file.c              | 2 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_gem.c     | 2 +-
>   4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index d8e683688daa..863cb668e000 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -969,7 +969,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused)
>   		 * Therefore, we need to protect this ->comm access using RCU.
>   		 */
>   		rcu_read_lock();
> -		task = pid_task(file->pid, PIDTYPE_PID);
> +		task = pid_task(file->pid, PIDTYPE_TGID);
>   		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
>   			   task ? task->comm : "<unknown>");
>   		rcu_read_unlock();
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 4f643a490dc3..4855230ba2c6 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -80,7 +80,7 @@ static int drm_clients_info(struct seq_file *m, void *data)
>   	seq_printf(m,
>   		   "%20s %5s %3s master a %5s %10s\n",
>   		   "command",
> -		   "pid",
> +		   "tgid",
>   		   "dev",
>   		   "uid",
>   		   "magic");
> @@ -94,7 +94,7 @@ static int drm_clients_info(struct seq_file *m, void *data)
>   		bool is_current_master = drm_is_current_master(priv);
>   
>   		rcu_read_lock(); /* locks pid_task()->comm */
> -		task = pid_task(priv->pid, PIDTYPE_PID);
> +		task = pid_task(priv->pid, PIDTYPE_TGID);
>   		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
>   		seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u\n",
>   			   task ? task->comm : "<unknown>",
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index a51ff8cee049..c1018c470047 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -156,7 +156,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
>   	if (!file)
>   		return ERR_PTR(-ENOMEM);
>   
> -	file->pid = get_pid(task_pid(current));
> +	file->pid = get_pid(task_tgid(current));
>   	file->minor = minor;
>   
>   	/* for compatibility root is always authenticated */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> index d6baf73a6458..c0da89e16e6f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> @@ -241,7 +241,7 @@ static int vmw_debugfs_gem_info_show(struct seq_file *m, void *unused)
>   		 * Therefore, we need to protect this ->comm access using RCU.
>   		 */
>   		rcu_read_lock();
> -		task = pid_task(file->pid, PIDTYPE_PID);
> +		task = pid_task(file->pid, PIDTYPE_TGID);
>   		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
>   			   task ? task->comm : "<unknown>");
>   		rcu_read_unlock();