[PATCH 3/4] bitmap: define a cleanup function for bitmaps

Bartosz Golaszewski posted 4 patches 2 years, 4 months ago
[PATCH 3/4] bitmap: define a cleanup function for bitmaps
Posted by Bartosz Golaszewski 2 years, 4 months ago
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Add support for autopointers for bitmaps allocated with bitmap_alloc()
et al.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 include/linux/bitmap.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 03644237e1ef..6709807ebb59 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -6,6 +6,7 @@
 
 #include <linux/align.h>
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/find.h>
 #include <linux/limits.h>
 #include <linux/string.h>
@@ -125,6 +126,8 @@ unsigned long *bitmap_alloc_node(unsigned int nbits, gfp_t flags, int node);
 unsigned long *bitmap_zalloc_node(unsigned int nbits, gfp_t flags, int node);
 void bitmap_free(const unsigned long *bitmap);
 
+DEFINE_FREE(bitmap, unsigned long *, if (_T) bitmap_free(_T))
+
 /* Managed variants of the above. */
 unsigned long *devm_bitmap_alloc(struct device *dev,
 				 unsigned int nbits, gfp_t flags);
-- 
2.39.2
Re: [PATCH 3/4] bitmap: define a cleanup function for bitmaps
Posted by Yury Norov 2 years, 4 months ago
On Sat, Aug 12, 2023 at 09:44:56PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Add support for autopointers for bitmaps allocated with bitmap_alloc()
> et al.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  include/linux/bitmap.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 03644237e1ef..6709807ebb59 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -6,6 +6,7 @@
>  
>  #include <linux/align.h>
>  #include <linux/bitops.h>
> +#include <linux/cleanup.h>
>  #include <linux/find.h>
>  #include <linux/limits.h>
>  #include <linux/string.h>
> @@ -125,6 +126,8 @@ unsigned long *bitmap_alloc_node(unsigned int nbits, gfp_t flags, int node);
>  unsigned long *bitmap_zalloc_node(unsigned int nbits, gfp_t flags, int node);
>  void bitmap_free(const unsigned long *bitmap);
>  
> +DEFINE_FREE(bitmap, unsigned long *, if (_T) bitmap_free(_T))

bitmap_free() is a wrapper around kfree(), and kfree() checks against
NULL, and returns immediately. Why this tests again?
Re: [PATCH 3/4] bitmap: define a cleanup function for bitmaps
Posted by Bartosz Golaszewski 2 years, 4 months ago
On Mon, Aug 14, 2023 at 3:02 AM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Sat, Aug 12, 2023 at 09:44:56PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Add support for autopointers for bitmaps allocated with bitmap_alloc()
> > et al.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  include/linux/bitmap.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> > index 03644237e1ef..6709807ebb59 100644
> > --- a/include/linux/bitmap.h
> > +++ b/include/linux/bitmap.h
> > @@ -6,6 +6,7 @@
> >
> >  #include <linux/align.h>
> >  #include <linux/bitops.h>
> > +#include <linux/cleanup.h>
> >  #include <linux/find.h>
> >  #include <linux/limits.h>
> >  #include <linux/string.h>
> > @@ -125,6 +126,8 @@ unsigned long *bitmap_alloc_node(unsigned int nbits, gfp_t flags, int node);
> >  unsigned long *bitmap_zalloc_node(unsigned int nbits, gfp_t flags, int node);
> >  void bitmap_free(const unsigned long *bitmap);
> >
> > +DEFINE_FREE(bitmap, unsigned long *, if (_T) bitmap_free(_T))
>
> bitmap_free() is a wrapper around kfree(), and kfree() checks against
> NULL, and returns immediately. Why this tests again?

I don't see any kernel docs making it a contract though. :)

It's a good question however. I carried it over from the kfree()'s
DEFINE_FREE() itself. I can't find any discussion about it in the
thread under Peter Zijlstra's patches.

May be worth revisiting it for free() functions that handle NULL.

Bart