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: <dc7b30b6-951e-2f4f-13d9-aa393cc37dfc@amd.com>
Date:   Mon, 14 Nov 2022 11:31:26 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH v1] drm/gem-vram: Fix deadlock in drm_gem_vram_vmap()
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Dmitry Vyukov <dvyukov@google.com>
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20221113233850.427704-1-dmitry.osipenko@collabora.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20221113233850.427704-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?NGxCS2N5ZEx4ajZUazBZZ3dEdHdESmFLV2tTZ2Jjb25QMURXQWxaQnk5QUN1?=
 =?utf-8?B?aFpkTTFJS2pzTExpeGs1Q2I3M2tFR2FtRlcxRW5hejZjRjJjNVhHb3NPcXdZ?=
 =?utf-8?B?enVxdVVoa09kZ29sa1RoK1QwK3VMQjVoOThvajZOczJWQmxyTG9CY1VkWi83?=
 =?utf-8?B?b081c3JlKzAySWxnN1d2MlZMUDN0RDNxMmxVTDVVdTZuMk1sM203ZDFjZW0v?=
 =?utf-8?B?VDAyUDZpL21QSDFYSkhxK3R4QTVaRGx5eGhjRHJWMlQzc1dHY1NlN2VUTGEx?=
 =?utf-8?B?azJudHUyemZ3K0JSZnZuVlFtb0Zmbklkd05PYnFZYk5GNFB3NzlHeHZ0UWN5?=
 =?utf-8?B?V0xhMVdCRnVEZkszamZKazRBbHlRSzVEMkIrLyswUEQ0YitFalhBcnJVY25h?=
 =?utf-8?B?RFVnL3RqZzJWSVhsSzRTTGxnZWRja0VIQ2FLNmUwZUhCcGJaT1hPMXN2c0lT?=
 =?utf-8?B?Y2lKT1IrWDdTVlczdi8xOSthZzl4R1Q1c0Z4TDVROW02UDVNc3FBTDBXcmN4?=
 =?utf-8?B?UUNkcmVDdmpQV1JCcnA0TU45STZjVzk1QlpSbGJDWHVQR0FYeUJpYjhVWi9o?=
 =?utf-8?B?Y3o2YTEzTFkwRnBrZTZGNlJlakRvbWlGR1hFU0tnbVZGUDhscGZzK2pMcE9E?=
 =?utf-8?B?MGszYlJyRzF3a3V6YTZqSUMwdTNIUVBnYU1JdldwNVlVN3NQZTYyVHloWloy?=
 =?utf-8?B?SWlXNlNIYlZObzFham5KcEV2SUFQSmRXNTV6TkZ1bnlPU250WXNKbFV4eVJw?=
 =?utf-8?B?NGs3V3Qxb090OFFDd1k0ZndBNXJMUDJmN2xCbFFWd2d4Q01ucUlKUXRxK3Zu?=
 =?utf-8?B?ZmJNOUFwTWRZT3ppL3hsaTZQd1B3c25WcFphU3MvYW41N080TFpwKzN5UlZh?=
 =?utf-8?B?Z29MMHR6Y1NkRzd6VGpiWHBLbWxIbW9aY3Btc2U1RkJoU0ZWMjErK09vaWRr?=
 =?utf-8?B?YWNPV0k4b2J1Smx0Sm5WVUhRR0w3MFRmN2swc1dkejU2bldzVk51UjRJWHVG?=
 =?utf-8?B?enY2RkNlN253MWpUa2Q1NUtTMkhCUWlQY3VMaWkrM1RFTW1MTUpJNTlxUk9S?=
 =?utf-8?B?bld0L29XUDdEaTZiT2NqdnA5bkIwT2htZ2pDRW11VEx1ejdKWjZCcHllVkh5?=
 =?utf-8?B?WVh4T055TEZ6RUEyTExYaFQ5dnBCemozK2lscGwyblpXd3k1Rm5rS0lmdEF6?=
 =?utf-8?B?UFBRSlJuWFk1Q1pDSjBURUZqNlBRakQyV2pLazY3YSt2eFFmN0R2eVlvY2ZE?=
 =?utf-8?B?cm5sV0ZtREQ3ZU5naFNFczRHRHJGYVlINlhaNkNTbmlCUFQ2T2E4cUd6ZGwx?=
 =?utf-8?B?MnVtUjJwOS9nZjhyckNsR2lWVGpPWG00VlREZVF4OWdBOWFSRTRtYVltUnVE?=
 =?utf-8?B?YnZIdDBLRUUxcWt6cjdXWGFQcDk5eVhDU3RYVDFuSm00RHZaVzhjd2Ywa3BZ?=
 =?utf-8?B?ZUtXNlQzaVFtaXErQzFDbk51anl3YVNEdllBWi9BN1V4clAwMHZxY3IrcmxR?=
 =?utf-8?B?MVdmdWJOZ2NQRXVMRXd4T2V3MG04eVAyVUFpN1VZeFhHbFM3TUpHTCt1bEp6?=
 =?utf-8?B?Lytqc1pRRkZBYmZXK2pRR2xZUytzRnYyQmg1V3VwNk1CVU9lM2dvZDJHdHo4?=
 =?utf-8?B?QnNKcFFPZFhtVTMvYkNHNkNZS0tXdlhIVEsxRnovbU5LbEwrWmtwb0Frb1k5?=
 =?utf-8?B?cHZIeUcrYjBOaU9adzZSdVA0ekxBWSs2TXlPeGdUTmgrZDRMTy95dU12UG5E?=
 =?utf-8?B?Yi9zeTh5dXJuZ1Z5a0QvSHIzUXNRckF2RS9ycGN0Vm1kMmFndHZYdkxGaEIv?=
 =?utf-8?B?aGNpV2xSTnhnTEQrSVlFOXd2YjB5Y2U3NWpyWVJ5UjFFZURQOFJFbTdUSE1X?=
 =?utf-8?B?cWZ0SkZPRHMwZ0ZWVk5Qckx2amcyWit6dlFJRnhXRmZmK01MNGxQVE5hcGNB?=
 =?utf-8?B?RDFpcGdtV1YrcldiZkZDalNXNEJGS242a3BsWmEyVUdodU5OUTlINnEvOHls?=
 =?utf-8?B?VUs2WFZMNTE0Qy90VFRDRHV1UDVPeVRPNS81aitHR3UzSVlxUkFMbXZKZ3lB?=
 =?utf-8?B?d1dyclU1MXl2cDJYejdKaWhkVFcrcUlSZ0IxYk9hMzRyZjkzOVRJUzhNSFdL?=
 =?utf-8?Q?chBUVpSuRWIqyg7ymukGQOy3r?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f90a3f40-400c-4c55-cae5-08dac62b660a
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Nov 2022 10:31:32.6410
 (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: EOTOjM1oL4YMWexb/tca5XX67E/wSoajWGS23uayyJKVOZPosnHE6zLNPBng/8MH
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB6681
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 14.11.22 um 00:38 schrieb Dmitry Osipenko:
> Recently DRM framebuffer core and all drivers were moved to unlocked
> vmapping functions that take the reservation lock. The drm_gem_vram_vmap()
> was missed out by accident and now deadlocks drivers that use drm_gem_vram
> helpers when framebuffer is updated, like Bochs driver. Remove the locking
> from drm_gem_vram_v[un]map() functions to fix the deadlock.
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations")
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

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

> ---
>   drivers/gpu/drm/drm_gem_vram_helper.c | 18 +++---------------
>   1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 125160b534be..b6c7e3803bb3 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -433,25 +433,19 @@ int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, struct iosys_map *map)
>   {
>   	int ret;
>   
> -	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
> -	if (ret)
> -		return ret;
> +	dma_resv_assert_held(gbo->bo.base.resv);
>   
>   	ret = drm_gem_vram_pin_locked(gbo, 0);
>   	if (ret)
> -		goto err_ttm_bo_unreserve;
> +		return ret;
>   	ret = drm_gem_vram_kmap_locked(gbo, map);
>   	if (ret)
>   		goto err_drm_gem_vram_unpin_locked;
>   
> -	ttm_bo_unreserve(&gbo->bo);
> -
>   	return 0;
>   
>   err_drm_gem_vram_unpin_locked:
>   	drm_gem_vram_unpin_locked(gbo);
> -err_ttm_bo_unreserve:
> -	ttm_bo_unreserve(&gbo->bo);
>   	return ret;
>   }
>   EXPORT_SYMBOL(drm_gem_vram_vmap);
> @@ -467,16 +461,10 @@ EXPORT_SYMBOL(drm_gem_vram_vmap);
>   void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo,
>   			 struct iosys_map *map)
>   {
> -	int ret;
> -
> -	ret = ttm_bo_reserve(&gbo->bo, false, false, NULL);
> -	if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret))
> -		return;
> +	dma_resv_assert_held(gbo->bo.base.resv);
>   
>   	drm_gem_vram_kunmap_locked(gbo, map);
>   	drm_gem_vram_unpin_locked(gbo);
> -
> -	ttm_bo_unreserve(&gbo->bo);
>   }
>   EXPORT_SYMBOL(drm_gem_vram_vunmap);
>