Re: [PATCH] ftrace: handle archs that do not support irqs_disabled_flags

Re: [PATCH] ftrace: handle archs that do not support irqs_disabled_flags

[Geert Uytterhoeven]

On Fri, 24 Oct 2008, Steven Rostedt wrote:
> Some architectures do not support a way to read the irq flags that
> is set from "local_irq_save(flags)" to determine if interrupts were
> disabled or enabled. Ftrace uses this information to display to the user
> if the trace occurred with interrupts enabled or disabled.

Both alpha

	#define irqs_disabled() (getipl() == IPL_MAX)

and m68k

	static inline int irqs_disabled(void)
	{
		unsigned long flags;
		local_save_flags(flags);
		return flags & ~ALLOWINT;
	}

do have irqs_disabled(), but they don't have irqs_disabled_flags().

M68knommu has both, but they don't check the same thing:

	#define irqs_disabled()                 \
	({                                      \
		unsigned long flags;            \
		local_save_flags(flags);        \
		((flags & 0x0700) == 0x0700);   \
	})

	static inline int irqs_disabled_flags(unsigned long flags)
	{
		if (flags & 0x0700)
			return 0;
		else
			return 1;
	}

Is there a semantic difference between them (except that the latter takes the
flags as a parameter)?

Or can we just extract the core logic of irqs_disabled() into
irqs_disabled_flags()?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] ftrace: handle archs that do not support irqs_disabled_flags

[Greg Ungerer]

Hi Geert,

Geert Uytterhoeven wrote:
> On Fri, 24 Oct 2008, Steven Rostedt wrote:
>> Some architectures do not support a way to read the irq flags that
>> is set from "local_irq_save(flags)" to determine if interrupts were
>> disabled or enabled. Ftrace uses this information to display to the user
>> if the trace occurred with interrupts enabled or disabled.
> 
> Both alpha
> 
> 	#define irqs_disabled() (getipl() == IPL_MAX)
> 
> and m68k
> 
> 	static inline int irqs_disabled(void)
> 	{
> 		unsigned long flags;
> 		local_save_flags(flags);
> 		return flags & ~ALLOWINT;
> 	}
> 
> do have irqs_disabled(), but they don't have irqs_disabled_flags().
> 
> M68knommu has both, but they don't check the same thing:
> 
> 	#define irqs_disabled()                 \
> 	({                                      \
> 		unsigned long flags;            \
> 		local_save_flags(flags);        \
> 		((flags & 0x0700) == 0x0700);   \
> 	})
> 
> 	static inline int irqs_disabled_flags(unsigned long flags)
> 	{
> 		if (flags & 0x0700)
> 			return 0;
> 		else
> 			return 1;
> 	}
> 
> Is there a semantic difference between them (except that the latter takes the
> flags as a parameter)?

No...


> Or can we just extract the core logic of irqs_disabled() into
> irqs_disabled_flags()?

Yep, could certainly do that. I'll put a patch together.

Regards
Greg



------------------------------------------------------------------------
Greg Ungerer  --  Chief Software Dude       EMAIL:     gerg@snapgear.com
Secure Computing Corporation                PHONE:       +61 7 3435 2888
825 Stanley St,                             FAX:         +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia         WEB: http://www.SnapGear.com

Re: [PATCH] tracing: fix a build error on alpha and m68k

[Geert Uytterhoeven]

On Fri, 31 Oct 2008, Alexey Dobriyan wrote:
> On Thu, Oct 23, 2008 at 08:34:33PM +0400, Alexey Dobriyan wrote:
> > On Thu, Oct 23, 2008 at 07:27:48PM +0200, Frederic Weisbecker wrote:
> > > When tracing is enabled, some arch have included <linux/irqflags.h>
> > > on their <asm/system.h> but others like alpha or m68k don't.
> > > 
> > > Build error on alpha:
> > > 
> > > kernel/trace/trace.c: In function 'tracing_generic_entry_update':
> > > kernel/trace/trace.c:658: error: implicit declaration of function 'irqs_disabled_flags'
> 
> FYI, this error still exists in mainline (7f82f000ed030d1108b4de47d9e2d556092980c6).

Now it hit mainline (I can't be bothered to develop _new_ code to fix things in
-next ;-), I'm implementing irqs_disabled_flags() for m68k. Stay tuned...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] tracing: fix a build error on alpha and m68k

[Ingo Molnar]

* Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On Fri, 31 Oct 2008, Alexey Dobriyan wrote:
> > On Thu, Oct 23, 2008 at 08:34:33PM +0400, Alexey Dobriyan wrote:
> > > On Thu, Oct 23, 2008 at 07:27:48PM +0200, Frederic Weisbecker wrote:
> > > > When tracing is enabled, some arch have included <linux/irqflags.h>
> > > > on their <asm/system.h> but others like alpha or m68k don't.
> > > > 
> > > > Build error on alpha:
> > > > 
> > > > kernel/trace/trace.c: In function 'tracing_generic_entry_update':
> > > > kernel/trace/trace.c:658: error: implicit declaration of function 'irqs_disabled_flags'
> > 
> > FYI, this error still exists in mainline (7f82f000ed030d1108b4de47d9e2d556092980c6).
> 
> Now it hit mainline (I can't be bothered to develop _new_ code to 
> fix things in -next ;-), I'm implementing irqs_disabled_flags() for 
> m68k. Stay tuned...

this (now mainline) commit should have fixed it:

 9244489: ftrace: handle archs that do not support irqs_disabled_flags

can you still see problems?

	Ingo

Re: [PATCH] tracing: fix a build error on alpha and m68k

[Geert Uytterhoeven]

On Fri, 31 Oct 2008, Ingo Molnar wrote:
> * Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, 31 Oct 2008, Alexey Dobriyan wrote:
> > > On Thu, Oct 23, 2008 at 08:34:33PM +0400, Alexey Dobriyan wrote:
> > > > On Thu, Oct 23, 2008 at 07:27:48PM +0200, Frederic Weisbecker wrote:
> > > > > When tracing is enabled, some arch have included <linux/irqflags.h>
> > > > > on their <asm/system.h> but others like alpha or m68k don't.
> > > > > 
> > > > > Build error on alpha:
> > > > > 
> > > > > kernel/trace/trace.c: In function 'tracing_generic_entry_update':
> > > > > kernel/trace/trace.c:658: error: implicit declaration of function 'irqs_disabled_flags'
> > > 
> > > FYI, this error still exists in mainline (7f82f000ed030d1108b4de47d9e2d556092980c6).
> > 
> > Now it hit mainline (I can't be bothered to develop _new_ code to 
> > fix things in -next ;-), I'm implementing irqs_disabled_flags() for 
> > m68k. Stay tuned...
> 
> this (now mainline) commit should have fixed it:
> 
>  9244489: ftrace: handle archs that do not support irqs_disabled_flags
> 
> can you still see problems?

Yes, the "undefined reference to `save_stack_trace'", as already pointed out
by Al. I'm trying his fix now...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds