Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <8716bf72-e4ca-4f83-8a30-327baf459dbb@intel.com>
Date:   Mon, 11 Dec 2023 11:26:56 -0700
User-Agent: Betterbird (Linux)
Subject: Re: [PATCH] nvdimm-btt: simplify code with the scope based resource
 management
To:     Dinghao Liu <dinghao.liu@zju.edu.cn>
CC:     Vishal Verma <vishal.l.verma@intel.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Ira Weiny <ira.weiny@intel.com>, <nvdimm@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>
References: <20231210102747.13545-1-dinghao.liu@zju.edu.cn>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20231210102747.13545-1-dinghao.liu@zju.edu.cn>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZGxLWHB5ZldwSlMrcGI5Vm15TU9EV2pESDZocDlHSklqT2YvWndkRFZxN0pk?=
 =?utf-8?B?UU50YzQ2c1hLbTgyUEl1enNmR3d4MXkzTkRlYm9kTWYvNTRmUVAwaEQ2YW5R?=
 =?utf-8?B?N3R4emtXSGlFVk94U0VaQ3VoeGJ0T092OEFaeXdnYlRmZFg5czlIS3VUYWxk?=
 =?utf-8?B?dldrNFZrV0ZhOEdlV1A3ejRSL2dURXpXVUM5L0VzSDlTNTFPRHJiQkNNZk9s?=
 =?utf-8?B?MUJTYjNOU0xhL2t0VnhYc0Fmb0dOV2V6M0FzWVlrMmcwRE9ESXd6bDV6RHFR?=
 =?utf-8?B?aHdpOWdSNDlIbGt0MDJOb2NUSUR1UXlVY2RzZmp0MTZDTW1EVkd5cFNuaWZh?=
 =?utf-8?B?ZG9YZkY3KzdOd3ZEOTR3Zmw4UG5LaS9zTlFFU3R1L0I4cDkrakpnL3hhZ1RK?=
 =?utf-8?B?bnpCdm5HSVkzY0l6MXpoa05qOUlWY29zVmRnWStjWDZqYWk2eEIrU2E4aDFO?=
 =?utf-8?B?Ry9RVGQwdUcrejU4Sk5HMStLMkdWSHBLMEZ4TGd3SSt6emRMUlc2ZkMzOGkz?=
 =?utf-8?B?VzMrM1NNdm9VdFE3VkMwV0FmbTh5NW1VQ25rTzJrd0Q2L2VGeUY3dDRSaU1B?=
 =?utf-8?B?L0Q0S093M0g4RTFSVzI0WnpkdlR4TGd6WW1OZVhlSEZ1cWVxUGRJSnM3RC9H?=
 =?utf-8?B?djNUbE5CdHR3QllDcTNGYlBSWWZITmZSUEFqenMxK3k2N0N1akdmSE9LWGhp?=
 =?utf-8?B?TzhGbTFhQkNNblNvdVQveSs4WVZEbWFoaE0vcHp0V1NUOHBNUmpsS0t5VWxN?=
 =?utf-8?B?T1BRQkFJQ0xOdUZVeHcwQzdrL2JwWW5VQnVvUUtuNGV5dE9yeXhUQ2FvS3Vi?=
 =?utf-8?B?bjY4blRMQ1BMTGdpNHlnZWNqTmFiRE5FUjcvQ0drVG9ya2d3TlllbDlUQk84?=
 =?utf-8?B?K0F3VjFHL001THdBYkZLb0d3SS94dEZWRHdhb3JsTDBsNnFxSytjcEFUQWJ5?=
 =?utf-8?B?ODZVSlNqQ3d1ekd5OGlKblc3aUx4L01rbFpzeTIwZGJsSXRSNVc1eDBJeDA2?=
 =?utf-8?B?SXFBQ2tVSG52dklpTU8yak8xUGVzd29tOXJRM3R4ZTkrRG5ibjJvSFpFOWQ4?=
 =?utf-8?B?eVVYZU9ocTRMSkY2eE5aZ0RXVGtpZWgxVzhTVWRuYXZnbDdXa1ZnWGY1Sitr?=
 =?utf-8?B?SThEdWdMbnV0a0xXSnloTXN3bm5TZi9zU0VMSDBGei9KZzlNd0dBQVZ5Q21L?=
 =?utf-8?B?a21Mc3AyTGNtQ3JMaDBTb0xKeEpoVVVPanJrN0xLcVRqSkRaMldrUHJtWlJV?=
 =?utf-8?B?MUxRc05RMk54dDF1SlV1a1F3U3dNRWhmOVZoSlF1R2VDYWZoM2NtNkFiMVZ1?=
 =?utf-8?B?NjNaUjUrdWtaelNWYzBraW9rdFR4SzArOWwrSXV0NVB3TlFyVHJ4aytDRFlh?=
 =?utf-8?B?VjAvRVh1c1k4Yi9HUzgzTDRLbGFOMy9OTUJKck00UVplMWJ3OEpFeHN1OWp0?=
 =?utf-8?B?OGphN0p4R1d4TkNua0pMZ2FKWDBNYXJnejZoZDRxb0UrWEIwWmVnUjQ3Tmlx?=
 =?utf-8?B?ZDkxWnhiMnhHbjF0TEtHbjh0cGlxMElJcFRjdG5FOXlFdHNYVjlPcHN1cTdN?=
 =?utf-8?B?ZlJNZ1c4QnlmL1VDUjg4MTJBYWVaQTBpdlpRamVnRU4xbWhHeVNaaDVzSkJJ?=
 =?utf-8?B?Z1Noa1BqZVdwQm9pUG1QM0Y4TmRCZTlQS2VDUkVZMkFNL1lybCtuL2U0a2RB?=
 =?utf-8?B?Vno4RE1CVjFYdmxjalpoZEp0TlIzeVYzVHpGWjE0VkpnYlpwU3FYWDRxajRt?=
 =?utf-8?B?eENyQjl3WmJ3T1k4Nk4wV09ESWNxd1B4NWhqTFpBTXNNVHZGQXhYQTlDcWRa?=
 =?utf-8?B?N3h3REVNc1dBdC9LL09wSUJRa0ZnN09VcDcwaTdTcFBrVk9EV1FTbE84eTdy?=
 =?utf-8?B?QzNFSTZJQ2g3ZmwwajR5eEhNTXZNSC8vbXRmNDNYTGRGYmJrd2pVbURQdDZT?=
 =?utf-8?B?NVl6SEFzTG9MNDRkSnpZYktaTThwNmRvT0tGN0lYU09Wc1V6VkhFRmlNWkIw?=
 =?utf-8?B?MFVRc1dRckRTTU5wUlBtMitWblIwVVlhZzNxVVVZYXk3bk1zRGU0WmVQdjk5?=
 =?utf-8?B?OXdkbGVpSk5lNzVhVmRodHdkaENFU0kzR0ZsRDVXMG5yemNwYmlDS2pTNmM0?=
 =?utf-8?Q?P9bauyIbpIkch4YhdBc+Lbuen?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5c6754ab-5ea1-4003-4f97-08dbfa76c4b9
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Dec 2023 18:27:01.8540
 (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: HQ2+kl+2mK5KWRs0UZqLIHTb8tXmuwstCZXK7U1sXw4YmG8EdbxjiaN613tySNFW+pYjTl7mn8bC7JUDURDubg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5247
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Mon, 11 Dec 2023 10:27:35 -0800 (PST)


On 12/10/23 03:27, Dinghao Liu wrote:
> Use the scope based resource management (defined in
> linux/cleanup.h) to automate resource lifetime
> control on struct btt_sb *super in discover_arenas().
> 
> Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> ---
>  drivers/nvdimm/btt.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index d5593b0dc700..ff42778b51de 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -16,6 +16,7 @@
>  #include <linux/fs.h>
>  #include <linux/nd.h>
>  #include <linux/backing-dev.h>
> +#include <linux/cleanup.h>
>  #include "btt.h"
>  #include "nd.h"
>  
> @@ -847,7 +848,7 @@ static int discover_arenas(struct btt *btt)
>  {
>  	int ret = 0;
>  	struct arena_info *arena;
> -	struct btt_sb *super;
> +	struct btt_sb *super __free(kfree) = NULL;
>  	size_t remaining = btt->rawsize;
>  	u64 cur_nlba = 0;
>  	size_t cur_off = 0;
> @@ -860,10 +861,8 @@ static int discover_arenas(struct btt *btt)
>  	while (remaining) {
>  		/* Alloc memory for arena */
>  		arena = alloc_arena(btt, 0, 0, 0);
> -		if (!arena) {
> -			ret = -ENOMEM;
> -			goto out_super;
> -		}
> +		if (!arena)
> +			return -ENOMEM;
>  
>  		arena->infooff = cur_off;
>  		ret = btt_info_read(arena, super);
> @@ -919,14 +918,11 @@ static int discover_arenas(struct btt *btt)
>  	btt->nlba = cur_nlba;
>  	btt->init_state = INIT_READY;
>  
> -	kfree(super);
>  	return ret;
>  
>   out:
>  	kfree(arena);
>  	free_arenas(btt);
> - out_super:
> -	kfree(super);
>  	return ret;
>  }
>  

I would do the allocation like something below for the first chunk. Otherwise the rest LGTM. 

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index d5593b0dc700..143921e7f26c 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -16,6 +16,7 @@
 #include <linux/fs.h>
 #include <linux/nd.h>
 #include <linux/backing-dev.h>
+#include <linux/cleanup.h>
 #include "btt.h"
 #include "nd.h"
 
@@ -845,25 +846,23 @@ static void parse_arena_meta(struct arena_info *arena, struct btt_sb *super,
 
 static int discover_arenas(struct btt *btt)
 {
+       struct btt_sb *super __free(kfree) =
+               kzalloc(sizeof(*super), GFP_KERNEL);
        int ret = 0;
        struct arena_info *arena;
-       struct btt_sb *super;
        size_t remaining = btt->rawsize;
        u64 cur_nlba = 0;
        size_t cur_off = 0;
        int num_arenas = 0;
 
-       super = kzalloc(sizeof(*super), GFP_KERNEL);
        if (!super)
                return -ENOMEM;
 
        while (remaining) {
                /* Alloc memory for arena */
                arena = alloc_arena(btt, 0, 0, 0);
-               if (!arena) {
-                       ret = -ENOMEM;
-                       goto out_super;
-               }
+               if (!arena)
+                       return -ENOMEM;
 
                arena->infooff = cur_off;
                ret = btt_info_read(arena, super);