Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c3ea3992-df42-3a26-6d30-834368faf91c@amd.com>
Date:   Tue, 30 Aug 2022 10:32:46 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: crypto: ccp - Release dma channels before dmaengine unrgister
Content-Language: en-US
To:     Koba Ko <koba.ko@canonical.com>, John Allen <john.allen@amd.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220830093439.951960-1-koba.ko@canonical.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <20220830093439.951960-1-koba.ko@canonical.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?LzBDWXBSQitTeGRObE83azNUaXFLWUVDNFN5WHVveU4ydGpGVTNaZnI3MS83?=
 =?utf-8?B?cTlTMVNsYVNhUFRIaVRZd0xSVy9LaG5zTjdQdmc0dzlnZHBUVkdvaDM5bVNP?=
 =?utf-8?B?emt0Vk5odkFaQlFwQWIxbjdUQ3NXdEhCdjY0cFRxZHFYRnlKMDF6clJWUXUx?=
 =?utf-8?B?SnZkd3dORmduOFlPcHN4SVpkeitvSUtaSWY5UCtsUloyVExEbDFBcm83dU1H?=
 =?utf-8?B?UDQ3TVQyMnYvc0IwbWtNUGVXVnlqbVcyOXg0NzJ4MXkrb05iWXZnY3FSRVla?=
 =?utf-8?B?bXQ5TVhoQ0JzY1BsMG0yd1NONTRSa0RIeFZHaHI2cU1XVDZKc1JJd2YrVzhI?=
 =?utf-8?B?L3NrMHNLR0NLcGtnWVFPWEpGMEpFSzBTRVcyaE1PNXdTMlgvV2dhVko4bkpI?=
 =?utf-8?B?Y2ROVmtCcCtFU0dSVDB1S28zOFRkOFhqUGtlY3k5djNYWjhndWRUQVRzcEVL?=
 =?utf-8?B?MWYxaElnK3VPVllTK0FmT3h5NGRDb21ZaDdNQzgzeTFHeWg5N085dEdCNm03?=
 =?utf-8?B?WkU4UmVxdjhYcXhBVTZrTk5CQXU1QTFHSm5BRjU0TmNia0R6aUYvVUFZZmov?=
 =?utf-8?B?VkNZODYydkZXSDkydTFOdFB6cHJxUFBwbVVRWEFaS1ozdUpudDA4a1JMbXZQ?=
 =?utf-8?B?MlRSMVVHZ2JHTkdla0JwMHdrVW9hNVo0QTZoUDVnajBybnpTaE1yQy9SdGZa?=
 =?utf-8?B?TGRYam1EWUk0VTQ5azQ3NEVnTzRFa0ltWWFjT2wyVkNkSjFJM2NMWS9YZEJx?=
 =?utf-8?B?a3pDOGViUFMvcnFFbWxCbkNQaDhXMUlIL0dibmRLbEFUdlVocWdqaWhaM1RQ?=
 =?utf-8?B?Slo1M0dBS1dNK0VFaHFWSG9ZQS9JYWFqQlFXM2pQdTNlYUlPWGNUanY0eFhR?=
 =?utf-8?B?ZENHV2M0VTliNTF5VjlnTDBJYjVMMEwyR1VTUnVxZHdHLzhsQmpKbnBNY1RE?=
 =?utf-8?B?L0txRkVSM2RTb0dUbnVEdTFlcHRKVGF1Y0VhVnpTTmJEWDJIQXVIcytJdGZH?=
 =?utf-8?B?d2FFKzUvMjVkcFFuMnRPWXM4V0VBcXRaeUVqV3RZOWs5d2xzbXlJT0ZJOWJp?=
 =?utf-8?B?Zks1cUVXTUNCMEwrcVJLazNCNDAzWVUrcjgrZnlVSnRROS96T3FBSW5YbHVE?=
 =?utf-8?B?YXBFRERvRXp5cUpFWTlma1JJdW1kSzE0V2todWVrdE1TamVLWDJXL3duTWRW?=
 =?utf-8?B?UU05SitUZzR0eGZHTVdqRnFHcndTOW1aTnlWQ21xZnZ3cDZrQjNIRnFCRFVa?=
 =?utf-8?B?THlERHl2N2l0b21BaHVDOEswWlZUd3UxMTZpamVTYzFQUDVjMm0ydXBmWnJk?=
 =?utf-8?B?bjN6aStDSDUrT3hOZmoyR2xGVFdXaGlZRmpPaVlvUG1YVGY3NlZsUEgxeER3?=
 =?utf-8?B?QkoyTVZBa0tMbE45UGRMYm8zSVgwYVVoZ3N2cjd2aXdoT2x0cWJ1c1dFTDVH?=
 =?utf-8?B?V1NISWpzYVBnelJSSVZjN0hMRWU1cGIyUkxSb1Rla3pBMm1zNnVuZ0hOWWll?=
 =?utf-8?B?b0NHQjhybllwdnpoMFRiNWpLNHVYOVpMWmxGTlNtRU1LZDhEYlh0Z3pkYm5r?=
 =?utf-8?B?RkRWa05LSnBzNGJLN1Vta3J2Q3NvQzhLUHNoajYvaEdHeWtoZVpRNEwwbUNx?=
 =?utf-8?B?ZCt0MmZYOUM4TFhkMGpzcWhWcll2MTVhNVQ4aUNyZnlrNlhSLzU4dU9LL0Nv?=
 =?utf-8?B?djlBL1VOaWZ1TkVZYnZackZHOUJEUGcwQ2lVWVdlNUlxWEJZTk51QytOY0Jp?=
 =?utf-8?B?UGpmaGdOL21SUVNROEtnWlRGYm1scmpmRmxSYTJIbXpKSGJZUExYdmwwNith?=
 =?utf-8?B?VjhEQ3VvdTFzSldXNys2UXE2b1JacjAwNVdqTUI3S2N5andsZUNmbzhvVTZY?=
 =?utf-8?B?b1pUQy9aeUZBU1JUUTdkblUwQXpKVUhRS3BlT3pmUStSSUVZdFlESnRBclZM?=
 =?utf-8?B?QUl2WDlTRDZEa3Y0MWJjQzBlbjQrdW9rcDY0YUplanZxYzlwdWp0c0lyQ3NN?=
 =?utf-8?B?d1lMLzFGMnNjUnVHRDNYS1VJOHAvcDBQN21qY05NekNUYkRPOG5XNmMxa2hn?=
 =?utf-8?B?WW1LVFIwcmlBVy9JWE5QdEh2cXUyaEh1aDNoS1AyRFBRYmsrWlo0Rys4TkpZ?=
 =?utf-8?Q?E4tf+2Gwof+W/AThEnkbx9gE2?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 092521bd-5a50-449f-f806-08da8a9ce4a0
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Aug 2022 15:32:48.3506
 (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: BuufsAVh94TFDqWv91VeYL96hitVhZ7vJmaPJfkTynYseBcAIZ/KYT0Y5Q3MIwzsWbdfKBFJeyg45KbPfuxi8Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR12MB2579
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-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 8/30/22 04:34, Koba Ko wrote:
> A warning is shown during shutdown,
> 
> __dma_async_device_channel_unregister called while 2 clients hold a reference
> WARNING: CPU: 15 PID: 1 at drivers/dma/dmaengine.c:1110
> __dma_async_device_channel_unregister
> 
> Call dma_release_channel for occupied channles
> before dma_async_device_unregister.
> 
> Fixes: 4cbe9bc34ed0 ("crypto: ccp - ccp_dmaengine_unregister release dma channels")
> Signed-off-by: Koba Ko <koba.ko@canonical.com>
> ---
>   drivers/crypto/ccp/ccp-dmaengine.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/ccp-dmaengine.c b/drivers/crypto/ccp/ccp-dmaengine.c
> index 7d4b4ad1db1f3..ba3eb1ac3b55d 100644
> --- a/drivers/crypto/ccp/ccp-dmaengine.c
> +++ b/drivers/crypto/ccp/ccp-dmaengine.c
> @@ -641,6 +641,10 @@ static void ccp_dma_release(struct ccp_device *ccp)
>   	for (i = 0; i < ccp->cmd_q_count; i++) {
>   		chan = ccp->ccp_dma_chan + i;
>   		dma_chan = &chan->dma_chan;
> +
> +		if (dma_chan->client_count)
> +			dma_release_channel(dma_chan);
> +
>   		tasklet_kill(&chan->cleanup_tasklet);
>   		list_del_rcu(&dma_chan->device_node);
>   	}
> @@ -762,12 +766,14 @@ int ccp_dmaengine_register(struct ccp_device *ccp)
>   void ccp_dmaengine_unregister(struct ccp_device *ccp)
>   {
>   	struct dma_device *dma_dev = &ccp->dma_dev;
> +	struct dma_chan *dma_chan;
> +	unsigned int i;
>   
>   	if (!dmaengine)
>   		return;
>   
> -	dma_async_device_unregister(dma_dev);
>   	ccp_dma_release(ccp);
> +	dma_async_device_unregister(dma_dev);

Seems the right way to get rid of the warn, but can there still be a race 
where the channel is grabbed again after ccp_dma_release() and before 
dma_async_device_unregister() is called?

Thanks,
Tom

>   
>   	kmem_cache_destroy(ccp->dma_desc_cache);
>   	kmem_cache_destroy(ccp->dma_cmd_cache);