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: <d88185fb-0c8c-f379-a6ea-856c00a4e5c9@amd.com>
Date:   Fri, 9 Sep 2022 22:02:34 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.1
Subject: Re: [PATCH v2 2/4] dma-buf: enable signaling for the stub fence on
 debug
Content-Language: en-US
To:     =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        Arvind Yadav <Arvind.Yadav@amd.com>, andrey.grodzovsky@amd.com,
        shashank.sharma@amd.com, amaranath.somalapuram@amd.com,
        Arunpravin.PaneerSelvam@amd.com, sumit.semwal@linaro.org,
        gustavo@padovan.org, airlied@linux.ie, daniel@ffwll.ch,
        linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org
References: <20220905163502.4032-1-Arvind.Yadav@amd.com>
 <20220905163502.4032-3-Arvind.Yadav@amd.com>
 <37ec6f1d-a3f4-6fa3-6d5c-fe89bb958d87@amd.com>
From:   "Yadav, Arvind" <arvyadav@amd.com>
In-Reply-To: <37ec6f1d-a3f4-6fa3-6d5c-fe89bb958d87@amd.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?aS8yRVpEWlQ4OFpDYWxvVWREUXdGbjArY3A4WkgzZUZDaTI1eEZrbXRUS1M3?=
 =?utf-8?B?YW5pVGEvVDdRS2VOTy83Qk9xTldQL0RXdWlubWI5MlhHVXE0amp2WnpNNXhi?=
 =?utf-8?B?a2lWZHJYUUxKVjUyS0dtKytia2piM3NTb2pwRFBSZ3J5b3Q1bloyNEZGa1R4?=
 =?utf-8?B?WmpYN3NEZVJqZDN0dE5DSzlUQXc3Z2ZIS05OY2xEUWROR2tWWHNjUU9ZNkQ4?=
 =?utf-8?B?bHZWNDAvTWIzSlhPbG5VbmI0M2pJVDRsSGM5S3laM0VsVEE5K3Z5dDJKejVj?=
 =?utf-8?B?YzZ3aTdwcjNta2haaEMxQWtHU2s4YnZDMEZmUTE4UnpqNk1mdTNwUjVzWTc5?=
 =?utf-8?B?MmtCU2ZKRjBiVHpqRmEzdEYzbHZzWGNYOWFFY01DRHQvVHRnekE4cnRjZ081?=
 =?utf-8?B?Q0ZjQkFYTDd5cGNKcnNnSVBTbHZVUHQxRG9tRkhIcVdFVm5RT2xXeHEwU0hh?=
 =?utf-8?B?T0ZzTjJuaVZKd2RTS0dQalhNN0FDT0RNM3Q1RzFtS2JJS0lNVUxqNDFrMHhs?=
 =?utf-8?B?YmdPbkJhQXJ0eEZWY09iTlNnOGJGSnpLSU9uSEYwbEt2WkIrTlNGd2pwcGVo?=
 =?utf-8?B?ZVduMHErMzBpNHo2cGZ3cDdaWFRPZDBGQXlGWjg4aVVaQldEVEpOeDB0NVBq?=
 =?utf-8?B?cG5PaGYrSzhmOVVVcUo2Z2lXL2NvQlJ1ZlRDU0g1NkdRWXZiaVZCd3pZUk5Q?=
 =?utf-8?B?cmpidThYOWJLdWNEMjZpTTlPZEc0eXM0WmFIYU9oekV3NGNOQTdlRTc3ZUR2?=
 =?utf-8?B?YnRvT3VBaGo4ZUtIZW5CZ0JFRVM1YkFnZS96eWdaaHU4T20wYkh0OURtcEVF?=
 =?utf-8?B?R0pmM2F5cUczMWNqYlNhWW5YalJVM0k2UFRUZDVvZVFVUWlDNlVsQ1V2RVBj?=
 =?utf-8?B?NFFvRVN6djFIejZNOVhqUDlqdEdqVmtCR1k0MHhxc09JSEYzRXFwRWRqdXAy?=
 =?utf-8?B?UjFzZllRRGNtelYyczlHVkNOWHRoUDVsVE1UUGkxYWpZSDdTd1FxZDcvNUY3?=
 =?utf-8?B?QzBCcEQxcDF1U2craVhwUG9IbjhucnJjUEV6ZU9udFMxRE1adjdIa2dwTlNo?=
 =?utf-8?B?cEIvaDI4amh4YTUzQXM5SVN5dGU2SmhvWCtrd0ZDeCt6cEw5elJsZ1FHR2tZ?=
 =?utf-8?B?SnhYYk1Fc2EyL1htWmtId1dSSFEvUklqSFNZOWhPVGMwTG5yVUxyQnlTTTB0?=
 =?utf-8?B?bnNOY3JNM3VWQmNjQ21yK2xyOE82WGp4TVBzQzd0eGxBQkZyaXZZY21yQmcv?=
 =?utf-8?B?L3pFZzdxWk9Ya1FuN1pSWnRhUWdYT1ArekJWN2ZtQzNVNHlVMW55dlhBNWtx?=
 =?utf-8?B?NmlKQVVZcENvRllyelNaNWNBeHJ5dzRtMjlUY2lScmp3YndraVNpVmd1V1FV?=
 =?utf-8?B?VXd6RVJ3M0dDMTY2YnI1d0JFY2lHMWVPZDJ5bVN1K0NmZ2RpWU16dFVuZlBt?=
 =?utf-8?B?U2NMSU5xUS96WUZwSmo1K2I5Tkt5NzRiUnpwdFZlOFNEbE1oV3owektkbzM2?=
 =?utf-8?B?Rkkva3duVVhQS1AwUjhBdjBaNGdzdTlEZDNHMitWMDRaanJ1Ui9MZ2VON0ZZ?=
 =?utf-8?B?amRuRDVkY2NqblRmZXh1RlVrUDd1c0QvQmhMNEp6YjZ1SHZadWJ0TGRxZ0tp?=
 =?utf-8?B?TXpEcW5LZWFobDBweE1HY1lsSi9BRERSbzdUVjVYdDA3RUM2NmFtV3FwdzZi?=
 =?utf-8?B?UzZBUWkybXJaT2lzYUp0MGxFTWhva0NSc1poa3lEWDZwU2E1TlJuN1MvUXNy?=
 =?utf-8?B?aXE1b1ZwRFBQSVltdEZKLzJNMFM0ZEhEdjJ5RUVjb0FjZUFIUGpvdVFmaDk5?=
 =?utf-8?B?d1kwMTdPaStEM1dlZlpJTC95R01GS1U2dlp2VzQwSmJuMDNCZzJvT1BQT0lv?=
 =?utf-8?B?Smk3cUdqU015OHVESnhjS1E2UjJTQkwya2hnbjFHbXZSUDNIRnpENmYrL05l?=
 =?utf-8?B?NmkrZG9JMHBaVGlOdG5McGlJbGIyeHVudUJ5NVlsY3Bxd1F6OGsyZk5OQmx4?=
 =?utf-8?B?S29nQko4NnRkNXVTanZDN0RtOFlPdG16SjhCWTBvL1dMK1ZuSWJXY3lQUGU0?=
 =?utf-8?B?U0FKR1dEQnNESEZqRGhuNi9QWXNZRU9hdlhJdWNaOXM5cTk1akVkR3NSdDRD?=
 =?utf-8?Q?SAgAbAZklDZhuGZVJUzjkdJKI?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a0c87008-84c4-4719-c8de-08da9280ee05
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB6000.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Sep 2022 16:32:47.7702
 (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: mHJRgWcGsQ5WC3SkO0h9/9suwF3Aq/VMnQJ/cVlJQcv6Hhi9wvfeC67ki5vyVLv66nXno5sxEq4CLEtcQRGGBQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6210
X-Spam-Status: No, score=-3.2 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


On 9/6/2022 12:39 PM, Christian König wrote:
>
>
> Am 05.09.22 um 18:35 schrieb Arvind Yadav:
>> Here's on debug enabling software signaling for the stub fence
>> which is always signaled. This fence should enable software
>> signaling otherwise the AMD GPU scheduler will cause a GPU reset
>> due to a GPU scheduler cleanup activity timeout.
>>
>> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
>> ---
>>
>> Changes in v1 :
>> 1- Addressing Christian's comment to remove unnecessary callback.
>> 2- Replacing CONFIG_DEBUG_WW_MUTEX_SLOWPATH instead of CONFIG_DEBUG_FS.
>> 3- The version of this patch is also changed and previously
>> it was [PATCH 3/4]
>>
>> ---
>>   drivers/dma-buf/dma-fence.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 066400ed8841..2378b12538c4 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -27,6 +27,10 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
>>   static DEFINE_SPINLOCK(dma_fence_stub_lock);
>>   static struct dma_fence dma_fence_stub;
>>   +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
>> +static bool __dma_fence_enable_signaling(struct dma_fence *fence);
>> +#endif
>> +
>
> I would rename the function to something like 
> dma_fence_enable_signaling_locked().
>
> And please don't add any #ifdef if it isn't absolutely necessary. This 
> makes the code pretty fragile.
>
>>   /*
>>    * fence context counter: each execution context should have its own
>>    * fence context, this allows checking if fences belong to the same
>> @@ -136,6 +140,9 @@ struct dma_fence *dma_fence_get_stub(void)
>>                      &dma_fence_stub_ops,
>>                      &dma_fence_stub_lock,
>>                      0, 0);
>> +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
>> +        __dma_fence_enable_signaling(&dma_fence_stub);
>> +#endif
>
> Alternatively in this particular case you could just set the bit 
> manually here since this is part of the dma_fence code anyway.
>
> Christian.
>
As per per review comment. I will set the bit manually.

~arvind

>> dma_fence_signal_locked(&dma_fence_stub);
>>       }
>>       spin_unlock(&dma_fence_stub_lock);
>