Received-SPF: pass (google.com: domain of linux-kernel+bounces-7731-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <77344325-a27d-4479-82e1-bae88e0e9e46@intel.com>
Date: Wed, 20 Dec 2023 19:49:29 -0600
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] dmaengine: idxd: Remove usage of the deprecated
 ida_simple_xx() API
Content-Language: en-US
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>, Fenghua Yu
	<fenghua.yu@intel.com>, Dave Jiang <dave.jiang@intel.com>, Vinod Koul
	<vkoul@kernel.org>
CC: <linux-kernel@vger.kernel.org>, <kernel-janitors@vger.kernel.org>,
	<dmaengine@vger.kernel.org>
References: <ac991f5f42112fa782a881d391d447529cbc4a23.1702967302.git.christophe.jaillet@wanadoo.fr>
From: Lijun Pan <lijun.pan@intel.com>
In-Reply-To: <ac991f5f42112fa782a881d391d447529cbc4a23.1702967302.git.christophe.jaillet@wanadoo.fr>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ajNleVpaME1qdm9ZTWZhQlJqM3NxOFpabnlrZjJxNldzRjZMcU54N2R3NWVT?=
 =?utf-8?B?VWVKUGMxVXM2RnUyaDhTN3E4M3B3Y1pRdHpZWEYwMHd1RG1NK28yZ2VZRVR3?=
 =?utf-8?B?U00wMVNKY1JXZUVSU3JOR1Nwcmg3Z0p1TDd0c3Vmc2lvQXdzL2NreDBFZ0xF?=
 =?utf-8?B?MERhYWlwTGlHQXgzZEtJbkNBeXp6SEREVU1LSkFsWW5HYUVFSEtTV2Q1eDUw?=
 =?utf-8?B?MDNZRjhyNmZDYXdva3FLQWtQTEVZQ0RoUTNzQXpLcHFsaU9hVENMU1dIWGpW?=
 =?utf-8?B?VFRQdGZEcDRRM1ZIVEFlOWVkcUlEdE1HTzR5LzJsTkt0ZXdIU0w5WWdudXpH?=
 =?utf-8?B?MllsNWQybFQ2Y25EUzcrNU1lUWdYb2hyN3BsSy9Fb2VRTDhtWHZsTnpuQUNl?=
 =?utf-8?B?YXliU2VhSlZVYk5ZSURtR3R4UFkydXBja0QxN0I5TjlLOEMvK2ZGbDZpaGpr?=
 =?utf-8?B?NzlWYWJxMzVlZjl2ZnFmWkc4ZzlJaWQ2ampPaWJBWldqK3JnTDBqODlORW1Y?=
 =?utf-8?B?cjNxdUptaTY0WjV4QkM0d21keXFXQnppeXg1ZWd4VjgyalJVNnQ1VGRMQlFz?=
 =?utf-8?B?VVdzME1kWHNIRmMrMTlKMmZHazZvak9qMng2SnJqT2pqYjltTWFLQ29QaEtp?=
 =?utf-8?B?S2ZPWFJQNnhubWs3enVhWFF3N2hKY1FuNnJUeVlURDdWUGdJUkNQVHQ2ZXA0?=
 =?utf-8?B?WmpWVFp2K2pRRHo2dVZLZk5Uemh5ZzNWeUFRRnJVUzdoQ2RpdC9pcFAyUUo1?=
 =?utf-8?B?b1hVWkQwc0NCd2JhM3RVcTVOZUxUV2x1V0JQOFpTVFBBdExQM2syYVB6R1lT?=
 =?utf-8?B?TURma2NlU2lENTZLbXl3Yjd2bVRQS2h0RXJ5bkYwUGJNaytnNGp1ZFpSSVVt?=
 =?utf-8?B?NzBWSkxZc3lVSTRwMmp5VG1XSDFscVpQTlQycGZlOXNLMDFXZmdicmlGdS9S?=
 =?utf-8?B?aHZOcSt6bFFCbE5RRXU3TlFQd0lzM0todCtTOGh3a045K1dGY3ZiMlQ1OTg1?=
 =?utf-8?B?Q0tEenRmakN6YXpCbkdBVisyZ0hlMUxKOFZNcUxrN1B1bFFvV2dnbXBmU2o1?=
 =?utf-8?B?dHUyUXBoUXJLMGpZT0tJVFdwRk92S0E4S1RsMVdvcDl0elMySWVIbEdiNkpi?=
 =?utf-8?B?SmY3K0hZNkR3Q2ROZU1MVzdIdCtmTkVteXNDS3F5RUNBb1JiWEJ3R2w5R0lV?=
 =?utf-8?B?T3FMQ3laa2pwMkFJU3RPbjJqVlYzZi82R3M2UmR5VUNsZS8ySUZhV3ZCclZp?=
 =?utf-8?B?K3grcEVwd28rNjQ2bm55QUxuWm5wenB2SzlYanpnaVIwVUwvZUE2NFpLZmJa?=
 =?utf-8?B?TjM5c1Q4VXJ5YjlnMjBHZjZsdUt3Q2pia1JVbGNLRWpZVm9XQVNWMjBZZkxQ?=
 =?utf-8?B?d2JrMkdRUVA2MnQvbU82K1F1eWVJN25YbkFxVERvd0huTTgyT1NyOFc3TitY?=
 =?utf-8?B?UnczaWVBaFVZL1RSOVVPMFBha2ZERlljUDhyOFMrSWYvaGx3emFWbE1ZMFNi?=
 =?utf-8?B?a3E4MnFjWGVKYUowNFpKL3FpZTUyTEtUV01JVXlFVWc2d2ZGVTdGMjduRHlY?=
 =?utf-8?B?TU1JNTlJRTVaKzBlZmszbnJzL1UrQVdOU2lObHkwR3FMa0NnOGlDQTZGV0l3?=
 =?utf-8?B?UGUrZnVxVzZ1dDA2WnZOTWhXcGNWeWM4RjJGS1M3Mmc3VnNpaWFsZGZmd0hS?=
 =?utf-8?B?ZzJFMHlyVXFKNnNWVUxxWnBpYmlObElzZERiUHBTU1ZFejVhbVF1WVBBdVhh?=
 =?utf-8?B?clNYb09nUlRUZmNQT0EwazY0cDR4WGN1Zk9sTzZiVkZZMHQ3bGVBNDQ1bHlj?=
 =?utf-8?B?RnRlOGgreDNEVFVyN3RTc0dqV0FJSkt2ZGo1dzg4STlpamtsblV1d3MvU3Jm?=
 =?utf-8?B?WGdlWkJ3S1o5ZE5oSHNQS25MT3hwQlNDR2lnSVNWK0prOHFKeUQ2V1poSmp5?=
 =?utf-8?B?L0F1OWljeFJ2cXJrR3EwM3E5SEFBbnR1R3ltOEVHeklLM25ST24zTmRacHFT?=
 =?utf-8?B?OWcxRVBuZ0Vpcm9sRkpUS2tnZWFoSi9IYTFCcHhFWDhOUDhOdFkyVDRGQzlY?=
 =?utf-8?B?WXZCZ0hrL05WQkJYS3J5dElZVnZTN3NnaCtQQThSN2pUWWZOTkhMd081cEpt?=
 =?utf-8?Q?BwABC0AOh7ITZaJvcnffkAMPu?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 24db4f17-fb68-4f4d-e8e0-08dc01c71488
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB7141.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Dec 2023 01:49:33.5809
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6sBg0v6mTlTot7vCqjzX9XnsckuONQKqH3bhn6I9+9CeMho7OEmxFlYIEJRTO8TrYyLaHAD4Cwhzh79ewXqU6Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4834
X-OriginatorOrg: intel.com


On 12/19/2023 1:33 PM, Christophe JAILLET wrote:
> ida_alloc() and ida_free() should be preferred to the deprecated
> ida_simple_get() and ida_simple_remove().
> 
> This is less verbose.
> 
> Note that the upper limit of ida_simple_get() is exclusive, but the one of
> ida_alloc_range() is inclusive. Sothis change allows one more device. >
> MINORMASK is ((1U << MINORBITS) - 1), so allowing MINORMASK as a maximum value
> makes sense. It is also consistent with other "ida_.*MINORMASK" and
> "ida_*MINOR()" usages.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---

Acked-by: Lijun Pan <lijun.pan@intel.com>


> Changes v2:
>    - remove note about compile tested only, now tested on hw by Fenghua Yu
>    - fix some wording in the commit description   [Fenghua Yu]
>    
> v1: https://lore.kernel.org/all/a899125f42c12fa782a881d341d147519cbb4a23.1702967302.git.christophe.jaillet@wanadoo.fr/
> ---
>   drivers/dma/idxd/cdev.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> index 0423655f5a88..b00926abc69a 100644
> --- a/drivers/dma/idxd/cdev.c
> +++ b/drivers/dma/idxd/cdev.c
> @@ -165,7 +165,7 @@ static void idxd_cdev_dev_release(struct device *dev)
>   	struct idxd_wq *wq = idxd_cdev->wq;
>   
>   	cdev_ctx = &ictx[wq->idxd->data->type];
> -	ida_simple_remove(&cdev_ctx->minor_ida, idxd_cdev->minor);
> +	ida_free(&cdev_ctx->minor_ida, idxd_cdev->minor);
>   	kfree(idxd_cdev);
>   }
>   
> @@ -463,7 +463,7 @@ int idxd_wq_add_cdev(struct idxd_wq *wq)
>   	cdev = &idxd_cdev->cdev;
>   	dev = cdev_dev(idxd_cdev);
>   	cdev_ctx = &ictx[wq->idxd->data->type];
> -	minor = ida_simple_get(&cdev_ctx->minor_ida, 0, MINORMASK, GFP_KERNEL);
> +	minor = ida_alloc_max(&cdev_ctx->minor_ida, MINORMASK, GFP_KERNEL);

It shouldn't be an issue though it should be below statement if directly 
converting to ida_alloc_max
+	minor = ida_alloc_max(&cdev_ctx->minor_ida, MINORMASK - 1, GFP_KERNEL);

>   	if (minor < 0) {
>   		kfree(idxd_cdev);
>   		return minor;