* [PATCH] softirq: Be more verbose on t->state BUG()
@ 2021-03-15 15:44 Eugeniu Rosca
2021-03-16 14:31 ` Thomas Gleixner
0 siblings, 1 reply; 5+ messages in thread
From: Eugeniu Rosca @ 2021-03-15 15:44 UTC (permalink / raw)
To: linux-kernel, Thomas Gleixner, Peter Zijlstra, Ingo Molnar,
Will Deacon, Frederic Weisbecker, Jiafei Pan, Romain Perier
Cc: Andrew Gabbasov, Dirk Behme, Eugeniu Rosca, Eugeniu Rosca
From: Dirk Behme <dirk.behme@de.bosch.com>
In case this BUG() is hit, it helps debugging a lot to get an idea
what tasklet is the root cause. So, be slightly more verbose here.
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
kernel/softirq.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 9908ec4a9bfe..a6b602ad48d6 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -550,9 +550,13 @@ static void tasklet_action_common(struct softirq_action *a,
if (tasklet_trylock(t)) {
if (!atomic_read(&t->count)) {
- if (!test_and_clear_bit(TASKLET_STATE_SCHED,
- &t->state))
+ if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state)) {
+ if (t->use_callback)
+ pr_emerg("tasklet failed, cb: %pS\n", t->callback);
+ else
+ pr_emerg("tasklet failed, func: %pS\n", t->func);
BUG();
+ }
if (t->use_callback)
t->callback(t);
else
--
2.29.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Be more verbose on t->state BUG()
2021-03-15 15:44 [PATCH] softirq: Be more verbose on t->state BUG() Eugeniu Rosca
@ 2021-03-16 14:31 ` Thomas Gleixner
2021-03-16 15:10 ` Eugeniu Rosca
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2021-03-16 14:31 UTC (permalink / raw)
To: Eugeniu Rosca, linux-kernel, Peter Zijlstra, Ingo Molnar,
Will Deacon, Frederic Weisbecker, Jiafei Pan, Romain Perier
Cc: Andrew Gabbasov, Dirk Behme, Eugeniu Rosca, Eugeniu Rosca
On Mon, Mar 15 2021 at 16:44, Eugeniu Rosca wrote:
> From: Dirk Behme <dirk.behme@de.bosch.com>
>
> In case this BUG() is hit, it helps debugging a lot to get an idea
> what tasklet is the root cause. So, be slightly more verbose here.
>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
> kernel/softirq.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 9908ec4a9bfe..a6b602ad48d6 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -550,9 +550,13 @@ static void tasklet_action_common(struct softirq_action *a,
>
> if (tasklet_trylock(t)) {
> if (!atomic_read(&t->count)) {
> - if (!test_and_clear_bit(TASKLET_STATE_SCHED,
> - &t->state))
> + if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state)) {
> + if (t->use_callback)
> + pr_emerg("tasklet failed, cb: %pS\n", t->callback);
> + else
> + pr_emerg("tasklet failed, func: %pS\n", t->func);
> BUG();
> + }
> if (t->use_callback)
> t->callback(t);
> else
This belongs into unreadable land and actually the BUG() should just be
replaced by a WARN_ONCE(). Something like the below. Hmm?
Thanks,
tglx
---
+static bool tasklet_should_run(struct tasklet_struct *t)
+{
+ if (test_and_clear_bit(TASKLET_STATE_SCHED, &t->state))
+ return true;
+
+ WARN_ONCE(1, "tasklet SCHED state not set: %s %pS\n",
+ t->use_callback ? "callback" : "func",
+ t->use_callback ? (void*)t->callback : (void*)t->func);
+
+ return false;
+}
+
static void tasklet_action_common(struct softirq_action *a,
struct tasklet_head *tl_head,
unsigned int softirq_nr)
@@ -550,13 +562,12 @@ static void tasklet_action_common(struct
if (tasklet_trylock(t)) {
if (!atomic_read(&t->count)) {
- if (!test_and_clear_bit(TASKLET_STATE_SCHED,
- &t->state))
- BUG();
- if (t->use_callback)
- t->callback(t);
- else
- t->func(t->data);
+ if (tasklet_should_run(t)) {
+ if (t->use_callback)
+ t->callback(t);
+ else
+ t->func(t->data);
+ }
tasklet_unlock(t);
continue;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Be more verbose on t->state BUG()
2021-03-16 14:31 ` Thomas Gleixner
@ 2021-03-16 15:10 ` Eugeniu Rosca
2021-03-16 17:13 ` Thomas Gleixner
0 siblings, 1 reply; 5+ messages in thread
From: Eugeniu Rosca @ 2021-03-16 15:10 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Eugeniu Rosca, linux-kernel, Peter Zijlstra, Ingo Molnar,
Will Deacon, Frederic Weisbecker, Jiafei Pan, Romain Perier,
Andrew Gabbasov, Dirk Behme, Eugeniu Rosca
Hello Thomas,
On Tue, Mar 16, 2021 at 03:31:50PM +0100, Thomas Gleixner wrote:
> On Mon, Mar 15 2021 at 16:44, Eugeniu Rosca wrote:
> > From: Dirk Behme <dirk.behme@de.bosch.com>
> >
> > In case this BUG() is hit, it helps debugging a lot to get an idea
> > what tasklet is the root cause. So, be slightly more verbose here.
> >
> > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > ---
> > kernel/softirq.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > index 9908ec4a9bfe..a6b602ad48d6 100644
> > --- a/kernel/softirq.c
> > +++ b/kernel/softirq.c
> > @@ -550,9 +550,13 @@ static void tasklet_action_common(struct softirq_action *a,
> >
> > if (tasklet_trylock(t)) {
> > if (!atomic_read(&t->count)) {
> > - if (!test_and_clear_bit(TASKLET_STATE_SCHED,
> > - &t->state))
> > + if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state)) {
> > + if (t->use_callback)
> > + pr_emerg("tasklet failed, cb: %pS\n", t->callback);
> > + else
> > + pr_emerg("tasklet failed, func: %pS\n", t->func);
> > BUG();
> > + }
> > if (t->use_callback)
> > t->callback(t);
> > else
>
> This belongs into unreadable land and actually the BUG() should just be
> replaced by a WARN_ONCE(). Something like the below. Hmm?
Many thanks for the quick and constructive reply.
If no other comments in the next days, I will resubmit your proposal as
v2, marked with 'Suggested-by: Thomas Gleixner <tglx@linutronix.de>'.
Alternatively, feel free to author the patch and submit it with us in Cc.
Thanks again.
--
Best regards,
Eugeniu Rosca
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Be more verbose on t->state BUG()
2021-03-16 15:10 ` Eugeniu Rosca
@ 2021-03-16 17:13 ` Thomas Gleixner
2021-03-17 10:26 ` Eugeniu Rosca
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2021-03-16 17:13 UTC (permalink / raw)
To: Eugeniu Rosca
Cc: Eugeniu Rosca, linux-kernel, Peter Zijlstra, Ingo Molnar,
Will Deacon, Frederic Weisbecker, Jiafei Pan, Romain Perier,
Andrew Gabbasov, Dirk Behme, Eugeniu Rosca
On Tue, Mar 16 2021 at 16:10, Eugeniu Rosca wrote:
> If no other comments in the next days, I will resubmit your proposal as
> v2, marked with 'Suggested-by: Thomas Gleixner <tglx@linutronix.de>'.
>
> Alternatively, feel free to author the patch and submit it with us in Cc.
Just send a v2 please
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Be more verbose on t->state BUG()
2021-03-16 17:13 ` Thomas Gleixner
@ 2021-03-17 10:26 ` Eugeniu Rosca
0 siblings, 0 replies; 5+ messages in thread
From: Eugeniu Rosca @ 2021-03-17 10:26 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Eugeniu Rosca, linux-kernel, Peter Zijlstra, Ingo Molnar,
Will Deacon, Frederic Weisbecker, Jiafei Pan, Romain Perier,
Andrew Gabbasov, Dirk Behme, Eugeniu Rosca
Hello Thomas,
On Tue, Mar 16, 2021 at 06:13:12PM +0100, Thomas Gleixner wrote:
> On Tue, Mar 16 2021 at 16:10, Eugeniu Rosca wrote:
> > If no other comments in the next days, I will resubmit your proposal as
> > v2, marked with 'Suggested-by: Thomas Gleixner <tglx@linutronix.de>'.
> >
> > Alternatively, feel free to author the patch and submit it with us in Cc.
>
> Just send a v2 please
https://lore.kernel.org/lkml/20210317102012.32399-1-erosca@de.adit-jv.com/
("[PATCH v2] softirq: s/BUG/WARN_ONCE/ on tasklet SCHED state not set")
--
Best regards,
Eugeniu Rosca
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-17 10:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 15:44 [PATCH] softirq: Be more verbose on t->state BUG() Eugeniu Rosca
2021-03-16 14:31 ` Thomas Gleixner
2021-03-16 15:10 ` Eugeniu Rosca
2021-03-16 17:13 ` Thomas Gleixner
2021-03-17 10:26 ` Eugeniu Rosca
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).