Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b76af325-0b4d-25bf-a009-fbbbb3f99146@amd.com>
Date:   Wed, 26 Apr 2023 12:29:32 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] drm/amdgpu: add a missing lock for AMDGPU_SCHED
Content-Language: en-US
To:     Chia-I Wu <olvaffe@gmail.com>, dri-devel@lists.freedesktop.org
Cc:     stable@vger.kernel.org, Alex Deucher <alexander.deucher@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, amd-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
References: <20230426004831.650908-1-olvaffe@gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230426004831.650908-1-olvaffe@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WFAzZ0NTcFVDSFZrdEQrUE90Z0Y2UXJkbWx5TzVSd1pzRkp5ME5SUkVZYkpU?=
 =?utf-8?B?VU43UFcrUTVNMWVEeTVNakJ3aU02elI0bDdISXd5RGNtMi9aZmRGeFZBS0pj?=
 =?utf-8?B?VjgzSGVGQndoOE84ZHk4MlhuNXRUSWJqTHRrblBoNkV6bm5rWm5jeXkzbmh4?=
 =?utf-8?B?bGxWdEpjUWk2Mm8yU2JhVmxmNmZma0J4V3BUMXlmTUt3Q3Byd014dnBWWVVh?=
 =?utf-8?B?bmVqUWFMSTVmUS83MDI5U1lUU0tCRk5LcU00VG1TRnZnajVubGx2alczYWpU?=
 =?utf-8?B?MitlM2lhUkp2REF5cHUwSDAxdi9FWjJLcDNWa210M0RSWGZKZXAxc0tVaU81?=
 =?utf-8?B?TnBHYVVnOU05TFV4YWdZaWJZQjQrM1hVU21Icy8wa0dVSitTOG5waEszc3BH?=
 =?utf-8?B?cVBlSnhZV000dU1URk02ZkNoaG10bFV2Q1hneTc0ekQzZjVyUFROOE1hK1Ry?=
 =?utf-8?B?ampQMWMra2h4YWovbWVpMnhZNCtYendpdTEvam9CTDhxRCtqczZpbFBBd3V3?=
 =?utf-8?B?RS95TExCTisvdXAycG9iWnZvRko1aHlza0o4NWI4QTlHYS9hOTJ6eGRiZkRt?=
 =?utf-8?B?SjJoR3AvSk9FL1ZUcFF4Yy9CcS9jV1lwL3VPUUFJWDAvUk5qNHJDenNUanlY?=
 =?utf-8?B?c0FsSjJLUWFDQkZVSnplam5xUGtwQTV1VytFemVYSzJ6cU0vQVkxaW9qazBS?=
 =?utf-8?B?cEd6MUtTcFZCNURlQ1NQM1c2dmJGMzE1WE1qdThmZ2xWOXQxZmw4emR3ZlR6?=
 =?utf-8?B?b0lDbUNJRlpucGY1MkFIdDJKbG0vMjRBRmxHcHJHcThIUEJERm4xZFdsdVJX?=
 =?utf-8?B?c2k4QnBhRTBsWURZZ0lGbVkvTDgvbHZSMTNGazQ4T3BPSTI4dnl1eEF0NGd0?=
 =?utf-8?B?Slc2Y2tWZm1ieHFvT2R6VzQ4NmhBbTRnWDJIK2dRbnFBQUp0dTdTNUludCta?=
 =?utf-8?B?TzFGUWMrWXB3a24yK2tUZWVXWUtBS1ZmV1JybXlkOTh6Z2h6RUtFM2pZeVVs?=
 =?utf-8?B?eGJ3eE9DT3lTeFVhOCt5V0t6aUtXbWZHS05pUm9pTUlhanY5N3ZyRnk1QUh4?=
 =?utf-8?B?dXhJSk5iUmJSRlJNRlFXUmxLSFNUZ1BBMmlUR1BCUjN1UDlaN3BZQmRLbTg3?=
 =?utf-8?B?WUFsS0hnM00wanZjSTZSeWZ6NnJWT2R1clRpUnhQelVCMUpDVTVHMkdISGhv?=
 =?utf-8?B?ejcwekRMZ0w4VUZHWTVKQjNJTzZNdTRvUVJYM3BEOGlSVTUyWkxleGRPd3RG?=
 =?utf-8?B?WHNJWWQ5bnBCYm0xellqbHlETnBUcUVHRXNFQzdJZHJ6bGdLbG5ESGNhUUtZ?=
 =?utf-8?B?ZnhDQS9IN081blBHQlFRVWxka0luVFBwYy9jR0ZRaWVzcUVTYTM2Q1BVL3hG?=
 =?utf-8?B?dlFRVlI4NWRZbTI1cWhwZ1ZqTmRMV3NabG0vS2NoaE1JVy83d0FWYkRrS2lz?=
 =?utf-8?B?UkR6ZjZ2VGMyOTJwcWxiVmVmL20xb1BqOU82dnNhMEJnZGlhQzd6SGlaU2hM?=
 =?utf-8?B?MGVqdVk2RWV3YzY1WkFFMG5Mbi84S05zTExMMmMzakZ4U3E3S2EwRTZpY0R6?=
 =?utf-8?B?MWJXTWZxZGYvRmx5d1B3ZTNadDNSUWVyTk1LSW5LSWdpOUZtUWdBQUVaSjRW?=
 =?utf-8?B?Vyt0enA1U2tkZEh4akRuSnJIUlhBVjB6TC8rZzcxcHVKdm5hWE9IZHl1enVU?=
 =?utf-8?B?eVFXbEFMdS9FSjM5YXI5UHZ1emhBQnZDY0lieTh1REZDRTJMR1dyVDNkdzhV?=
 =?utf-8?B?TkI3R3VEODJEZVQyNlViUWN3YldQMktrSWttbExWQW44c2pOTXprQWtPd0Zs?=
 =?utf-8?B?dnJRb3BHc3VkVU1WaEhETFpsV20yS2w3cDlzOEhGM21tZlZzRTdRYkxvYlJq?=
 =?utf-8?B?b2doaWVLeDM4QlRFSXFINjRDQ0lwMDRaRHplajFyYThTVXBhbGJDTng1ODI1?=
 =?utf-8?B?TVRTdWw1TERqekQyMkMyZ0JMV2RpSXRzL0VvZlN4dTQ0L1NGb0pwd1FkWXpu?=
 =?utf-8?B?WWV2Nm5JbkswYzBMWUpIM2RDWGN4YzdiaGRJbVV1aEZ3WEUwS3hJS0U2REVS?=
 =?utf-8?B?SkljYmx1V09kdnU3dDdubG5iSUp2dllGT241dTJ6K2hwUnpZK1ZuSXRpYklS?=
 =?utf-8?Q?pUjCEmPrLbAxG/KkGCXFq/Kul?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 946d3186-916a-47f1-6c2d-08db464123b2
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2023 10:29:39.1300
 (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: X0OkgdJ2HUjcdkCf2cMxZq/OfWv7fFGRA3hblIRN5yc9lEM2MnA/Jf9IeMIm02cO
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB8799
X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 26.04.23 um 02:48 schrieb Chia-I Wu:

Good catch, but you need some commit message here. Something like "Need 
to hold the lock while iterating the idr to make sure no context is 
destroyed." should be sufficient.

Apart from that looks good to me.

Regards,
Christian.

> Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> index e9b45089a28a6..863b2a34b2d64 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
> @@ -38,6 +38,7 @@ static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
>   {
>   	struct fd f = fdget(fd);
>   	struct amdgpu_fpriv *fpriv;
> +	struct amdgpu_ctx_mgr *mgr;
>   	struct amdgpu_ctx *ctx;
>   	uint32_t id;
>   	int r;
> @@ -51,8 +52,11 @@ static int amdgpu_sched_process_priority_override(struct amdgpu_device *adev,
>   		return r;
>   	}
>   
> -	idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx, id)
> +	mgr = &fpriv->ctx_mgr;
> +	mutex_lock(&mgr->lock);
> +	idr_for_each_entry(&mgr->ctx_handles, ctx, id)
>   		amdgpu_ctx_priority_override(ctx, priority);
> +	mutex_unlock(&mgr->lock);
>   
>   	fdput(f);
>   	return 0;