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: <ba83fc40-e090-ace5-58e4-bb28feb6034e@amd.com>
Date:   Wed, 23 Nov 2022 07:40:43 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v1] drm/scheduler: Fix lockup in drm_sched_entity_kill()
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        Luben Tuikov <luben.tuikov@amd.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20221123001303.533968-1-dmitry.osipenko@collabora.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20221123001303.533968-1-dmitry.osipenko@collabora.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?U1VvMzRGdXlGVnlvWVFUVzQrL1RET0VBczJQbHYwekNwTjhISndES3dKNERi?=
 =?utf-8?B?SjNvaUphK2NQZURvTDlZVHZscFhCODFGUk5Vb0NtRCt6SEY1alRzMUxENmhF?=
 =?utf-8?B?eTIwb1BtVWx2UCt1VldvY0VSaXc5OWlKR3JUbEcvanBNRi95cGsvcHVjS1NI?=
 =?utf-8?B?MXRCdWJJdEhoLzNjOGFFaE9UbS91aWpVM1NOS244TFMzVC9yaG1TNnZZSzZE?=
 =?utf-8?B?NmZhdFZGMys1bXhPT3YrL1g4ZnRGQlMvR213dVB6blJaa3Vrc0dJVTNvK0t4?=
 =?utf-8?B?cnRDWGpybjVBbUE4OFlVc1RNTm5Sb0JXVzh5QXFnWU9uQjNjWlVsQUZ0MkF1?=
 =?utf-8?B?dUc1K1pwUWlvVkV6Uit3MXIrY05hOE5MbnJsbUtLemFaaFk0c3N2VGRjL3dm?=
 =?utf-8?B?WE1leFBYUVJneVQrQnRQN0kzZ3drSWlrVFArS2NLb3dHYkhaZFpZUjVpbEI0?=
 =?utf-8?B?NXY5KzdTS2xFRlJKVGFyUWp3Mk1Ddi9PTUUxVDRycWRiYXI2ZDgvRHhoL0xF?=
 =?utf-8?B?UGtLWHd1ZEgzWmVNc2dMYnZFZFB0RU1vaEg5VjhBbTFUVzgxYlg2aUdFYnE5?=
 =?utf-8?B?b0lYaWh4akVjOTd0UHN6V2NOb1ZCUVdVVzdsdWsxdm5nSSszWm8rbnd0ak0z?=
 =?utf-8?B?UE1DblVLVWhQUmc4RkFSdzlNRlRjcTkvYWhGenUra2ZMTUpvcEdLREJ3YzF1?=
 =?utf-8?B?VkhUdW0wdW9ZTkZxcFpIY3ZQOERjSFRRVlNYQnc2ejFOaFgxdFpETE5oTDU4?=
 =?utf-8?B?VFdJWG5IbmlNdkprd01oMWU0YTZWb0pVUnB1bEtBNjE2MUVNNk8xMFlYb0Zr?=
 =?utf-8?B?Z2luTkE0d3lwWUNBRmxMeHJYSWcwY2xhdGdpUTJOK0JQYnh1SDd0VUZkdnVu?=
 =?utf-8?B?MUp4aC9sdnlPSS9KV0I5VjJneUZkSm54RFRHMWlBeTI4MDJpMUNZVXJsUklu?=
 =?utf-8?B?SFI4VlNUeSs5d3dlTU9td1BlM21yUTBCMERqWDdaNVBWS3FWY3ZPL2NQa0RT?=
 =?utf-8?B?Z0xtUU5qVHdKaGhhcHgrbzZ3UTRYYVJNc3ZOYmx1bHo3OXdnbW1jOVNXaldr?=
 =?utf-8?B?c2lKOGxKSWh6S0hGcCs3dkE4UVU0U2tJZTYxWFNZZ1JYY1BVNGM1MnhIVk1t?=
 =?utf-8?B?Q0o0UjZxcCsrN25kcUtSMUFnRWpadGtWQXh3Z2ZIVCtEUWc1cTJvK3g2OW1Z?=
 =?utf-8?B?czFOTXE3dGgzR0w4TmpyeWF5ako5b2ZIcmNBeWhBSGFiNEthSEVqOTlWeDlC?=
 =?utf-8?B?V3RYdjhwcGhPMTZLYTdmTUp1aS9GTlJQdmFxcFBNRGpVRUVzZGtHZlZkM1Fk?=
 =?utf-8?B?cCsvVFUvQ3UwRWR1VGp4MXprKzJ4YklUQkNTVXRnTTAzWTFDcEdxMmQ1cXRy?=
 =?utf-8?B?djdkd0RCNEFrV0dHUVMvKzJKY213OGtnYkVrV1RndVN1cFRRem1UczJNNGpC?=
 =?utf-8?B?cW1RNWd0Z2M2N21OdlNtbEtaOEgrUWFLZWZPRFZrQk8ybTFHd2k5c2IyVncx?=
 =?utf-8?B?c3JwZm9neEtxWWRCeHhtcE5yZ2loMDU2OWtFQ1Rvam8vMFN5cjJ0U1Z1aE5R?=
 =?utf-8?B?a2ZjY1NUQkgyZmtkWEkyYnhGVjJjS2tJNXFmTDdzcXV6RHU0RU9HR2dDRE5y?=
 =?utf-8?B?RjBUMFlHd2ZaYVVJZ29nM0J2MXg3NDJiNVV2cnM5am1JeWI3NmhFUVNjWW9E?=
 =?utf-8?B?V1Y4Z0VYKzg5N3NMbmJGYjhRVitEVUs3eDJxTUNYUzJ6dHRJYndFWENtMVl6?=
 =?utf-8?B?Q2ZpVWJxYmU5UVI1ZVJLajJWZ0hWd1ZZVkZzbXNKemhWNUZDRmxjbnQwUXZ5?=
 =?utf-8?B?clNjMFMwTFU2Z2tFT29SMFBLRVhhMjBJSkhMdDMwSkZmWHpwVFhWdURwcnBl?=
 =?utf-8?B?aFJPd1czRGphQkdRaCtmNHBHN3ZrV1QwZHhzcUZMd3JGaWdSeXdRcVZZZE9D?=
 =?utf-8?B?RUJHQkVHQ2pRRzF0NUtDLythRHNnNDJwOXF4aW9aK0JmNEFENGs2Q1RrT0R4?=
 =?utf-8?B?R2hWR3JienBwNmRTWVhkTVdRWmtYanE1ekFDSzRRUWdmcWNjVTdpZ2dOVGlF?=
 =?utf-8?B?RlFvNnZPcnlpVkZyUW5xc2JuOU9WOTFKUXN4Mi9Ock9UWlV4NGhLdEtESzRw?=
 =?utf-8?B?SzR4aTdzb2ZBNDZ1cFRhY0d2bWQ5M2x0UWc2Q0V0b3FQWC9Od2dXU3llOElR?=
 =?utf-8?Q?EN6JoIlm9keuXJxxQ3Wvil5SY4ZpQFOd7Xlac0uKPBA2?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: add56683-c1ed-477e-b62d-08dacd1da7b1
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Nov 2022 06:40:47.9096
 (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: +ywaqDioMz1Zd7MYfoijvyb0jNueAszsDn1iIo7z6w/dFZ4DkoBVQvlA6hn2S87U
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB7151
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS 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 23.11.22 um 01:13 schrieb Dmitry Osipenko:
> The drm_sched_entity_kill() is invoked twice by drm_sched_entity_destroy()
> while userspace process is exiting or being killed. First time it's invoked
> when sched entity is flushed and second time when entity is released. This
> causes a lockup within wait_for_completion(entity_idle) due to how completion
> API works.
>
> Calling wait_for_completion() more times than complete() was invoked is a
> error condition that causes lockup because completion internally uses
> counter for complete/wait calls. The complete_all() must be used instead
> in such cases.
>
> This patch fixes lockup of Panfrost driver that is reproducible by killing
> any application in a middle of 3d drawing operation.
>
> Fixes: 2fdb8a8f07c2 ("drm/scheduler: rework entity flush, kill and fini")
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

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

> ---
>   drivers/gpu/drm/scheduler/sched_entity.c | 2 +-
>   drivers/gpu/drm/scheduler/sched_main.c   | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index fe09e5be79bd..15d04a0ec623 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -81,7 +81,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
>   	init_completion(&entity->entity_idle);
>   
>   	/* We start in an idle state. */
> -	complete(&entity->entity_idle);
> +	complete_all(&entity->entity_idle);
>   
>   	spin_lock_init(&entity->rq_lock);
>   	spsc_queue_init(&entity->job_queue);
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 6ce04c2e90c0..857ec20be9e8 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -1026,7 +1026,7 @@ static int drm_sched_main(void *param)
>   		sched_job = drm_sched_entity_pop_job(entity);
>   
>   		if (!sched_job) {
> -			complete(&entity->entity_idle);
> +			complete_all(&entity->entity_idle);
>   			continue;
>   		}
>   
> @@ -1037,7 +1037,7 @@ static int drm_sched_main(void *param)
>   
>   		trace_drm_run_job(sched_job, entity);
>   		fence = sched->ops->run_job(sched_job);
> -		complete(&entity->entity_idle);
> +		complete_all(&entity->entity_idle);
>   		drm_sched_fence_scheduled(s_fence);
>   
>   		if (!IS_ERR_OR_NULL(fence)) {