preempt-count oddities

preempt-count oddities

[Jesper Juhl]

While looking through the tree and checking out possible signedness issues 
pointed out to me by gcc -W I came across this one : 
  kernel/timer.c:464: warning: comparison between signed and unsigned
The issue there is this little bit of code :
  [...]
  462        u32 preempt_count = preempt_count();
  463        fn(data);
  464        if (preempt_count != preempt_count()) {
  [...]
gcc is complaining about that since preempt_count in struct thread_info 
(which is what preempt_count() returns) is a signed integer. Initially I 
thought "that's a bit sloppy, I'll just make the local variable a s32 and 
that should be that", but then I looked a little closer at struct 
thread_info for the different archs, and found that the type used differs 
between archs and it's not even a signed type on all (s390 being the 
unsigned exception). So all of a sudden fixing up this little warning was 
not so simple after all.
The different types used for struct thread_info.preempt_count are 
__s32, int and unsigned int.

Why are different types used for struct thread_info.preempt_count? Would 
it not make sense to make it a __s32 or plain int on all archs? I would 
think that consistency here would be a good thing.
And why on earth is it an unsigned int on s390? It seems to me that that 
makes it impossible to catch bugs where preempt_count is decremented below 
zero.

Any reason not to apply a patch like this one? 
my choice of 'int' over '__s32' was pretty arbitrary - as far as I know we 
don't support any archs where 'int' is less than 32bits, do we? So I 
figured that using a plain int type should give us at least 32 bits 
everywhere as well as using the given platforms fastest type. If any of 
the archs have <32bit int types, then I guess __s32 would be a better 
choice.

Signed-of-by: Jesper Juhl <juhl-lkml@dif.dk>

--- linux-2.6.12-rc2-mm3-orig/include/asm-arm/thread_info.h	2005-03-02 08:38:08.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-arm/thread_info.h	2005-04-23 23:16:04.000000000 +0200
@@ -45,7 +45,7 @@
  */
 struct thread_info {
 	unsigned long		flags;		/* low level flags */
-	__s32			preempt_count;	/* 0 => preemptable, <0 => bug */
+	int			preempt_count;	/* 0 => preemptable, <0 => bug */
 	mm_segment_t		addr_limit;	/* address limit */
 	struct task_struct	*task;		/* main task structure */
 	struct exec_domain	*exec_domain;	/* execution domain */
--- linux-2.6.12-rc2-mm3-orig/include/asm-arm26/thread_info.h	2005-03-02 08:37:50.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-arm26/thread_info.h	2005-04-23 23:16:22.000000000 +0200
@@ -44,7 +44,7 @@
  */
 struct thread_info {
 	unsigned long		flags;		/* low level flags */
-	__s32			preempt_count;	/* 0 => preemptable, <0 => bug */
+	int			preempt_count;	/* 0 => preemptable, <0 => bug */
 	mm_segment_t		addr_limit;	/* address limit */
 	struct task_struct	*task;		/* main task structure */
 	struct exec_domain      *exec_domain;   /* execution domain */
--- linux-2.6.12-rc2-mm3-orig/include/asm-cris/thread_info.h	2005-03-02 08:38:32.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-cris/thread_info.h	2005-04-23 23:16:29.000000000 +0200
@@ -31,7 +31,7 @@
 	struct exec_domain	*exec_domain;	/* execution domain */
 	unsigned long		flags;		/* low level flags */
 	__u32			cpu;		/* current CPU */
-	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
+	int			preempt_count; /* 0 => preemptable, <0 => BUG */
 
 	mm_segment_t		addr_limit;	/* thread address space:
 					 	   0-0xBFFFFFFF for user-thead
--- linux-2.6.12-rc2-mm3-orig/include/asm-frv/thread_info.h	2005-03-02 08:37:50.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-frv/thread_info.h	2005-04-23 23:16:37.000000000 +0200
@@ -33,7 +33,7 @@
 	unsigned long		flags;		/* low level flags */
 	unsigned long		status;		/* thread-synchronous flags */
 	__u32			cpu;		/* current CPU */
-	__s32			preempt_count;	/* 0 => preemptable, <0 => BUG */
+	int			preempt_count;	/* 0 => preemptable, <0 => BUG */
 
 	mm_segment_t		addr_limit;	/* thread address space:
 					 	   0-0xBFFFFFFF for user-thead
--- linux-2.6.12-rc2-mm3-orig/include/asm-i386/thread_info.h	2005-04-05 21:21:48.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-i386/thread_info.h	2005-04-23 23:16:50.000000000 +0200
@@ -31,7 +31,7 @@
 	unsigned long		flags;		/* low level flags */
 	unsigned long		status;		/* thread-synchronous flags */
 	__u32			cpu;		/* current CPU */
-	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
+	int			preempt_count; /* 0 => preemptable, <0 => BUG */
 
 
 	mm_segment_t		addr_limit;	/* thread address space:
--- linux-2.6.12-rc2-mm3-orig/include/asm-ia64/thread_info.h	2005-03-02 08:38:33.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-ia64/thread_info.h	2005-04-23 23:17:04.000000000 +0200
@@ -25,7 +25,7 @@
 	__u32 flags;			/* thread_info flags (see TIF_*) */
 	__u32 cpu;			/* current CPU */
 	mm_segment_t addr_limit;	/* user-level address space limit */
-	__s32 preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
+	int preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
 	struct restart_block restart_block;
 	struct {
 		int signo;
--- linux-2.6.12-rc2-mm3-orig/include/asm-m32r/thread_info.h	2005-03-02 08:38:26.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-m32r/thread_info.h	2005-04-23 23:17:21.000000000 +0200
@@ -28,7 +28,7 @@
 	unsigned long		flags;		/* low level flags */
 	unsigned long		status;		/* thread-synchronous flags */
 	__u32			cpu;		/* current CPU */
-	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
+	int			preempt_count; /* 0 => preemptable, <0 => BUG */
 
 	mm_segment_t		addr_limit;	/* thread address space:
 					 	   0-0xBFFFFFFF for user-thread
--- linux-2.6.12-rc2-mm3-orig/include/asm-m68k/thread_info.h	2005-04-05 21:21:49.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-m68k/thread_info.h	2005-04-23 23:17:29.000000000 +0200
@@ -8,7 +8,7 @@
 struct thread_info {
 	struct task_struct	*task;		/* main task structure */
 	struct exec_domain	*exec_domain;	/* execution domain */
-	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
+	int			preempt_count; /* 0 => preemptable, <0 => BUG */
 	__u32 cpu; /* should always be 0 on m68k */
 	struct restart_block    restart_block;
 
--- linux-2.6.12-rc2-mm3-orig/include/asm-mips/thread_info.h	2005-03-02 08:37:30.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-mips/thread_info.h	2005-04-23 23:17:47.000000000 +0200
@@ -27,7 +27,7 @@
 	struct exec_domain	*exec_domain;	/* execution domain */
 	unsigned long		flags;		/* low level flags */
 	__u32			cpu;		/* current CPU */
-	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
+	int			preempt_count; /* 0 => preemptable, <0 => BUG */
 
 	mm_segment_t		addr_limit;	/* thread address space:
 					 	   0-0xBFFFFFFF for user-thead
--- linux-2.6.12-rc2-mm3-orig/include/asm-parisc/thread_info.h	2005-04-05 21:21:49.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-parisc/thread_info.h	2005-04-23 23:17:55.000000000 +0200
@@ -12,7 +12,7 @@
 	unsigned long flags;		/* thread_info flags (see TIF_*) */
 	mm_segment_t addr_limit;	/* user-level address space limit */
 	__u32 cpu;			/* current CPU */
-	__s32 preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
+	int preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
 	struct restart_block restart_block;
 };
 
--- linux-2.6.12-rc2-mm3-orig/include/asm-s390/thread_info.h	2005-03-02 08:38:13.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-s390/thread_info.h	2005-04-23 23:18:12.000000000 +0200
@@ -50,7 +50,7 @@
 	struct exec_domain	*exec_domain;	/* execution domain */
 	unsigned long		flags;		/* low level flags */
 	unsigned int		cpu;		/* current CPU */
-	unsigned int		preempt_count; /* 0 => preemptable */
+	int			preempt_count; /* 0 => preemptable */
 	struct restart_block	restart_block;
 };
 
--- linux-2.6.12-rc2-mm3-orig/include/asm-sh/thread_info.h	2005-03-02 08:38:13.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-sh/thread_info.h	2005-04-23 23:18:20.000000000 +0200
@@ -20,7 +20,7 @@
 	struct exec_domain	*exec_domain;	/* execution domain */
 	__u32			flags;		/* low level flags */
 	__u32			cpu;
-	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
+	int			preempt_count; /* 0 => preemptable, <0 => BUG */
 	struct restart_block	restart_block;
 	__u8			supervisor_stack[0];
 };
--- linux-2.6.12-rc2-mm3-orig/include/asm-sh64/thread_info.h	2005-04-05 21:21:49.000000000 +0200
+++ linux-2.6.12-rc2-mm3/include/asm-sh64/thread_info.h	2005-04-23 23:18:27.000000000 +0200
@@ -22,7 +22,7 @@
 	struct exec_domain	*exec_domain;	/* execution domain */
 	unsigned long		flags;		/* low level flags */
 	/* Put the 4 32-bit fields together to make asm offsetting easier. */
-	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
+	int			preempt_count; /* 0 => preemptable, <0 => BUG */
 	__u16			cpu;
 
 	mm_segment_t		addr_limit;
--- linux-2.6.12-rc2-mm3-orig/include/asm-um/thread_info.h	2005-03-02 08:37:54.000000000 +0100
+++ linux-2.6.12-rc2-mm3/include/asm-um/thread_info.h	2005-04-23 23:18:50.000000000 +0200
@@ -17,7 +17,7 @@
 	struct exec_domain	*exec_domain;	/* execution domain */
 	unsigned long		flags;		/* low level flags */
 	__u32			cpu;		/* current CPU */
-	__s32			preempt_count;  /* 0 => preemptable, 
+	int			preempt_count;  /* 0 => preemptable, 
 						   <0 => BUG */
 	mm_segment_t		addr_limit;	/* thread address space:
 					 	   0-0xBFFFFFFF for user


and then the litle detail in kernel/timer.c can be fixed nicely like this: 

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

--- linux-2.6.12-rc2-mm3-orig/kernel/timer.c	2005-04-11 21:20:56.000000000 +0200
+++ linux-2.6.12-rc2-mm3/kernel/timer.c	2005-04-23 23:19:21.000000000 +0200
@@ -459,7 +459,7 @@ static inline void __run_timers(tvec_bas
 			detach_timer(timer, 1);
 			spin_unlock_irq(&base->t_base.lock);
 			{
-				u32 preempt_count = preempt_count();
+				int preempt_count = preempt_count();
 				fn(data);
 				if (preempt_count != preempt_count()) {
 					printk("huh, entered %p with %08x, exited with %08x?\n", fn, preempt_count, preempt_count());


or is there some good reason I'm missing that make the above a bad idea?


-- 
Jesper Juhl

PS. Please CC: me on replies.

Re: preempt-count oddities - still looking for comments :)

[Jesper Juhl]

Replying to myself here since the initial mail got no response. Here's 
hoping that it showing up on the list again draws some comments :-)

-- 
Jesper


On Sat, 23 Apr 2005, Jesper Juhl wrote:

> While looking through the tree and checking out possible signedness issues 
> pointed out to me by gcc -W I came across this one : 
>   kernel/timer.c:464: warning: comparison between signed and unsigned
> The issue there is this little bit of code :
>   [...]
>   462        u32 preempt_count = preempt_count();
>   463        fn(data);
>   464        if (preempt_count != preempt_count()) {
>   [...]
> gcc is complaining about that since preempt_count in struct thread_info 
> (which is what preempt_count() returns) is a signed integer. Initially I 
> thought "that's a bit sloppy, I'll just make the local variable a s32 and 
> that should be that", but then I looked a little closer at struct 
> thread_info for the different archs, and found that the type used differs 
> between archs and it's not even a signed type on all (s390 being the 
> unsigned exception). So all of a sudden fixing up this little warning was 
> not so simple after all.
> The different types used for struct thread_info.preempt_count are 
> __s32, int and unsigned int.
> 
> Why are different types used for struct thread_info.preempt_count? Would 
> it not make sense to make it a __s32 or plain int on all archs? I would 
> think that consistency here would be a good thing.
> And why on earth is it an unsigned int on s390? It seems to me that that 
> makes it impossible to catch bugs where preempt_count is decremented below 
> zero.
> 
> Any reason not to apply a patch like this one? 
> my choice of 'int' over '__s32' was pretty arbitrary - as far as I know we 
> don't support any archs where 'int' is less than 32bits, do we? So I 
> figured that using a plain int type should give us at least 32 bits 
> everywhere as well as using the given platforms fastest type. If any of 
> the archs have <32bit int types, then I guess __s32 would be a better 
> choice.
> 
> Signed-of-by: Jesper Juhl <juhl-lkml@dif.dk>
> 
> --- linux-2.6.12-rc2-mm3-orig/include/asm-arm/thread_info.h	2005-03-02 08:38:08.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-arm/thread_info.h	2005-04-23 23:16:04.000000000 +0200
> @@ -45,7 +45,7 @@
>   */
>  struct thread_info {
>  	unsigned long		flags;		/* low level flags */
> -	__s32			preempt_count;	/* 0 => preemptable, <0 => bug */
> +	int			preempt_count;	/* 0 => preemptable, <0 => bug */
>  	mm_segment_t		addr_limit;	/* address limit */
>  	struct task_struct	*task;		/* main task structure */
>  	struct exec_domain	*exec_domain;	/* execution domain */
> --- linux-2.6.12-rc2-mm3-orig/include/asm-arm26/thread_info.h	2005-03-02 08:37:50.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-arm26/thread_info.h	2005-04-23 23:16:22.000000000 +0200
> @@ -44,7 +44,7 @@
>   */
>  struct thread_info {
>  	unsigned long		flags;		/* low level flags */
> -	__s32			preempt_count;	/* 0 => preemptable, <0 => bug */
> +	int			preempt_count;	/* 0 => preemptable, <0 => bug */
>  	mm_segment_t		addr_limit;	/* address limit */
>  	struct task_struct	*task;		/* main task structure */
>  	struct exec_domain      *exec_domain;   /* execution domain */
> --- linux-2.6.12-rc2-mm3-orig/include/asm-cris/thread_info.h	2005-03-02 08:38:32.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-cris/thread_info.h	2005-04-23 23:16:29.000000000 +0200
> @@ -31,7 +31,7 @@
>  	struct exec_domain	*exec_domain;	/* execution domain */
>  	unsigned long		flags;		/* low level flags */
>  	__u32			cpu;		/* current CPU */
> -	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
> +	int			preempt_count; /* 0 => preemptable, <0 => BUG */
>  
>  	mm_segment_t		addr_limit;	/* thread address space:
>  					 	   0-0xBFFFFFFF for user-thead
> --- linux-2.6.12-rc2-mm3-orig/include/asm-frv/thread_info.h	2005-03-02 08:37:50.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-frv/thread_info.h	2005-04-23 23:16:37.000000000 +0200
> @@ -33,7 +33,7 @@
>  	unsigned long		flags;		/* low level flags */
>  	unsigned long		status;		/* thread-synchronous flags */
>  	__u32			cpu;		/* current CPU */
> -	__s32			preempt_count;	/* 0 => preemptable, <0 => BUG */
> +	int			preempt_count;	/* 0 => preemptable, <0 => BUG */
>  
>  	mm_segment_t		addr_limit;	/* thread address space:
>  					 	   0-0xBFFFFFFF for user-thead
> --- linux-2.6.12-rc2-mm3-orig/include/asm-i386/thread_info.h	2005-04-05 21:21:48.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-i386/thread_info.h	2005-04-23 23:16:50.000000000 +0200
> @@ -31,7 +31,7 @@
>  	unsigned long		flags;		/* low level flags */
>  	unsigned long		status;		/* thread-synchronous flags */
>  	__u32			cpu;		/* current CPU */
> -	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
> +	int			preempt_count; /* 0 => preemptable, <0 => BUG */
>  
>  
>  	mm_segment_t		addr_limit;	/* thread address space:
> --- linux-2.6.12-rc2-mm3-orig/include/asm-ia64/thread_info.h	2005-03-02 08:38:33.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-ia64/thread_info.h	2005-04-23 23:17:04.000000000 +0200
> @@ -25,7 +25,7 @@
>  	__u32 flags;			/* thread_info flags (see TIF_*) */
>  	__u32 cpu;			/* current CPU */
>  	mm_segment_t addr_limit;	/* user-level address space limit */
> -	__s32 preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
> +	int preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
>  	struct restart_block restart_block;
>  	struct {
>  		int signo;
> --- linux-2.6.12-rc2-mm3-orig/include/asm-m32r/thread_info.h	2005-03-02 08:38:26.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-m32r/thread_info.h	2005-04-23 23:17:21.000000000 +0200
> @@ -28,7 +28,7 @@
>  	unsigned long		flags;		/* low level flags */
>  	unsigned long		status;		/* thread-synchronous flags */
>  	__u32			cpu;		/* current CPU */
> -	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
> +	int			preempt_count; /* 0 => preemptable, <0 => BUG */
>  
>  	mm_segment_t		addr_limit;	/* thread address space:
>  					 	   0-0xBFFFFFFF for user-thread
> --- linux-2.6.12-rc2-mm3-orig/include/asm-m68k/thread_info.h	2005-04-05 21:21:49.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-m68k/thread_info.h	2005-04-23 23:17:29.000000000 +0200
> @@ -8,7 +8,7 @@
>  struct thread_info {
>  	struct task_struct	*task;		/* main task structure */
>  	struct exec_domain	*exec_domain;	/* execution domain */
> -	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
> +	int			preempt_count; /* 0 => preemptable, <0 => BUG */
>  	__u32 cpu; /* should always be 0 on m68k */
>  	struct restart_block    restart_block;
>  
> --- linux-2.6.12-rc2-mm3-orig/include/asm-mips/thread_info.h	2005-03-02 08:37:30.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-mips/thread_info.h	2005-04-23 23:17:47.000000000 +0200
> @@ -27,7 +27,7 @@
>  	struct exec_domain	*exec_domain;	/* execution domain */
>  	unsigned long		flags;		/* low level flags */
>  	__u32			cpu;		/* current CPU */
> -	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
> +	int			preempt_count; /* 0 => preemptable, <0 => BUG */
>  
>  	mm_segment_t		addr_limit;	/* thread address space:
>  					 	   0-0xBFFFFFFF for user-thead
> --- linux-2.6.12-rc2-mm3-orig/include/asm-parisc/thread_info.h	2005-04-05 21:21:49.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-parisc/thread_info.h	2005-04-23 23:17:55.000000000 +0200
> @@ -12,7 +12,7 @@
>  	unsigned long flags;		/* thread_info flags (see TIF_*) */
>  	mm_segment_t addr_limit;	/* user-level address space limit */
>  	__u32 cpu;			/* current CPU */
> -	__s32 preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
> +	int preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
>  	struct restart_block restart_block;
>  };
>  
> --- linux-2.6.12-rc2-mm3-orig/include/asm-s390/thread_info.h	2005-03-02 08:38:13.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-s390/thread_info.h	2005-04-23 23:18:12.000000000 +0200
> @@ -50,7 +50,7 @@
>  	struct exec_domain	*exec_domain;	/* execution domain */
>  	unsigned long		flags;		/* low level flags */
>  	unsigned int		cpu;		/* current CPU */
> -	unsigned int		preempt_count; /* 0 => preemptable */
> +	int			preempt_count; /* 0 => preemptable */
>  	struct restart_block	restart_block;
>  };
>  
> --- linux-2.6.12-rc2-mm3-orig/include/asm-sh/thread_info.h	2005-03-02 08:38:13.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-sh/thread_info.h	2005-04-23 23:18:20.000000000 +0200
> @@ -20,7 +20,7 @@
>  	struct exec_domain	*exec_domain;	/* execution domain */
>  	__u32			flags;		/* low level flags */
>  	__u32			cpu;
> -	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
> +	int			preempt_count; /* 0 => preemptable, <0 => BUG */
>  	struct restart_block	restart_block;
>  	__u8			supervisor_stack[0];
>  };
> --- linux-2.6.12-rc2-mm3-orig/include/asm-sh64/thread_info.h	2005-04-05 21:21:49.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/include/asm-sh64/thread_info.h	2005-04-23 23:18:27.000000000 +0200
> @@ -22,7 +22,7 @@
>  	struct exec_domain	*exec_domain;	/* execution domain */
>  	unsigned long		flags;		/* low level flags */
>  	/* Put the 4 32-bit fields together to make asm offsetting easier. */
> -	__s32			preempt_count; /* 0 => preemptable, <0 => BUG */
> +	int			preempt_count; /* 0 => preemptable, <0 => BUG */
>  	__u16			cpu;
>  
>  	mm_segment_t		addr_limit;
> --- linux-2.6.12-rc2-mm3-orig/include/asm-um/thread_info.h	2005-03-02 08:37:54.000000000 +0100
> +++ linux-2.6.12-rc2-mm3/include/asm-um/thread_info.h	2005-04-23 23:18:50.000000000 +0200
> @@ -17,7 +17,7 @@
>  	struct exec_domain	*exec_domain;	/* execution domain */
>  	unsigned long		flags;		/* low level flags */
>  	__u32			cpu;		/* current CPU */
> -	__s32			preempt_count;  /* 0 => preemptable, 
> +	int			preempt_count;  /* 0 => preemptable, 
>  						   <0 => BUG */
>  	mm_segment_t		addr_limit;	/* thread address space:
>  					 	   0-0xBFFFFFFF for user
> 
> 
> and then the litle detail in kernel/timer.c can be fixed nicely like this: 
> 
> Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
> 
> --- linux-2.6.12-rc2-mm3-orig/kernel/timer.c	2005-04-11 21:20:56.000000000 +0200
> +++ linux-2.6.12-rc2-mm3/kernel/timer.c	2005-04-23 23:19:21.000000000 +0200
> @@ -459,7 +459,7 @@ static inline void __run_timers(tvec_bas
>  			detach_timer(timer, 1);
>  			spin_unlock_irq(&base->t_base.lock);
>  			{
> -				u32 preempt_count = preempt_count();
> +				int preempt_count = preempt_count();
>  				fn(data);
>  				if (preempt_count != preempt_count()) {
>  					printk("huh, entered %p with %08x, exited with %08x?\n", fn, preempt_count, preempt_count());
> 
> 
> or is there some good reason I'm missing that make the above a bad idea?
> 
> 
> -- 
> Jesper Juhl
> 
> PS. Please CC: me on replies.
> 
> 

Re: preempt-count oddities - still looking for comments :)

[Robert Love]

On Tue, 2005-04-26 at 19:31 +0200, Jesper Juhl wrote:
> Replying to myself here since the initial mail got no response. Here's 
> hoping that it showing up on the list again draws some comments :-)

I didn't think it was that big of a deal.  ;-)

It seems the right approach.  Personally, I would of made the type an
s32, since fixed-sizes seems to be sensible in the thread_info
structure, but an int is the same thing.  Cool with me.

Acked-by: Robert Love <rml@novell.com>

	Robert Love

Re: preempt-count oddities - still looking for comments :)

[Robert Love]

On Tue, 2005-04-26 at 19:46 +0200, Jesper Juhl wrote:

> I'll update the patch(es) then and use __s32 in the structure and s32 
> elsewhere.

You can actually use s32 everywhere, since the structure is never
exported to user-space...although some architectures already have the
__ugly versions in there.

	Robert Love

Re: preempt-count oddities - still looking for comments :)

[Jesper Juhl]

On Tue, 26 Apr 2005, Robert Love wrote:

> On Tue, 2005-04-26 at 19:31 +0200, Jesper Juhl wrote:
> > Replying to myself here since the initial mail got no response. Here's 
> > hoping that it showing up on the list again draws some comments :-)
> 
> I didn't think it was that big of a deal.  ;-)
> 
It's not really that big of a deal. I was just currious if I'd gotten it 
right since I spend a good deal of time digging for possible reasons for 
the differences (and finding none). :-)

> It seems the right approach.  Personally, I would of made the type an
> s32, since fixed-sizes seems to be sensible in the thread_info
> structure, but an int is the same thing.  Cool with me.
> 
I'll update the patch(es) then and use __s32 in the structure and s32 
elsewhere.

> Acked-by: Robert Love <rml@novell.com>
> 
Thanks.

> 	Robert Love
> 

-- 
Jesper Juhl

Re: preempt-count oddities - still looking for comments :)

[Jesper Juhl]

On Tue, 26 Apr 2005, Robert Love wrote:

> On Tue, 2005-04-26 at 19:46 +0200, Jesper Juhl wrote:
> 
> > I'll update the patch(es) then and use __s32 in the structure and s32 
> > elsewhere.
> 
> You can actually use s32 everywhere, since the structure is never
> exported to user-space...

Right, I'll do that then.

> although some architectures already have the
> __ugly versions in there.
> 

Not for long :)


-- 
Jesper

Re: preempt-count oddities - still looking for comments :)

[Jesper Juhl]

On Tue, 26 Apr 2005, Robert Love wrote:

> On Tue, 2005-04-26 at 19:46 +0200, Jesper Juhl wrote:
> 
> > I'll update the patch(es) then and use __s32 in the structure and s32 
> > elsewhere.
> 
> You can actually use s32 everywhere, since the structure is never
> exported to user-space...although some architectures already have the
> __ugly versions in there.
> 
Hmm, one downside to using "s32" instead of plain "int" is that not all 
thread_info.h files get asm/types.h pulled in and then won't have that 
type defined (m68knommu is one such as far as I can see). Would this make 
"int" prefered after all or should I just include asm/types.h where needed 
or just include it everywhere? seems logical that the file that uses 
header includes it directly instead of it getting included implicitly by 
other headers (like i386 where thread_info.h includes asm/page.h that then 
includes asm/mmx.h that then includes linux/types.h that finally includes 
asm/types.h).
Personally I'd just add the asm/types.h include to all the thread_info.h 
files (or go back to using int) - what's your preference?

-- 
Jesper

Re: preempt-count oddities - still looking for comments :)

[Robert Love]

On Tue, 2005-04-26 at 22:05 +0200, Jesper Juhl wrote:

> Hmm, one downside to using "s32" instead of plain "int" is that not all 
> thread_info.h files get asm/types.h pulled in and then won't have that 
> type defined (m68knommu is one such as far as I can see). Would this make 
> "int" prefered after all or should I just include asm/types.h where needed 
> or just include it everywhere? seems logical that the file that uses 
> header includes it directly instead of it getting included implicitly by 
> other headers (like i386 where thread_info.h includes asm/page.h that then 
> includes asm/mmx.h that then includes linux/types.h that finally includes 
> asm/types.h).
> Personally I'd just add the asm/types.h include to all the thread_info.h 
> files (or go back to using int) - what's your preference?

Well, guess it depends how much we like s32 over int.  Both are
identical on all supported architectures, so it is just a style issue,
really.

If m68knommu is the only arch needing asm/typed.h included, I'd so just
include it.  If more and more arches need it, just go with int.

It is probably an easier sell.

	Robert Love

Re: preempt-count oddities - still looking for comments :)

[Jesper Juhl]

On Tue, 26 Apr 2005, Robert Love wrote:

> On Tue, 2005-04-26 at 22:05 +0200, Jesper Juhl wrote:
> 
> > Hmm, one downside to using "s32" instead of plain "int" is that not all 
> > thread_info.h files get asm/types.h pulled in and then won't have that 
> > type defined (m68knommu is one such as far as I can see). Would this make 
> > "int" prefered after all or should I just include asm/types.h where needed 
> > or just include it everywhere? seems logical that the file that uses 
> > header includes it directly instead of it getting included implicitly by 
> > other headers (like i386 where thread_info.h includes asm/page.h that then 
> > includes asm/mmx.h that then includes linux/types.h that finally includes 
> > asm/types.h).
> > Personally I'd just add the asm/types.h include to all the thread_info.h 
> > files (or go back to using int) - what's your preference?
> 
> Well, guess it depends how much we like s32 over int.  Both are
> identical on all supported architectures, so it is just a style issue,
> really.
> 
> If m68knommu is the only arch needing asm/typed.h included, I'd so just
> include it.  If more and more arches need it, just go with int.
> 
Quite a few would need it (I just checked), so I'll just stick to int.

Thank you for taking the time to reply and comment on this very low 
priority thing. It's appreciated.

-- 
Jesper