* [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; 10+ 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] 10+ 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; 10+ 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 related [flat|nested] 10+ 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; 10+ 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 related [flat|nested] 10+ 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; 10+ 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 related [flat|nested] 10+ 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; 10+ 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 related [flat|nested] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2023-09-12 14:34 UTC | newest]
Thread overview: 10+ 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox