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: <85da792e-d3c7-689f-e56a-9ec2cd7d9464@amd.com>
Date:   Tue, 15 Feb 2022 09:58:16 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH] amdgpu: assign adev after null check
Content-Language: en-US
To:     Qing Wang <wangqing@vivo.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <1644890361-65071-1-git-send-email-wangqing@vivo.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <1644890361-65071-1-git-send-email-wangqing@vivo.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?NkxWMXowcXkrL3EzVFg4L25SbXNSUWJlMlZyOXlwVzVTK3A1VmROVVcxNVNz?=
 =?utf-8?B?dU83M3d5RmVvVE1kK05SK21OSWR5NytvcndUSlF3YlM5UHE4YXNPbEVQc3g2?=
 =?utf-8?B?SENIS2w1c0JhdS8vbktCd1hwNVR0YkxiWFJERkpGUzVKcGZXTFMvd3g4OXAw?=
 =?utf-8?B?YVNKY01HRFROSHYyNURjMFJFWjhSbjZjbXkxZlhMOUtHaGFjQWJwTDE1aENI?=
 =?utf-8?B?UlY4WTU2cVBTZ1R4RmN2d2dwSGZuSHBRRXlocWorSkRNbUJrSWljNlVvMFBE?=
 =?utf-8?B?REdMV1NOTGtwcVJzYkRJSkU3NHhwSEFUSUJmQi9ZNkZFY1BhcVhQM292b2N6?=
 =?utf-8?B?bXFIY3hsbFRIeXUzdVlXNElNQmY0Tk5Ka0xtOTBHL1Q3RFpndkM5bTk3aVdo?=
 =?utf-8?B?RC9HVzVCVGRnTW5ETWNYM1V2TFI4WWNmUWVRdlc0S1htemNNNVQxWEtJY2lm?=
 =?utf-8?B?allWd01KdlBxaW1iYlpZRko4MEtLOTlFSGR5SlpiRENJQk12UllDSXdFTVpI?=
 =?utf-8?B?dUxMdGNwQkRzanZjVXo1VHJrb3BWblZUb3BuVHV4aUYrLytMTEh6L0psTCs1?=
 =?utf-8?B?REdsdEcxajVpcnlwSk9pRGQ0bk1uZzF0KysrM05nUHNCWnhjVnAzWXM0clEv?=
 =?utf-8?B?U1g4d1laU3VBdkl5UXlsNStzUU5zMUNmVERFWkl6ZjFJMDRoMUZZZUtFT2dw?=
 =?utf-8?B?b2x1Ni8zclJQamJQcXJ4RS9tbTBxLzBsdmtlaHpUUzlXbUNNbHNLdEptUm5R?=
 =?utf-8?B?SGdCR1BaRnd6RTB4VXFJQTk2KzJsWGVGRFU5WHFCcGwxcVI0cU8rN3BkUFVP?=
 =?utf-8?B?Vzl6N2g4cHl4Rk03eWdLcGRDRmJxNjFsZXQzQUN6Rk9sS21nM0FrQS9pMXFt?=
 =?utf-8?B?KzdMSWVodWdYWi9nUFIxSzJOQnN0TFIrQW1iTmkrYjJOVGcrbnFPeVdLckZp?=
 =?utf-8?B?TE1vYVVRb25qTG1MeXNvZXlZbEpwY3dIV1J4N05YQkRGTUFwOGJ4QklrWWR1?=
 =?utf-8?B?MDdZMGJnUU1oM2hJbXRpZFlHR3dTZEVJaTNRcDJCSUJQcjlEMHFUZXBUaDRH?=
 =?utf-8?B?STN0MnRtMDdwdDk0akkyZHR0V29SYS8vejI2ZmpBK2M1VW9HUXZldHF3UmZ5?=
 =?utf-8?B?V20wY1M3VU9hSFR6VlRmL0NsN1UwbmpPT1FtRUNndmFqclpFbjFneHVMallx?=
 =?utf-8?B?Tkl6WUh2MUFMRG1vcTkxMHlDMmdQNUphQ09OSXlrUW81TzUxQ3k5UHpxWW4y?=
 =?utf-8?B?OUlickMyVi9wNVFUVTRlRm9IZ2xraXY0ODVQSlNkUi9iMm4rTG5sRTE3TlVR?=
 =?utf-8?B?d2hTVVRNTVpQMzdFdjk4NkFDMmZtdTdYc3RLbGJabklPODIwYlpWUTFEQ1FE?=
 =?utf-8?B?OUJxdXhybzFJaVFkRUk1alRpMC9TVXdRQWwrN1A1TG40UERZUjIyYUxSalR5?=
 =?utf-8?B?amxyMGhVL1VPVlc0eFZ4L1lQMVdCVkxNdHBXTm9TVmJ6YXVUcm03S2xvMnhN?=
 =?utf-8?B?dFhwaWFJREpTN1RaYlZRbEFac01JdUFpaUtBeGo0NEZTa3RTaVhRVU1HTFV5?=
 =?utf-8?B?VWhiaGs1MUs3ZHpFRS9lTUlLSmFZN0I1K1JUejVacmRKRm5Ddlo2QWhrQVZK?=
 =?utf-8?B?dVRlMWdIZGltejF2WGNpV01YZWtHc1dnZ1lEcUJHdi9mUk54VjY0SnJDSkdS?=
 =?utf-8?B?OVVmQk5yZi8xMUZwT3ZMdVNiWHdhSnFuWVpheW40TnU2Z0FmSHNpL1BUNG9E?=
 =?utf-8?B?cWxZSkRmeHRUb3p4aHRkOWNwTXdwQWJQd3RXSEsxb2RxazRKNzFsRjlVWXBC?=
 =?utf-8?B?V1IrL3V1ZndqUS9oNUhhZlMvZDU1QXNZT0NHamh2NWxwWDNDSklWN2F0S3ZW?=
 =?utf-8?B?N28yMDdCQTNEQlR0SStMb2NDdTNobCt0TXQ1SG1CZTQzNUlvYjQyaC9HUUlZ?=
 =?utf-8?B?U1FmazNZWUZWQmF3Z2wvNWRYSVhJbzVseGxaWnJqQ3I4c1Q4VXpqUml6M1hs?=
 =?utf-8?B?QzRQbjBTcUJEeDdkQWgvcGlVdDZnd1FYci9Hb1NmWmk2amxoQVZWUEdhU2Zm?=
 =?utf-8?B?R1RtbldnQllFbFh0NXNabnRCaEJCTXBqc3F0K2UrYW12Z0JRZVhXUXZ1QXpx?=
 =?utf-8?B?SUZNQXNpWFFBdDZxcEZsb2lBZ2U1bVMxYkJhT2twTldDcVZnZjYzbGtBYWVZ?=
 =?utf-8?Q?zMATRX67C/hVYxVnrU+RGU8=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 159a936c-910e-4093-dfa0-08d9f06151f7
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2022 08:58:23.1234
 (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: w4K0qwjUeTkZxLPPm+v0t7+ac6ZKqrqDwmUNoxZk/yy/fkm1cW0wnQH4T0YXLwsa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4201
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,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 15.02.22 um 02:59 schrieb Qing Wang:
> From: Wang Qing <wangqing@vivo.com>
>
> adev should be assigned after a null check

I would rather remove the NULL check altogether.

The caller is supposed to make sure that the context is valid and even 
if it isn't that's not a recoverable error here.

Regards,
Christian.

>
> Signed-off-by: Wang Qing <wangqing@vivo.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 2c929fa..da114f7
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -259,12 +259,13 @@ static void amdgpu_ctx_fini_entity(struct amdgpu_ctx_entity *entity)
>   static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
>   					u32 *stable_pstate)
>   {
> -	struct amdgpu_device *adev = ctx->adev;
> +	struct amdgpu_device *adev;
>   	enum amd_dpm_forced_level current_level;
>   
>   	if (!ctx)
>   		return -EINVAL;
>   
> +	adev = ctx->adev;
>   	current_level = amdgpu_dpm_get_performance_level(adev);
>   
>   	switch (current_level) {
> @@ -290,13 +291,14 @@ static int amdgpu_ctx_get_stable_pstate(struct amdgpu_ctx *ctx,
>   static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx *ctx,
>   					u32 stable_pstate)
>   {
> -	struct amdgpu_device *adev = ctx->adev;
> +	struct amdgpu_device *adev;
>   	enum amd_dpm_forced_level level;
>   	int r;
>   
>   	if (!ctx)
>   		return -EINVAL;
>   
> +	adev = ctx->adev;
>   	mutex_lock(&adev->pm.stable_pstate_ctx_lock);
>   	if (adev->pm.stable_pstate_ctx && adev->pm.stable_pstate_ctx != ctx) {
>   		r = -EBUSY;