Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 17 Aug 2021 20:20:53 +0300
From:   Kari Argillander <kari.argillander@gmail.com>
To:     Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
        ntfs3@lists.linux.dev, Christoph Hellwig <hch@lst.de>
Cc:     linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fs/ntfs3: Do not use driver own alloc wrappers
Message-ID: <20210817172053.6gyy3nhpc75pygod@kari-VirtualBox>
References: <20210817143848.245324-1-kari.argillander@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210817143848.245324-1-kari.argillander@gmail.com>
Precedence: bulk

On Tue, Aug 17, 2021 at 05:38:48PM +0300, Kari Argillander wrote:
> Ntfs3 driver contains self made alloc wrappers. Problem with these
> wrappers is that we cannot take off example GFP_NOFS flag. It is not
> recomended use those in all places. Also if we change one driver
> specific wrapper to kernel wrapper then it would look really wierd.
> People should be most familiar with kernel wrappers so let's just use
> those ones.
> 
> Driver specific alloc wrapper might also confuse some static analyzing
> tools, but I do not have real world example.

Actually I do. checkpatch.pl will now give following warnings so
checkpatch was triggered with self made wrappers.

WARNING: Prefer kcalloc over kzalloc with multiply
#170: FILE: fs/ntfs3/bitmap.c:686:
+	wnd->free_bits = kzalloc(wnd->nwnd * sizeof(u16), GFP_NOFS);

WARNING: Prefer kmalloc_array over kmalloc with multiply
#179: FILE: fs/ntfs3/bitmap.c:1357:
+		new_free = kmalloc(new_wnd * sizeof(u16), GFP_NOFS);

WARNING: Prefer kcalloc over kzalloc with multiply
#284: FILE: fs/ntfs3/frecord.c:2053:
+	pages = kzalloc(pages_per_frame * sizeof(struct page *), GFP_NOFS);

WARNING: Prefer kcalloc over kzalloc with multiply
#302: FILE: fs/ntfs3/frecord.c:2136:
+	pages = kzalloc(pages_per_frame * sizeof(struct page *), GFP_NOFS);

WARNING: Prefer kmalloc_array over kmalloc with multiply
#1016: FILE: fs/ntfs3/index.c:684:
+	offs = kmalloc(sizeof(u16) * nslots, GFP_NOFS);

WARNING: Prefer kmalloc_array over kmalloc with multiply
#1025: FILE: fs/ntfs3/index.c:706:
+		ptr = kmalloc(sizeof(u16) * new_slots, GFP_NOFS);

> Following Coccinelle script was used to automate most of this patch:
> 
> virtual patch
> 
> @alloc depends on patch@
> expression x;
> expression y;
> @@
> (
> -	ntfs_malloc(x)
> +	kmalloc(x, GFP_NOFS)
> |
> -	ntfs_zalloc(x)
> +	kzalloc(x, GFP_NOFS)
> |
> -	ntfs_vmalloc(x)
> +	kvmalloc(x, GFP_NOFS)
> |
> -	ntfs_free(x)
> +	kfree(x)
> |
> -	ntfs_vfree(x)
> +	kvfree(x)
> |
> -	ntfs_memdup(x, y)
> +	kmemdup(x, y, GFP_NOFS)
> )
> 
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> ---
> 
> Christoph also nack about these so I CC him.