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: <ecc972c1-cf7d-cd99-805b-38dbf04adc79@amd.com>
Date:   Mon, 24 Apr 2023 09:07:02 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] drm:amd:amdgpu: Fix missing bo unlock in failure path
Content-Language: en-US
To:     Sukrut Bellary <sukrut.bellary@linux.com>, daniel@ffwll.ch,
        airlied@gmail.com, sumit.semwal@linaro.org, Hawking.Zhang@amd.com,
        Julia.Lawall@inria.fr, dri-devel@lists.freedesktop.org
Cc:     alexander.deucher@amd.com, Xinhui.Pan@amd.com,
        amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org
References: <20230424055910.15683-1-sukrut.bellary@linux.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230424055910.15683-1-sukrut.bellary@linux.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?eXRqdElrUk5aYmJzRk55d3BVM1NmclRGNVdXeDdxRmZIUS9ZSHI1SndLU1hM?=
 =?utf-8?B?bjdTcG1PUVAvTG5pQkFzZ0NzM1RzaTRsUnNXejc5SFl6R21CZEFkdUE3UkJK?=
 =?utf-8?B?TWN3N2UybXZQQ3VRZVB4bmZKbjZWalcvK1pTWnVXQjdjZ0RJb1ZtQ3NYdUVU?=
 =?utf-8?B?c3FOR1lLY29JV3RFMm1KbEUvNXpXUWhpUno1aXpnL1VrQTAxYlZFVC9ZSzFk?=
 =?utf-8?B?NVE5WFIrTk9yYi82Y3ZUQm9MaUExOXEvSEpHdVN5U2c0VHNicExHQUVxd1VB?=
 =?utf-8?B?TnJiZU9uYm9DZURmMy9XSDRHMnZ4Qm0xYXUxbHhURkVpOHdjdmtiWGdVSW9z?=
 =?utf-8?B?RjNJSmRUOU9BeHRndzhVWktyUmtGZlY4NDNxWkl6MnFQaW1PZE5tVm1QdHV1?=
 =?utf-8?B?RXY3OCsyb01UZ2t2OUlWcGcvdUZJdFJ5M0JXOTZYY1R3a0lZbCttK1paeHlY?=
 =?utf-8?B?cW5ScEt1elZOTkJLR2VZZmRiOVgwT0lyaGwyM0xlWENlSTdGNGd0UlpRSTNQ?=
 =?utf-8?B?WGZXenQ2Z1hjNUljbFdZZWNzVGs1MWdSazd5N3AwSFlSZERCUVdCQXIvMmpU?=
 =?utf-8?B?V3FPNzFlMVBvMjZHYjlqd0FaQnRlRTduaDdWblVpZHdudmx4amcwczV1ZkFa?=
 =?utf-8?B?VU5mRkFWMGRpamUyclV5cTdxYzFHK1FGWWw1b2NaS0hMYUZLa1NFSWIxNlJ5?=
 =?utf-8?B?UXlVQmNmZUNGdlZhckxQVGJSaXc4QXdSZjFQdWNUUlVMc2pkQlFyaFJteHhU?=
 =?utf-8?B?eW95L0d3cE5nY1hyeElaWkErUThUUzFXeW9nTlVsZ3MzNnF0aXUrTE96bzg4?=
 =?utf-8?B?cGFheFhMcmlqWTFSa2JGQzlVblhlMUFXVlpBdXFQWnBVdE1KNjdESHp2RGZH?=
 =?utf-8?B?elEzNEJNaXpDZWE1cWxhVDVEb3FDUGp6c3ZiTXEzTTg2VzdySjd2SkZJVFhS?=
 =?utf-8?B?c25vamJXeTU4WXNSWkdmV2wwKytwZ2t3alN2WlRTZ2pWQlBxTnNTYWttV2R2?=
 =?utf-8?B?OEN0MFdRcldlczJ6L01RYndDRFFVcm1EMVpvTWlFU2VWdzk4dUx4ZTBtd1Z0?=
 =?utf-8?B?YUI2Z0x1ZGEvY3QrVTl4NDdQTXl5dnZqVjlzRlZFL2VocmxkM2pRVGZDZ2tu?=
 =?utf-8?B?bHVCUUdWeHljYWRjR2VZd2FNUEZxcGJNNk5CTTFPTE1iT3l4dWNnOExLeVlX?=
 =?utf-8?B?NE5HZ1hIc0d5aE93TTdZc1Q0U3hoeTA4RkgxSVpKcjFFNjA0ZnN3WlY5VFNK?=
 =?utf-8?B?V1RqSEVMbnJKaGd3Q1ZML2o4cFZDZnJSMi9TOE04cVY1Skw3dlhOYm9SQ1oy?=
 =?utf-8?B?cmtjR2ZxaS91WTZWN0l5c1FZeGpWMS9Db2ZNczlKRC9KUDZLZTJRSE92blVI?=
 =?utf-8?B?b2tDTERwcExzVFZJeVlOdWdSQk5CUzFuWGFtVW53TEt5bE9zWUZFYkNyQll0?=
 =?utf-8?B?N2doeFQxLzBkdnpsNnN4aXJUYisvMjNyak5rbi9lcmozU0tHdjdqellPSUp6?=
 =?utf-8?B?bFVYNnNsNDJ5YTV2Mkx1QlA5R0VQci81K09DeE5NZkN0UHRhbWF5aDhWVVlX?=
 =?utf-8?B?eURWR2NCS2FYQlI0OFZqZFVWc1dMQTFVQWI3SU9FanNYNWsvNTh6UGFiZTdw?=
 =?utf-8?B?cVlMc3dhV0NkaEl0OFpKYy90RmxLanVTU0FuTzE1UmtBclFYcVpYdG9xWTBa?=
 =?utf-8?B?M1gvYlZLejdlOTU4UzdpT3BSZHhrVE0yNTlMYnkxaWZoV09idk5hNXBXakND?=
 =?utf-8?B?bW13YWViR0orVTNyUjBnY0NKa1hubmhKTkcwaWNMaHJ1czJJYWo1QkxPNjV3?=
 =?utf-8?B?RlU5MlJaSVpRTVMvSzMzWmc3WkZXZGN2a0JiRGpiTFBvZ1BXcGYxaXlYYVVw?=
 =?utf-8?B?SXhnc1dhS3VNQ0taN0lySUo2eFptdzBYR09IbTg3VS93KzEzek5NOHA5cFlS?=
 =?utf-8?B?eUdETS9rOFcvaWZqd0tIYUJtRFdRS1dkNmdUTGRxa2MxcFh6NnR0SDFNNW1s?=
 =?utf-8?B?RFE5Wk9BcXdxaVFERnRnVVhOSzZ0UGZiYnF1WnduUlUycm9USC9UMkhvbEdp?=
 =?utf-8?B?bnZsMStCZER2Y0FqeHJ2alE1KzY1WDRySkplQ0U5NVZpUjN4eXBCRzZnLzIz?=
 =?utf-8?Q?8khU=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 806e4d98-779d-499d-32a7-08db4492850a
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Apr 2023 07:07:09.3232
 (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: u+K+nGjJfTgreM46x1b+QQdAKiQ+mQa5rg32FAu6JqyVnR60J/MRCv7IMUmqXoCf
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB6744
X-Spam-Status: No, score=-3.9 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 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 24.04.23 um 07:59 schrieb Sukrut Bellary:
> smatch warning - inconsistent handling of buffer object reserve
> and unreserve.
>
> Signed-off-by: Sukrut Bellary <sukrut.bellary@linux.com>

For now that patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

But for the record mapping/unmapping the MQD like this is a very bad 
idea in the first place.

We could need to shuffle memory around for that during resume and that 
is not something we really want to do.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 278416acf060..5de44d7e92de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -4686,8 +4686,10 @@ static int gfx_v8_0_kiq_resume(struct amdgpu_device *adev)
>   		return r;
>   
>   	r = amdgpu_bo_kmap(ring->mqd_obj, &ring->mqd_ptr);
> -	if (unlikely(r != 0))
> +	if (unlikely(r != 0)) {
> +		amdgpu_bo_unreserve(ring->mqd_obj);
>   		return r;
> +	}
>   
>   	gfx_v8_0_kiq_init_queue(ring);
>   	amdgpu_bo_kunmap(ring->mqd_obj);