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: <bcacc450-5e09-9ce7-e389-0a564da2bcb7@amd.com>
Date:   Fri, 29 Apr 2022 12:34:19 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH] gpu: drm: remove redundant dma_fence_put() when
 drm_sched_job_add_dependency() fails
Content-Language: en-US
To:     Hangyu Hua <hbh25y@gmail.com>, yuq825@gmail.com, airlied@linux.ie,
        daniel@ffwll.ch
Cc:     dri-devel@lists.freedesktop.org, lima@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
References: <20220425083645.25922-1-hbh25y@gmail.com>
 <b7d8956e-5ac6-8200-d8b1-4ce828f04665@amd.com>
 <79b198d0-eff2-d658-4b5e-9084a834fc93@gmail.com>
 <e26e6ef8-6665-0b9d-804f-cf107f1788d4@amd.com>
 <88dd5d67-7dd5-2f58-5254-adaa941deb0f@gmail.com>
 <65b6cc23-1a77-7df0-5768-f81cd03b6514@amd.com>
 <d0790635-4b2e-cd58-0a51-36427800b39c@gmail.com>
 <9b91e06c-50fd-b567-7d2b-a2597d01c4dc@amd.com>
 <37a48008-0700-7c60-30d6-ac2f4152114c@gmail.com>
From:   Andrey Grodzovsky <andrey.grodzovsky@amd.com>
In-Reply-To: <37a48008-0700-7c60-30d6-ac2f4152114c@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: 2
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bW51YkNuQklUL0pNaVFlUXAxTXZkaDF0aXMwVzNOTDVkbmZRMWo2UGhXTUlM?=
 =?utf-8?B?ZlAydFFsQXY2WjUxOVIvVUd4Z29IK2hiQTNhN1FSQllYLzV6UUxyKzd5bzVB?=
 =?utf-8?B?eEhlSUZDMVZham4wVUw4eW45c3VxSlpuSDZrbU02Q0duR2ZoTGpMVDFhYUhI?=
 =?utf-8?B?TkYxbDRXVzVQS1RaSnFrbGt4WVNnZ0JacUNzcktjUWhmUXE3NnNXUzQrRUZ1?=
 =?utf-8?B?WGljaUppOEx6QVkrZTFGcngwK21TZVJuZldPSjNaeTZxRk8xZHZiV3R0d1dy?=
 =?utf-8?B?WHFiZHByajEvRFZ5TGE2MW56QjZPWXZwck51U08vR2pLMjNiMEYxc1BEZEVp?=
 =?utf-8?B?UXdYMkxsSFBaZXdoVVZla1J1eTg0VXI5WHRzTXNIbXNmZUhNeVZtY1MzcEFO?=
 =?utf-8?B?K2gyWElKcjdNVDBCdFBQZ1dManZVb2g4c3lDOE9HcXMxTTBwbHRzQVZ3TmUv?=
 =?utf-8?B?RitOR2l4UUkwQStlRThKZHByMnJkR041MmE2VlBCTHZCVnN4TU0yQis1S3Vv?=
 =?utf-8?B?RkJ2QkJqaHFJNkwyQUtXZGhwOFZ0US9OSUxzR2dxL0RuZ2ZyQnVtNE1DV3ZQ?=
 =?utf-8?B?WisyNU8xdVJUeFpkczg4Y2ZlTnRxUURNYWhMdjVEWUFlR04vOU5UY1JINUww?=
 =?utf-8?B?KzdweHh5WmhGRmFSaVJMVjNrU0diMFJDamwwbFh5OFN5V3hIODJua1Q1SlZn?=
 =?utf-8?B?UnUvaFNnUnNvb2syK1Z5QWhHUEVScnRuWFB3RWplaDg2Nk0yZXFLT2djNmpm?=
 =?utf-8?B?OVRXNmVlTzg4MGpDdnBEZ0NYaWNFWGJxNVN6WGY0YWhSSWM3Um5tZGk2dm1h?=
 =?utf-8?B?OFAxNmFCMHc4YTlGa3BDamQvYnY5Y2QwZzJXbW1KbElYKzMwbVcvckVFUzAy?=
 =?utf-8?B?RlRZdGpXaVB1Uzgzbm1QSHFBMFBwUVMvSTI1b2Q0anE0ajFrYVBrbUVDSDgw?=
 =?utf-8?B?ZzhkdXRJVWR1ajMvZURBSCtTeUZTNUJpKzRua0Q0bk13WjV2N1hxSWwxVHR3?=
 =?utf-8?B?YWpBVUVpT3pobGZmK09NdC96VEtmcWV5VC9ZS05adzdON1VJQnR0ditTcExK?=
 =?utf-8?B?cUdFN29OUCtNZTA5a3QzZm82TWo1bHU0QXNGYWxiWjVsWkpmNHBKTzlrRVBv?=
 =?utf-8?B?ekFjbTFJazlRbmN4ekY5b3FJZTdQYk40bmdnSjAvWW9neUU3ZEpHUlVpTVpE?=
 =?utf-8?B?OUVnWVFUalFjR0tmYkFCMUF5Z2NlektaMWdndG96a0pBcVNYS1FCK1ZBZ3R1?=
 =?utf-8?B?WkgrK2xtV1VDL2NpK0RyOHg2ZkFhcFRXR2JBeWZ2NHN6cDJtSHI4L3hYbEFJ?=
 =?utf-8?B?RFhXZzdIbWNQL2pPbS93azc1Vno5VW1pSGJqSnZrUlNPbFVQWnArU015Z1lB?=
 =?utf-8?B?QVJnZHY5OUVqWFZQbm1USzg1bE5GeFhRQmRPMHRSbGprV0E3TDBMbXVwbnNU?=
 =?utf-8?B?QmtYZWlpS3A0VC9xWFIyeElma3o4Ym9hOUUwN0JFQWlhdUV2bzFNQ2NaVGt1?=
 =?utf-8?B?eGlKSWhaYm54T1ozN3pjRlM1Szl4d1JCV2JUcEQzeGE1OGRDZ1d6UWt6MTg2?=
 =?utf-8?B?SDVzeC9LbithWWx1QXh1b2FnUkEwYVdrZGJ2VFh5OVlLWFBoa0hPMW9JbDNI?=
 =?utf-8?B?TzFBdmJiVlk4bU5LWlFPd0ZsOWhYTTBDbWhCQ0QwQkVxcWo0YWVBc3dCZ3E1?=
 =?utf-8?B?SXVpYUFFdGg0TXJUNGN2aXVoaXBqY014cVVYd1o3dU8wSVNkOGRRRnQxR0Mw?=
 =?utf-8?B?cVRZbjBJNlN4N2hxYndLKzk4RkhXYU1wOHBqSXJtTXRWL3lhdHNmK0htMlRN?=
 =?utf-8?B?S1ZvTWlnYVprS0JoeVdMenJ0ekZnTEI1OVJuZHRCTkd4d2hMZHFoYTlqT25M?=
 =?utf-8?B?bXdUSXNhbE9EU2drd1JXVi93amlnbVVwYlh1dWY5azRpdDZLaHc1QTFtNGlm?=
 =?utf-8?B?Z3dmazlFNS9ZbWJCU0JMQXEyMGluZWtyRzBBcFVGUmJBdjlnWnRhdWFxVVpK?=
 =?utf-8?B?SEtmWFlZQzM1N1VRdElJRkRuSmpFa3AwZXgxRjh1UG9xT1hhZ3JBWXdjRDNo?=
 =?utf-8?B?c0V1endUMkZyVEhlWnBZR0VmcXpYekxCUGJtSlNTdlBCZDJ4OE1ZN1Rjblls?=
 =?utf-8?B?NnVwMU5Ka1VEeEFvZVZaZWJNc1pKbG1IbXhtM1dtaGs0NTlSSUViQ21naXRq?=
 =?utf-8?B?UkhrZENlZHEzR3ZBWDVjUHNBL0tkVjR2cXh6VWN6K0FvMWljbXluMHczZGh0?=
 =?utf-8?B?T3lQaHJnbDYvWHg5YktJVXZXc2gxV3hUVTQ0K2krVi9xMHNNTkp1b2RHbFRw?=
 =?utf-8?B?STJWYXM4bjFTVWxMZnlYSnhtR3ZSWWJHaEMyNHdnYWlWTWovenJZUlVUdk5P?=
 =?utf-8?Q?65hH4zlVymtsdfL8Hl+0wjkW3TbULXxWRO1ofmp+1Oiad?=
X-MS-Exchange-AntiSpam-MessageData-1: LrMhmjbojx9z5A==
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f72f448e-15a4-4cb9-77d5-08da29fe1d3c
X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1947.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Apr 2022 16:34:21.7311
 (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: 6uZNxbwXJNcKICs1oLiQmitewIdVn+KgwuA98+UFLIaX8O7Ln9IRhW8AX1oUm658UB6U7MwARAyE8w6IS18WHg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5063
X-Spam-Status: No, score=-5.0 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

Now it's easy to see it's redundant, it's a problematic design where the 
fence 'get' part is happening outside the job->dependencies array logic 
but the fence 'put' part is happening inside, leads to confusions such 
as adding this 'put'
in the first place. I will look into improving this if possible.

Patch is Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

Andrey

On 2022-04-28 23:03, Hangyu Hua wrote:
> If fence is released in drm_sched_job_add_implicit_dependencies(), a 
> dangling pointer will be in obj->resv.
>
> specific scenario:
> recount = 1 init, obj->resv->fence_excl = fence
> recount = 1 before drm_sched_job_add_implicit_dependencies
> recount = 2 in [2]
> recount = 1 in [1]
> recount = 0 in [3] <--- fence release. But fence still in obj->resv
>
> Thanks,
> Hangyu