Prev: LOAN OFFERS
Next: [PATCH] GPIO: rdc321x fix inverted gpio data out registers
From: Cyrill Gorcunov on 16 May 2010 05:40
On Sun, May 16, 2010 at 07:39:51AM +0200, Frederic Weisbecker wrote:
....
> static struct jprobe lkdtm;
> @@ -320,6 +326,20 @@ static void lkdtm_do_action(enum ctype which)
> memset(data, 0x78, len);
> break;
> }
> + case SOFTLOCKUP:
> + preempt_disable();
> + for (;;)
> + cpu_relax();
> + break;
> + case HARDLOCKUP:
> + local_irq_disable();
> + for (;;)
> + cpu_relax();
> + break;
> + case HUNG_TASK:
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + schedule();
> + break;
> case NONE:
> default:
> break;

Looks good to me. Btw perhaps we may simplify it a bit:

case HARDLOCKUP:
local_irq_disable();
case SOFTLOCKUP:
preempt_disable();
for (;;)
cpu_relax();
break;

since it'll save a few bytes. What do you think? Did I miss
something?

-- Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 16 May 2010 06:40
On Sun, May 16, 2010 at 01:36:36PM +0400, Cyrill Gorcunov wrote:
> On Sun, May 16, 2010 at 07:39:51AM +0200, Frederic Weisbecker wrote:
> ...
> > static struct jprobe lkdtm;
> > @@ -320,6 +326,20 @@ static void lkdtm_do_action(enum ctype which)
> > memset(data, 0x78, len);
> > break;
> > }
> > + case SOFTLOCKUP:
> > + preempt_disable();
> > + for (;;)
> > + cpu_relax();
> > + break;
> > + case HARDLOCKUP:
> > + local_irq_disable();
> > + for (;;)
> > + cpu_relax();
> > + break;
> > + case HUNG_TASK:
> > + set_current_state(TASK_UNINTERRUPTIBLE);
> > + schedule();
> > + break;
> > case NONE:
> > default:
> > break;
>
> Looks good to me. Btw perhaps we may simplify it a bit:
>
> case HARDLOCKUP:
> local_irq_disable();
> case SOFTLOCKUP:
> preempt_disable();
> for (;;)
> cpu_relax();
> break;
>
> since it'll save a few bytes. What do you think? Did I miss
> something?


It would make the code a bit less clear in that people
might stick on the reason to disable preemption after disabling
irq, especially with a code that already does something rather
unusual ;)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Cyrill Gorcunov on 16 May 2010 06:50
On Sun, May 16, 2010 at 12:35:08PM +0200, Frederic Weisbecker wrote:
....
> > Looks good to me. Btw perhaps we may simplify it a bit:
> >
> > case HARDLOCKUP:
> > local_irq_disable();
> > case SOFTLOCKUP:
> > preempt_disable();
> > for (;;)
> > cpu_relax();
> > break;
> >
> > since it'll save a few bytes. What do you think? Did I miss
> > something?
>
>
> It would make the code a bit less clear in that people
> might stick on the reason to disable preemption after disabling
> irq, especially with a code that already does something rather
> unusual ;)
>

ok, convinced

-- Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: LOAN OFFERS
Next: [PATCH] GPIO: rdc321x fix inverted gpio data out registers