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: <4d7e96ac0125bf8296689385f3f9dd54f7e2c0f1.camel@perches.com>
Subject: Re: [PATCH 3/6] fs/ntfs3: Refactoring attr_punch_hole to restore
 after errors
From:   Joe Perches <joe@perches.com>
To:     Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
        ntfs3@lists.linux.dev
Cc:     linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Date:   Wed, 13 Jul 2022 11:58:48 -0700
In-Reply-To: <33c5b044-23a4-60a6-1649-9e5db228c2f7@paragon-software.com>
References: <2101d95b-be41-6e6d-e019-bc70f816b2e8@paragon-software.com>
         <33c5b044-23a4-60a6-1649-9e5db228c2f7@paragon-software.com>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 8BIT
User-Agent: Evolution 3.44.1-0ubuntu1 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2022-07-13 at 19:46 +0300, Konstantin Komarov wrote:
> Added comments to code
> Added new function run_clone to make a copy of run
> Added done and undo labels for restoring after errors

trivia:

> diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
[]
> index 7bcae3094712..24d545041787 100644llocate_ex(struct ntfs_sb_info *sbi, struct runs_tree *run,
>   		}
>   
>   		if (lcn != SPARSE_LCN) {
> -			mark_as_free_ex(sbi, lcn, clen, trim);
> +			if (sbi) {
> +				/* mark bitmap range [lcn + clen) as free and trim clusters. */

presumably the brackets or parentheses [ ) should match

[]

> @@ -2091,69 +2098,91 @@ int attr_punch_hole(struct ntfs_inode *ni, u64 vbo, u64 bytes, u32 *frame_size)
[]
> +
> +		/* Make a hole range (sparse) [vcn1 + zero). */

here too

> diff --git a/fs/ntfs3/run.c b/fs/ntfs3/run.c
[]
> @@ -1157,3 +1157,28 @@ int run_get_highest_vcn(CLST vcn, const u8 *run_buf, u64 *highest_vcn)
>   	*highest_vcn = vcn64 - 1;
>   	return 0;
>   }
> +
> +/*
> + * run_clone
> + *
> + * Make a copy of run
> + */
> +int run_clone(const struct runs_tree *run, struct runs_tree *new_run)
> +{
> +	size_t bytes = run->count * sizeof(struct ntfs_run);
> +
> +	if (bytes > new_run->allocated) {
> +		struct ntfs_run *new_ptr = kvmalloc(bytes, GFP_KERNEL);

kvmalloc_array ?

> +
> +		if (!new_ptr)
> +			return -ENOMEM;
> +
> +		kvfree(new_run->runs);
> +		new_run->runs = new_ptr;
> +		new_run->allocated = bytes;
> +	}
> +
> +	memcpy(new_run->runs, run->runs, bytes);
> +	new_run->count = run->count;
> +	return 0;
> +}