* [PATCH 0/4] genirq/irq_sim: misc updates
@ 2023-09-12 8:55 Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 1/4] bitmap: define a cleanup function for bitmaps Bartosz Golaszewski
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-09-12 8:55 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner,
Marc Zyngier
Cc: linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Here are a couple of updates to the interrupt simulator. Two are minor:
remove an unused field and reorder includes for readability. The third
one simplifies the error paths by using new cleanup macros. To that end
we also add a cleanup definition for dynamic bitmaps.
Bartosz Golaszewski (4):
bitmap: define a cleanup function for bitmaps
genirq/irq_sim: remove unused field from struct irq_sim_irq_ctx
genirq/irq_sim: order headers alphabetically
genirq/irq_sim: shrink code by using cleanup helpers
include/linux/bitmap.h | 3 +++
kernel/irq/irq_sim.c | 27 +++++++++++----------------
2 files changed, 14 insertions(+), 16 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] bitmap: define a cleanup function for bitmaps
2023-09-12 8:55 [PATCH 0/4] genirq/irq_sim: misc updates Bartosz Golaszewski
@ 2023-09-12 8:55 ` Bartosz Golaszewski
2023-09-12 9:36 ` Andy Shevchenko
2023-09-12 13:43 ` Yury Norov
2023-09-12 8:55 ` [PATCH 2/4] genirq/irq_sim: remove unused field from struct irq_sim_irq_ctx Bartosz Golaszewski
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-09-12 8:55 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner,
Marc Zyngier
Cc: linux-kernel, Bartosz Golaszewski
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..ba8c0d733842 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 *, 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] genirq/irq_sim: remove unused field from struct irq_sim_irq_ctx
2023-09-12 8:55 [PATCH 0/4] genirq/irq_sim: misc updates Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 1/4] bitmap: define a cleanup function for bitmaps Bartosz Golaszewski
@ 2023-09-12 8:55 ` Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 3/4] genirq/irq_sim: order headers alphabetically Bartosz Golaszewski
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-09-12 8:55 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner,
Marc Zyngier
Cc: linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The irqnum field is unused. Remove it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
kernel/irq/irq_sim.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index dd76323ea3fd..f5ebb3ba6f9a 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -19,7 +19,6 @@ struct irq_sim_work_ctx {
};
struct irq_sim_irq_ctx {
- int irqnum;
bool enabled;
struct irq_sim_work_ctx *work_ctx;
};
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] genirq/irq_sim: order headers alphabetically
2023-09-12 8:55 [PATCH 0/4] genirq/irq_sim: misc updates Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 1/4] bitmap: define a cleanup function for bitmaps Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 2/4] genirq/irq_sim: remove unused field from struct irq_sim_irq_ctx Bartosz Golaszewski
@ 2023-09-12 8:55 ` Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers Bartosz Golaszewski
2023-09-12 13:36 ` [PATCH 0/4] genirq/irq_sim: misc updates Yury Norov
4 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-09-12 8:55 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner,
Marc Zyngier
Cc: linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
For better readability and maintenance keep headers in alphabetical
order.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
kernel/irq/irq_sim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index f5ebb3ba6f9a..b0d50b48dbd1 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -4,10 +4,10 @@
* Copyright (C) 2020 Bartosz Golaszewski <bgolaszewski@baylibre.com>
*/
+#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irq_sim.h>
#include <linux/irq_work.h>
-#include <linux/interrupt.h>
#include <linux/slab.h>
struct irq_sim_work_ctx {
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers
2023-09-12 8:55 [PATCH 0/4] genirq/irq_sim: misc updates Bartosz Golaszewski
` (2 preceding siblings ...)
2023-09-12 8:55 ` [PATCH 3/4] genirq/irq_sim: order headers alphabetically Bartosz Golaszewski
@ 2023-09-12 8:55 ` Bartosz Golaszewski
2023-09-12 13:36 ` [PATCH 0/4] genirq/irq_sim: misc updates Yury Norov
4 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-09-12 8:55 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner,
Marc Zyngier
Cc: linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use the new __free helper from linux/cleanup.h to remove all gotos and
simplify the error paths.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
kernel/irq/irq_sim.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index b0d50b48dbd1..24d10267f582 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -4,6 +4,7 @@
* Copyright (C) 2020 Bartosz Golaszewski <bgolaszewski@baylibre.com>
*/
+#include <linux/cleanup.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irq_sim.h>
@@ -163,33 +164,28 @@ static const struct irq_domain_ops irq_sim_domain_ops = {
struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode,
unsigned int num_irqs)
{
- struct irq_sim_work_ctx *work_ctx;
+ struct irq_sim_work_ctx *work_ctx __free(kfree) = NULL;
+ unsigned long *pending __free(bitmap) = NULL;
work_ctx = kmalloc(sizeof(*work_ctx), GFP_KERNEL);
if (!work_ctx)
- goto err_out;
+ return ERR_PTR(-ENOMEM);
- work_ctx->pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
- if (!work_ctx->pending)
- goto err_free_work_ctx;
+ pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
+ if (!pending)
+ return ERR_PTR(-ENOMEM);
work_ctx->domain = irq_domain_create_linear(fwnode, num_irqs,
&irq_sim_domain_ops,
work_ctx);
if (!work_ctx->domain)
- goto err_free_bitmap;
+ return ERR_PTR(-ENOMEM);
work_ctx->irq_count = num_irqs;
work_ctx->work = IRQ_WORK_INIT_HARD(irq_sim_handle_irq);
+ work_ctx->pending = no_free_ptr(pending);
- return work_ctx->domain;
-
-err_free_bitmap:
- bitmap_free(work_ctx->pending);
-err_free_work_ctx:
- kfree(work_ctx);
-err_out:
- return ERR_PTR(-ENOMEM);
+ return no_free_ptr(work_ctx)->domain;
}
EXPORT_SYMBOL_GPL(irq_domain_create_sim);
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] bitmap: define a cleanup function for bitmaps
2023-09-12 8:55 ` [PATCH 1/4] bitmap: define a cleanup function for bitmaps Bartosz Golaszewski
@ 2023-09-12 9:36 ` Andy Shevchenko
2023-09-12 13:43 ` Yury Norov
1 sibling, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2023-09-12 9:36 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Yury Norov, Rasmus Villemoes, Thomas Gleixner, Marc Zyngier,
linux-kernel, Bartosz Golaszewski
On Tue, Sep 12, 2023 at 10:55:38AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Add support for autopointers for bitmaps allocated with bitmap_alloc()
> et al.
Makes sense,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] genirq/irq_sim: misc updates
2023-09-12 8:55 [PATCH 0/4] genirq/irq_sim: misc updates Bartosz Golaszewski
` (3 preceding siblings ...)
2023-09-12 8:55 ` [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers Bartosz Golaszewski
@ 2023-09-12 13:36 ` Yury Norov
2023-09-12 14:33 ` Bartosz Golaszewski
4 siblings, 1 reply; 13+ messages in thread
From: Yury Norov @ 2023-09-12 13:36 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner, Marc Zyngier,
linux-kernel, Bartosz Golaszewski
On Tue, Sep 12, 2023 at 10:55:37AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Here are a couple of updates to the interrupt simulator. Two are minor:
> remove an unused field and reorder includes for readability. The third
> one simplifies the error paths by using new cleanup macros. To that end
> we also add a cleanup definition for dynamic bitmaps.
This looks like a v2 for https://lore.kernel.org/lkml/20230812194457.6432-1-brgl@bgdev.pl/
Is it right? If so, can you keep history of submissions in the cover
email please?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] bitmap: define a cleanup function for bitmaps
2023-09-12 8:55 ` [PATCH 1/4] bitmap: define a cleanup function for bitmaps Bartosz Golaszewski
2023-09-12 9:36 ` Andy Shevchenko
@ 2023-09-12 13:43 ` Yury Norov
2023-09-12 14:28 ` Bartosz Golaszewski
1 sibling, 1 reply; 13+ messages in thread
From: Yury Norov @ 2023-09-12 13:43 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner, Marc Zyngier,
linux-kernel, Bartosz Golaszewski
On Tue, Sep 12, 2023 at 10:55:38AM +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..ba8c0d733842 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 *, bitmap_free(_T))
So now it doesn't do weird "if (_T) bitmap_free(_T)". Have you got any
feedback from Peter for that?
My point is that if the above is correct, all the following
declarations need to be revisited:
yury:linux$ git grep DEFINE_FREE|grep if
include/linux/cleanup.h:25: * DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
include/linux/device.h:1058:DEFINE_FREE(device_del, struct device *, if (_T) device_del(_T))
include/linux/device.h:1228:DEFINE_FREE(put_device, struct device *, if (_T) put_device(_T))
include/linux/mutex.h:224:DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T))
include/linux/rwsem.h:208:DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T))
include/linux/rwsem.h:209:DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T))
include/linux/sched/task.h:164:DEFINE_FREE(put_task, struct task_struct *, if (_T) put_task_struct(_T))
include/linux/slab.h:231:DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
For the patch:
Acked-by: Yury Norov <yury.norov@gmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] bitmap: define a cleanup function for bitmaps
2023-09-12 13:43 ` Yury Norov
@ 2023-09-12 14:28 ` Bartosz Golaszewski
0 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-09-12 14:28 UTC (permalink / raw)
To: Yury Norov
Cc: Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner, Marc Zyngier,
linux-kernel, Bartosz Golaszewski
On Tue, Sep 12, 2023 at 3:43 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Tue, Sep 12, 2023 at 10:55:38AM +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..ba8c0d733842 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 *, bitmap_free(_T))
>
> So now it doesn't do weird "if (_T) bitmap_free(_T)". Have you got any
> feedback from Peter for that?
>
I did get feedback from Peter. Unfortunately it was right *after* I
sent this. Turns out the NULL-pointer check is there for a reason,
please see the comment in this patch:
https://lore.kernel.org/all/169451499208.27769.5856056754166699857.tip-bot2@tip-bot2/T/#u
That means I will have to resend this one.
Bart
> My point is that if the above is correct, all the following
> declarations need to be revisited:
>
> yury:linux$ git grep DEFINE_FREE|grep if
> include/linux/cleanup.h:25: * DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
> include/linux/device.h:1058:DEFINE_FREE(device_del, struct device *, if (_T) device_del(_T))
> include/linux/device.h:1228:DEFINE_FREE(put_device, struct device *, if (_T) put_device(_T))
> include/linux/mutex.h:224:DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T))
> include/linux/rwsem.h:208:DEFINE_FREE(up_read, struct rw_semaphore *, if (_T) up_read(_T))
> include/linux/rwsem.h:209:DEFINE_FREE(up_write, struct rw_semaphore *, if (_T) up_write(_T))
> include/linux/sched/task.h:164:DEFINE_FREE(put_task, struct task_struct *, if (_T) put_task_struct(_T))
> include/linux/slab.h:231:DEFINE_FREE(kfree, void *, if (_T) kfree(_T))
>
> For the patch:
> Acked-by: Yury Norov <yury.norov@gmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] genirq/irq_sim: misc updates
2023-09-12 13:36 ` [PATCH 0/4] genirq/irq_sim: misc updates Yury Norov
@ 2023-09-12 14:33 ` Bartosz Golaszewski
0 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-09-12 14:33 UTC (permalink / raw)
To: Yury Norov
Cc: Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner, Marc Zyngier,
linux-kernel, Bartosz Golaszewski
On Tue, Sep 12, 2023 at 3:36 PM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Tue, Sep 12, 2023 at 10:55:37AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Here are a couple of updates to the interrupt simulator. Two are minor:
> > remove an unused field and reorder includes for readability. The third
> > one simplifies the error paths by using new cleanup macros. To that end
> > we also add a cleanup definition for dynamic bitmaps.
>
> This looks like a v2 for https://lore.kernel.org/lkml/20230812194457.6432-1-brgl@bgdev.pl/
> Is it right? If so, can you keep history of submissions in the cover
> email please?
That series was mainly about fixing a resource leak. While it
contained a part of the changes that ended up here, I consider this
one a stand-alone enough to not make it a v2. Probably should have
mentioned that one though.
Bart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers
2023-08-14 1:09 ` Yury Norov
@ 2023-08-14 6:58 ` Bartosz Golaszewski
0 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-08-14 6:58 UTC (permalink / raw)
To: Yury Norov
Cc: Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner, linux-kernel,
Bartosz Golaszewski
On Mon, Aug 14, 2023 at 3:09 AM Yury Norov <yury.norov@gmail.com> wrote:
>
> On Sat, Aug 12, 2023 at 09:44:57PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Use the new __free helper from linux/cleanup.h to remove all gotos and
> > simplify the error paths.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> > kernel/irq/irq_sim.c | 24 ++++++++++--------------
> > 1 file changed, 10 insertions(+), 14 deletions(-)
> >
> > diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
> > index a8b013d0c5be..202beb1169c9 100644
> > --- a/kernel/irq/irq_sim.c
> > +++ b/kernel/irq/irq_sim.c
> > @@ -4,6 +4,7 @@
> > * Copyright (C) 2020 Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > */
> >
> > +#include <linux/cleanup.h>
> > #include <linux/interrupt.h>
> > #include <linux/irq.h>
> > #include <linux/irq_sim.h>
> > @@ -170,34 +171,29 @@ static const struct irq_domain_ops irq_sim_domain_ops = {
> > struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode,
> > unsigned int num_irqs)
> > {
> > - struct irq_sim_work_ctx *work_ctx;
> > + struct irq_sim_work_ctx *work_ctx __free(kfree) = NULL;
> > + unsigned long *pending __free(bitmap) = NULL;
>
> Why initializing here as NULL ...
>
> > work_ctx = kmalloc(sizeof(*work_ctx), GFP_KERNEL);
> > if (!work_ctx)
> > - goto err_out;
> > + return ERR_PTR(-ENOMEM);
> >
> > - work_ctx->pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
> > - if (!work_ctx->pending)
> > - goto err_free_work_ctx;
> > + pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
> > + if (!pending)
> > + return ERR_PTR(-ENOMEM);
>
> and overriding immediately after that? Not familiar to __free()
> machinery in details, although. Does it require initialization?
>
> Thank,
> Yury
For the first variable: it's just good practice resulting from years
of user-space coding in GLib which makes heavy use of autopointers.
For the 'pending' variable, it's necessary as if the first kmalloc()
fails, it will go out of scope with random contents and bitmap_free()
will get called on it. Now if we ever add some statement that can
return between the start of the function and the first kmalloc() then
we'll save ourselves having to modify the variable declaration.
Bart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers
2023-08-12 19:44 ` [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers Bartosz Golaszewski
@ 2023-08-14 1:09 ` Yury Norov
2023-08-14 6:58 ` Bartosz Golaszewski
0 siblings, 1 reply; 13+ messages in thread
From: Yury Norov @ 2023-08-14 1:09 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner, linux-kernel,
Bartosz Golaszewski
On Sat, Aug 12, 2023 at 09:44:57PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Use the new __free helper from linux/cleanup.h to remove all gotos and
> simplify the error paths.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> kernel/irq/irq_sim.c | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
> index a8b013d0c5be..202beb1169c9 100644
> --- a/kernel/irq/irq_sim.c
> +++ b/kernel/irq/irq_sim.c
> @@ -4,6 +4,7 @@
> * Copyright (C) 2020 Bartosz Golaszewski <bgolaszewski@baylibre.com>
> */
>
> +#include <linux/cleanup.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/irq_sim.h>
> @@ -170,34 +171,29 @@ static const struct irq_domain_ops irq_sim_domain_ops = {
> struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode,
> unsigned int num_irqs)
> {
> - struct irq_sim_work_ctx *work_ctx;
> + struct irq_sim_work_ctx *work_ctx __free(kfree) = NULL;
> + unsigned long *pending __free(bitmap) = NULL;
Why initializing here as NULL ...
> work_ctx = kmalloc(sizeof(*work_ctx), GFP_KERNEL);
> if (!work_ctx)
> - goto err_out;
> + return ERR_PTR(-ENOMEM);
>
> - work_ctx->pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
> - if (!work_ctx->pending)
> - goto err_free_work_ctx;
> + pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
> + if (!pending)
> + return ERR_PTR(-ENOMEM);
and overriding immediately after that? Not familiar to __free()
machinery in details, although. Does it require initialization?
Thank,
Yury
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers
2023-08-12 19:44 [PATCH 0/4] genirq/irq_sim: fix a use-after-free bug + some Bartosz Golaszewski
@ 2023-08-12 19:44 ` Bartosz Golaszewski
2023-08-14 1:09 ` Yury Norov
0 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-08-12 19:44 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko, Rasmus Villemoes, Thomas Gleixner
Cc: linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use the new __free helper from linux/cleanup.h to remove all gotos and
simplify the error paths.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
kernel/irq/irq_sim.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index a8b013d0c5be..202beb1169c9 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -4,6 +4,7 @@
* Copyright (C) 2020 Bartosz Golaszewski <bgolaszewski@baylibre.com>
*/
+#include <linux/cleanup.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irq_sim.h>
@@ -170,34 +171,29 @@ static const struct irq_domain_ops irq_sim_domain_ops = {
struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode,
unsigned int num_irqs)
{
- struct irq_sim_work_ctx *work_ctx;
+ struct irq_sim_work_ctx *work_ctx __free(kfree) = NULL;
+ unsigned long *pending __free(bitmap) = NULL;
work_ctx = kmalloc(sizeof(*work_ctx), GFP_KERNEL);
if (!work_ctx)
- goto err_out;
+ return ERR_PTR(-ENOMEM);
- work_ctx->pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
- if (!work_ctx->pending)
- goto err_free_work_ctx;
+ pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
+ if (!pending)
+ return ERR_PTR(-ENOMEM);
work_ctx->domain = irq_domain_create_linear(fwnode, num_irqs,
&irq_sim_domain_ops,
work_ctx);
if (!work_ctx->domain)
- goto err_free_bitmap;
+ return ERR_PTR(-ENOMEM);
work_ctx->irq_count = num_irqs;
work_ctx->work = IRQ_WORK_INIT_HARD(irq_sim_handle_irq);
INIT_LIST_HEAD(&work_ctx->irqs);
+ work_ctx->pending = no_free_ptr(pending);
- return work_ctx->domain;
-
-err_free_bitmap:
- bitmap_free(work_ctx->pending);
-err_free_work_ctx:
- kfree(work_ctx);
-err_out:
- return ERR_PTR(-ENOMEM);
+ return no_free_ptr(work_ctx)->domain;
}
EXPORT_SYMBOL_GPL(irq_domain_create_sim);
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-09-12 14:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-12 8:55 [PATCH 0/4] genirq/irq_sim: misc updates Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 1/4] bitmap: define a cleanup function for bitmaps Bartosz Golaszewski
2023-09-12 9:36 ` Andy Shevchenko
2023-09-12 13:43 ` Yury Norov
2023-09-12 14:28 ` Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 2/4] genirq/irq_sim: remove unused field from struct irq_sim_irq_ctx Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 3/4] genirq/irq_sim: order headers alphabetically Bartosz Golaszewski
2023-09-12 8:55 ` [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers Bartosz Golaszewski
2023-09-12 13:36 ` [PATCH 0/4] genirq/irq_sim: misc updates Yury Norov
2023-09-12 14:33 ` Bartosz Golaszewski
-- strict thread matches above, loose matches on Subject: below --
2023-08-12 19:44 [PATCH 0/4] genirq/irq_sim: fix a use-after-free bug + some Bartosz Golaszewski
2023-08-12 19:44 ` [PATCH 4/4] genirq/irq_sim: shrink code by using cleanup helpers Bartosz Golaszewski
2023-08-14 1:09 ` Yury Norov
2023-08-14 6:58 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome