user-mode port 0.44-2.4.7

user-mode port 0.44-2.4.7

[Jeff Dike]

The user-mode port of 2.4.7 is available.

In a minor packaging breakthrogh, a .deb for UML is now available.

The UML block driver now supports a read-write COW layer above a shared 
read-only filesystem.  This allows multiple UMLs to boot off the same 
filesystem.  See http://user-mode-linux.sourceforge.net/shared_fs.html for
more information.

The ppc port is now fully merged.

The pid file and mconsole socket are now located in a directory defined 
by the UML umid.

There is now IO memory emulation.  This allows a host file to be mapped by a
UML driver, which can provide whatever interface it wants to that file to 
UML processes.  This is a first step towards doing hardware driver development
under UML.

gdbs are now killed properly.

A nasty bug involving a misunderstanding with FASTCALL was fixed.

Block devices and network devices are now pluggable from the mconsole - they
can be added to and removed from a running system.  See 
http://user-mode-linux.sourceforge.net/mconsole.html for more information.

SIGHUP no longer causes UML to go crazy.

The project's home page is http://user-mode-linux.sourceforge.net

Downloads are available at 
	http://sourceforge.net/project/showfiles.php?group_id=429
	ftp://ftp.nl.linux.org/pub/uml/
	http://uml-pub.ists.dartmouth.edu/uml/

				Jeff

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 12:08:04AM -0500, Jeff Dike wrote:
> The user-mode port of 2.4.7 is available.

in my tree I did some further cleanup, here the ones that you can
interested about:

diff -urN uml-ref/arch/um/include/kern_util.h uml/arch/um/include/kern_util.h
--- uml-ref/arch/um/include/kern_util.h	Mon Jul 23 17:07:17 2001
+++ uml/arch/um/include/kern_util.h	Mon Jul 23 17:08:33 2001
@@ -28,7 +28,6 @@
 extern long execute_syscall(struct sys_pt_regs regs);
 extern void syscall_segv(int sig);
 extern int current_pid(void);
-extern void do_bh(void);
 extern void set_init_pid(int pid);
 extern unsigned long alloc_stack(void);
 extern int do_signal(unsigned long *error, int *again_out);
diff -urN uml-ref/arch/um/kernel/process_kern.c uml/arch/um/kernel/process_kern.c
--- uml-ref/arch/um/kernel/process_kern.c	Mon Jul 23 17:07:17 2001
+++ uml/arch/um/kernel/process_kern.c	Mon Jul 23 17:09:54 2001
@@ -217,25 +217,12 @@
 	return(current->thread.request.u.cswitch.from);
 }
 
-void do_bh(void)
-{
-#ifndef CONFIG_SMP
-	if (softirq_pending(0)){
-		do_softirq();
-		unblock_signals();
-	}
-#else
-#error Need to update do_bh
-#endif
-}
-
 void ret_from_sys_call(void *t)
 {
 	struct task_struct *task;
 
 	task = t;
 	if(task == NULL) task = current;
-	do_bh();
 	if(task->need_resched) schedule();
 	if(task->sigpending != 0) do_signal(NULL, NULL);
 }
diff -urN uml-ref/arch/um/kernel/time.c uml/arch/um/kernel/time.c
--- uml-ref/arch/um/kernel/time.c	Mon Jul 23 17:07:17 2001
+++ uml/arch/um/kernel/time.c	Mon Jul 23 17:08:36 2001
@@ -16,7 +16,7 @@
 #include "user.h"
 #include "process.h"
 
-extern struct timeval xtime;
+extern volatile struct timeval xtime;
 
 void timer_handler(int sig, void *sc, int usermode)
 {


this one for compiling uml with the gcc-3_0-branch of yesterday:

--- 2.4.7aa1/include/asm-um/pgalloc.h.~1~	Mon Jul 23 05:13:13 2001
+++ 2.4.7aa1/include/asm-um/pgalloc.h	Mon Jul 23 05:25:10 2001
@@ -21,7 +21,7 @@
  * Allocate and free page tables.
  */
 
-extern __inline__ pgd_t *get_pgd_slow(void)
+static __inline__ pgd_t *get_pgd_slow(void)
 {
 	pgd_t *pgd = (pgd_t *)__get_free_page(GFP_KERNEL);
 


BTW, Linus the _below_ patches against mainline are needed to compile
the x86 port with gcc-3_0-branch of yesterday, it is safe to include it
in mainline:

--- 2.4.7aa1/include/asm-i386/pgalloc.h.~1~	Mon Jul 23 04:34:24 2001
+++ 2.4.7aa1/include/asm-i386/pgalloc.h	Mon Jul 23 04:50:44 2001
@@ -23,7 +23,7 @@
 extern void *kmalloc(size_t, int);
 extern void kfree(const void *);
 
-extern __inline__ pgd_t *get_pgd_slow(void)
+static __inline__ pgd_t *get_pgd_slow(void)
 {
 	int i;
 	pgd_t *pgd = kmalloc(PTRS_PER_PGD * sizeof(pgd_t), GFP_KERNEL);
@@ -48,7 +48,7 @@
 
 #else
 
-extern __inline__ pgd_t *get_pgd_slow(void)
+static __inline__ pgd_t *get_pgd_slow(void)
 {
 	pgd_t *pgd = (pgd_t *)__get_free_page(GFP_KERNEL);
 
--- 2.4.7aa1/include/asm-i386/siginfo.h.~1~	Mon Jul 23 04:34:24 2001
+++ 2.4.7aa1/include/asm-i386/siginfo.h	Mon Jul 23 04:51:30 2001
@@ -216,7 +216,7 @@
 #ifdef __KERNEL__
 #include <linux/string.h>
 
-extern inline void copy_siginfo(siginfo_t *to, siginfo_t *from)
+static inline void copy_siginfo(siginfo_t *to, siginfo_t *from)
 {
 	if (from->si_code < 0)
 		memcpy(to, from, sizeof(siginfo_t));
--- 2.4.7aa1/net/core/rtnetlink.c.~1~	Mon Feb 28 03:45:10 2000
+++ 2.4.7aa1/net/core/rtnetlink.c	Mon Jul 23 04:52:13 2001
@@ -274,7 +274,7 @@
 
 /* Process one rtnetlink message. */
 
-extern __inline__ int
+static __inline__ int
 rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, int *errp)
 {
 	struct rtnetlink_link *link;

--- 2.4.6pre2aa1/include/linux/sched.h.~1~	Wed Jun 13 00:44:45 2001
+++ 2.4.6pre2aa1/include/linux/sched.h	Wed Jun 13 00:47:23 2001
@@ -541,7 +541,7 @@
 extern unsigned long volatile jiffies;
 extern unsigned long itimer_ticks;
 extern unsigned long itimer_next;
-extern struct timeval xtime;
+extern volatile struct timeval xtime;
 extern void do_timer(struct pt_regs *);
 
 extern unsigned int * prof_buffer;

Andrea

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 05:56:35PM +0200, Andrea Arcangeli wrote:
> BTW, Linus the _below_ patches against mainline are needed to compile
> the x86 port with gcc-3_0-branch of yesterday, it is safe to include it
> in mainline:

here another one for reiserfs:

--- 2.4.7aa1/include/linux/reiserfs_fs.h.~1~	Mon Jul 23 06:56:14 2001
+++ 2.4.7aa1/include/linux/reiserfs_fs.h	Mon Jul 23 17:57:46 2001
@@ -828,7 +828,7 @@
 
 /* compose directory item containing "." and ".." entries (entries are
    not aligned to 4 byte boundary) */
-extern inline void make_empty_dir_item_v1 (char * body, __u32 dirid, __u32 objid,
+static inline void make_empty_dir_item_v1 (char * body, __u32 dirid, __u32 objid,
 					   __u32 par_dirid, __u32 par_objid)
 {
     struct reiserfs_de_head * deh;
@@ -859,7 +859,7 @@
 }
 
 /* compose directory item containing "." and ".." entries */
-extern inline void make_empty_dir_item (char * body, __u32 dirid, __u32 objid,
+static inline void make_empty_dir_item (char * body, __u32 dirid, __u32 objid,
 					__u32 par_dirid, __u32 par_objid)
 {
     struct reiserfs_de_head * deh;

Andrea

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 05:59:07PM +0200, Andrea Arcangeli wrote:
> On Mon, Jul 23, 2001 at 05:56:35PM +0200, Andrea Arcangeli wrote:
> > BTW, Linus the _below_ patches against mainline are needed to compile
> > the x86 port with gcc-3_0-branch of yesterday, it is safe to include it
> > in mainline:
> 
> here another one for reiserfs:

here another one:

--- 2.4.7aa1/net/ipv4/netfilter/ip_queue.c.~1~	Sat Jul 21 00:04:34 2001
+++ 2.4.7aa1/net/ipv4/netfilter/ip_queue.c	Mon Jul 23 18:14:45 2001
@@ -492,7 +492,7 @@
 
 #define RCV_SKB_FAIL(err) do { netlink_ack(skb, nlh, (err)); return; } while (0);
 
-extern __inline__ void netlink_receive_user_skb(struct sk_buff *skb)
+static __inline__ void netlink_receive_user_skb(struct sk_buff *skb)
 {
 	int status, type;
 	struct nlmsghdr *nlh;

Andrea

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 06:17:11PM +0200, Andrea Arcangeli wrote:
> On Mon, Jul 23, 2001 at 05:59:07PM +0200, Andrea Arcangeli wrote:
> > On Mon, Jul 23, 2001 at 05:56:35PM +0200, Andrea Arcangeli wrote:
> > > BTW, Linus the _below_ patches against mainline are needed to compile
> > > the x86 port with gcc-3_0-branch of yesterday, it is safe to include it
> > > in mainline:
> > 
> > here another one for reiserfs:
> 
> here another one:

another one:

--- 2.4.7aa1/drivers/net/arlan.c.~1~	Sat Jul 21 00:04:15 2001
+++ 2.4.7aa1/drivers/net/arlan.c	Mon Jul 23 18:46:35 2001
@@ -1435,7 +1435,7 @@
 	ARLAN_DEBUG_EXIT("arlan_queue_retransmit");
 };
 
-extern inline void RetryOrFail(struct net_device *dev)
+static inline void RetryOrFail(struct net_device *dev)
 {
 	struct arlan_private *priv = ((struct arlan_private *) dev->priv);
 

Andrea

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
>
> in my tree I did some further cleanup, here the ones that you can
> interested about:

Andrea, please drop the "volatile" from xtime. It's bogus.

There's no reason why gcc couldn't make one or many accesses to that
variable, and volatile is an evil keyword that is badly specified and only
makes the compiler generate worse code without ever fixing any real bugs.

If you need a stable value, use a lock.

		Linus

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 09:33:28AM -0700, Linus Torvalds wrote:
> 
> On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
> >
> > in my tree I did some further cleanup, here the ones that you can
> > interested about:
> 
> Andrea, please drop the "volatile" from xtime. It's bogus.

it's the other way around, it's needed and gcc trapped a kernel bug.

If the contents of memory not declared volatile changes under GCC (like
it can happen right now for xtime since it's declared non volatile), gcc
has the full rights to crash the kernel at runtime.

I know there are other bugs like this one in the kernel, but this is not
a good reason to fix the known ones IMHO.

Andrea

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
>
> it's the other way around, it's needed and gcc trapped a kernel bug.

No it's not.

> If the contents of memory not declared volatile changes under GCC (like
> it can happen right now for xtime since it's declared non volatile), gcc
> has the full rights to crash the kernel at runtime.

Absolutely not.

If we care abotu the thing always having the same value, we HAVE to use a
lock. "volatile" is not the answer.

Show me a place where we care.

		Linus

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 10:32:32AM -0700, Linus Torvalds wrote:
> 
> On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
> >
> > it's the other way around, it's needed and gcc trapped a kernel bug.
> 
> No it's not.
> 
> > If the contents of memory not declared volatile changes under GCC (like
> > it can happen right now for xtime since it's declared non volatile), gcc
> > has the full rights to crash the kernel at runtime.
> 
> Absolutely not.
> 
> If we care abotu the thing always having the same value, we HAVE to use a
> lock. "volatile" is not the answer.
> 
> Show me a place where we care.

The problem is not at the source code level, of course all places where
we read the xtime cannot crash the kernel as far as the kernel is
concerned, but the problem is instead at the gcc level: and when the
logic implemented by the asm generated by gcc chokes we can also get a
dangling poitner and crash in the kernel.

GCC internally is allowed to generate code that relies on the contents
of the memory to stay constant, this because of the C standard, the
usual example is a fast path jump table for the "case" statement.

So in short having non volatile memory that changes under gcc gives gcc
the full rights to crash the kernel at runtime anytime.

Andrea

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
>
> GCC internally is allowed to generate code that relies on the contents
> of the memory to stay constant, this because of the C standard, the
> usual example is a fast path jump table for the "case" statement.

Bah.

If we have a case where we _really_ care about the value being stable, I
_still_ claim that that is the one that we need to protect. Not waving
some magic wand over the thing, and saying that "volatile" makes an
unstable value ok.

"volatile" (in the data sense, not the "asm volatile" sense) is almost
always a sign of a bug. If you have an algorithm that depends on the
(compiler-specifi) behaviour of "volatile", you need to change the
algorithm, not try to hide the fact that you have a bug.

Now, the change of algorithm might be something like

	/*
	 * We need to get _one_ value here, because our
	 * state machine ....
	 */
	unsigned long stable = *(volatile unsigned long *)ptr;

	switch (stable) {
	....
	}

where the volatile is in the place where we care, and the volatile is
commented on WHY it actually is the correct thing to do.

The C standard doesn't say crap about volatile. It leaves it up to the
compiler to do something about it.

This is _doubly_ true of multi-word data structures like "xtime". The
meaning of "volatile" on the data structure is so unclear that it's not
even funny.

		Linus

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 11:11:25AM -0700, Linus Torvalds wrote:
> Now, the change of algorithm might be something like
> 
> 	/*
> 	 * We need to get _one_ value here, because our
> 	 * state machine ....
> 	 */

gcc can assume 'state' stays constant in memory not just during the
'case'.

> The C standard doesn't say crap about volatile. It leaves it up to the
> compiler to do something about it.

The C folks definitely say it is a kernel bug if you don't apply my
patch and I agree with them.  Ask Jan.

Andrea

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
>
> gcc can assume 'state' stays constant in memory not just during the
> 'case'.

The point is that if the kernel has _any_ algorithm where it cares, it's a
kernel bug. With volatile or without.

SHOW ME THE CASE WHERE IT CARES. Let's fix it. Let's not just hide it with
"volatile".

		Linus

Re: user-mode port 0.44-2.4.7

[Jonathan Lundell]

At 1:00 PM -0700 2001-07-23, Linus Torvalds wrote:
>On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
>>
>>  gcc can assume 'state' stays constant in memory not just during the
>>  'case'.
>
>The point is that if the kernel has _any_ algorithm where it cares, it's a
>kernel bug. With volatile or without.
>
>SHOW ME THE CASE WHERE IT CARES. Let's fix it. Let's not just hide it with
>"volatile".

in arch/i386/kernel/io_apic.c:

>static int __init timer_irq_works(void)
>{
>	unsigned int t1 = jiffies;
>
>	sti();
>	/* Let ten ticks pass... */
>	mdelay((10 * 1000) / HZ);
>
>	/*
>	* Expect a few ticks at least, to be sure some possible
>	* glue logic does not lock up after one or two first
>	* ticks in a non-ExtINT mode.  Also the local APIC
>	* might have cached one ExtINT interrupt.  Finally, at
>	* least one tick may be lost due to delays.
>	*/
>	if (jiffies - t1 > 4)
>		return 1;
>
>	return 0;
>}

If jiffies were not volatile, this initializing assignment and the 
test at the end could be optimized away, leaving an unconditional 
"return 0". A lock is of no help.
-- 
/Jonathan Lundell.

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Mon, 23 Jul 2001, Jonathan Lundell wrote:
>
> If jiffies were not volatile, this initializing assignment and the
> test at the end could be optimized away, leaving an unconditional
> "return 0". A lock is of no help.

Right.

We want optimization barriers, and there's an explicit "barrier()"  thing
for Linux exactly for this reason.

For historical reasons "jiffies" is actually marked volatile, but at least
it has reasonably good semantics with a single-data item. Which is not to
say I like it. But grep for "barrier()" to see how many times we make this
explicit in the algorithms.

And really, THAT is my whole point. Notice in the previous mail how I used
"volatile" when it was part of the _algorithm_. You should not hide
algorithmic choices in your data structures. You should make them
explicit, so that when you read the code you _see_ what assumptions people
make.

For example, if you fix the code by adding an explicit barrier(), people
see that (a) you're aware of the fact that you expect the values to change
and (b) they see that it is taken care of.

In contrast, if your data structure is marked volatile, how is anybody
reading the code every going to be sure that the code is correct? You have
to look at the header file defining all the data structures. That kind of
thing is NOT GOOD.

So make the algorithm be correct. Then you will notice that there is
_never_ any reason (except for being lazy with tons of existing code) to
add "volatile" to data structures.

Ponder. Understand.

		Linus

Re: user-mode port 0.44-2.4.7

[Jonathan Lundell]

At 3:51 PM -0700 2001-07-23, Linus Torvalds wrote:
>On Mon, 23 Jul 2001, Jonathan Lundell wrote:
>>
>>  If jiffies were not volatile, this initializing assignment and the
>>  test at the end could be optimized away, leaving an unconditional
>>  "return 0". A lock is of no help.
>
>Right.
>
>We want optimization barriers, and there's an explicit "barrier()"  thing
>for Linux exactly for this reason.
>
>For historical reasons "jiffies" is actually marked volatile, but at least
>it has reasonably good semantics with a single-data item. Which is not to
>say I like it. But grep for "barrier()" to see how many times we make this
>explicit in the algorithms.
>
>And really, THAT is my whole point. Notice in the previous mail how I used
>"volatile" when it was part of the _algorithm_. You should not hide
>algorithmic choices in your data structures. You should make them
>explicit, so that when you read the code you _see_ what assumptions people
>make.

OK, sure, that's fine. Better than barrier() in some respects, too. 
Namely, 1) volatile is portable C; barrier() isn't (not that that's 
much of an issue for compiling Linux), and 2) volatile can be 
specific to a variable, unlike the indiscriminate barrier(), which 
forces a reload of everything that might be cached (OK, not a big 
deal for IA32, but nontrivial for many-register architectures). One 
could imagine a more specific barrier(jiffies) syntax, I suppose, but 
the volatile cast is nice, restricting the effect not only to the 
single variable but to the single reference to a single variable.

>For example, if you fix the code by adding an explicit barrier(), people
>see that (a) you're aware of the fact that you expect the values to change
>and (b) they see that it is taken care of.
>
>In contrast, if your data structure is marked volatile, how is anybody
>reading the code every going to be sure that the code is correct? You have
>to look at the header file defining all the data structures. That kind of
>thing is NOT GOOD.

-- 
/Jonathan Lundell.

Re: user-mode port 0.44-2.4.7

[Davide Libenzi]

On 24-Jul-2001 Jonathan Lundell wrote:
> At 3:51 PM -0700 2001-07-23, Linus Torvalds wrote:
>>On Mon, 23 Jul 2001, Jonathan Lundell wrote:
>>>
>>>  If jiffies were not volatile, this initializing assignment and the
>>>  test at the end could be optimized away, leaving an unconditional
>>>  "return 0". A lock is of no help.
>>
>>Right.
>>
>>We want optimization barriers, and there's an explicit "barrier()"  thing
>>for Linux exactly for this reason.
>>
>>For historical reasons "jiffies" is actually marked volatile, but at least
>>it has reasonably good semantics with a single-data item. Which is not to
>>say I like it. But grep for "barrier()" to see how many times we make this
>>explicit in the algorithms.
>>
>>And really, THAT is my whole point. Notice in the previous mail how I used
>>"volatile" when it was part of the _algorithm_. You should not hide
>>algorithmic choices in your data structures. You should make them
>>explicit, so that when you read the code you _see_ what assumptions people
>>make.
> 
> OK, sure, that's fine. Better than barrier() in some respects, too. 
> Namely, 1) volatile is portable C; barrier() isn't (not that that's 
> much of an issue for compiling Linux), and 2) volatile can be 
> specific to a variable, unlike the indiscriminate barrier(), which 
> forces a reload of everything that might be cached (OK, not a big 
> deal for IA32, but nontrivial for many-register architectures). One 
> could imagine a more specific barrier(jiffies) syntax, I suppose, but 
> the volatile cast is nice, restricting the effect not only to the 
> single variable but to the single reference to a single variable.

One more thing, with volatile you specify it one time ( declaration time ),
while with barrier() you've to spread inside the code tons of such macro
everywhere you touch the variable.



- Davide

Re: user-mode port 0.44-2.4.7

[Alexander Viro]

On Tue, 24 Jul 2001, Davide Libenzi wrote:

> One more thing, with volatile you specify it one time ( declaration time ),
> while with barrier() you've to spread inside the code tons of such macro
> everywhere you touch the variable.

That's the whole point, damnit. Syntax (or semantics) sugar is a Bad Thing(tm).
If your algorithm depends on something in a nontrivial way - _spell_ _it_ _out_.

Re: user-mode port 0.44-2.4.7

[Davide Libenzi]

On 24-Jul-2001 Alexander Viro wrote:
> 
> 
> On Tue, 24 Jul 2001, Davide Libenzi wrote:
> 
>> One more thing, with volatile you specify it one time ( declaration time ),
>> while with barrier() you've to spread inside the code tons of such macro
>> everywhere you touch the variable.
> 
> That's the whole point, damnit. Syntax (or semantics) sugar is a Bad Thing(tm).
> If your algorithm depends on something in a nontrivial way - _spell_ _it_ _out_.

I would not call, to pretend the compiler to issue memory loads every time it access
a variable, a nontrivial way.
It sounds pretty clear to me.




- Davide

Re: user-mode port 0.44-2.4.7

[Alexander Viro]

On Tue, 24 Jul 2001, Davide Libenzi wrote:

> I would not call, to pretend the compiler to issue memory loads every time it access
> a variable, a nontrivial way.
> It sounds pretty clear to me.

You know, one of the nice things about C is that unless you abuse
preprocessor, reading code doesn't require doing far lookups. Most
of it can be read and understood with very little context. "Do it
once when you declare a variable" goes against that and that's
not a good thing.

Re: user-mode port 0.44-2.4.7

[Davide Libenzi]

On 24-Jul-2001 Alexander Viro wrote:
> 
> 
> On Tue, 24 Jul 2001, Davide Libenzi wrote:
> 
>> I would not call, to pretend the compiler to issue memory loads every time it access
>> a variable, a nontrivial way.
>> It sounds pretty clear to me.
> 
> You know, one of the nice things about C is that unless you abuse
> preprocessor, reading code doesn't require doing far lookups. Most
> of it can be read and understood with very little context. "Do it
> once when you declare a variable" goes against that and that's
> not a good thing.

Look, you're not going to request any kind of black magic over that variable.
You're simply telling the compiler the way it has to ( not ) optimize the code.
This is IMHO a declaration time issue.
Looking at this code :

while (jiffies < ...) {
        ...
}

the "natural" behaviour that a reader expects is that the "content" of the memory
pointed by  jiffied  is loaded and compared.
That content, not the content of a register loaded 100 asm instructions before the load.
If you like this code more :

for (;;) {
        barrier();
        if (jiffies >= ...)
                break;
        ...
}

It's clear that a declaration like :

__locked_access__ struct pio {
        int a, b, c;
};


for (;;) {
        ++a;
        if (a > b && c < a)
        ...
}

sounds a "Bad Thing"(tm) even to me.




- Davide

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Tue, 24 Jul 2001, Davide Libenzi wrote:
>
> Look, you're not going to request any kind of black magic over that variable.
> You're simply telling the compiler the way it has to ( not ) optimize the code.

Ehh.

But it shouldn't optimize it that way _every_ time. You only want the
specific optimizations in specific places. Which is why you use
"barrier()" or volatile in the _code_, not the data declaration.

For example, if you're holding a lock that protects it or you otherwise
know that nothing is touching it at the same time, you do NOT want to have
the compiler generate bad code.

And trust me, "volatile" generates _bad_ code a lot more often than it
generates correct code.

Look at this:

	volatile int i;
	int j;

	int main()
	{
	        i++;
	        j++;
	}

turning into this:

	main:
	        movl i,%eax
	        incl %eax
	        movl %eax,i
	        incl j
	        ret

Now, ask yourself why? The two _should_ be the same. Both do a
read-modify-write cycle. But the fact is, that when you add "volatile" to
the register, it really tells gcc "Be afraid.  Be very afraid. This user
expects some random behaviour that is not actually covered by any
standard, so just don't ever use this variable for any optimizations, even
if they are obviously correct. That way he can't complain".

See? "volatile" is evil. It has _no_ standard semantics, which makes it
really hard to implement sanely for the compiler. It also means that the
compiler can change the semantics of what "volatile" means, without you
really being able to complain.

Also note how the "incl j" instruction is actually _better_ from a
"atomicity" standpoint than the "load+inc+store" instruction. In this
case, adding a "volatile" actually made the accesses to "i" be _less_
likely to be correct - you could have had an interrupt happen in between
that also updated "i", and got lost when we wrote the value back.

Moral of the story: don't use volatile. If you want to have a counter that
is updated in interrupts etc, use "atomic_t" or locking. "volatile" makes
things worse or better based on completely random criteria that don't
necessarily have anything to do with what you _want_ to do.

			Linus

Re: user-mode port 0.44-2.4.7

[Davide Libenzi]

On 24-Jul-2001 Linus Torvalds wrote:
> But it shouldn't optimize it that way _every_ time. You only want the
> specific optimizations in specific places. Which is why you use
> "barrier()" or volatile in the _code_, not the data declaration.
> 
> For example, if you're holding a lock that protects it or you otherwise
> know that nothing is touching it at the same time, you do NOT want to have
> the compiler generate bad code.
> 
> And trust me, "volatile" generates _bad_ code a lot more often than it
> generates correct code.
> 
> Look at this:
> 
>       volatile int i;
>       int j;
> 
>       int main()
>       {
>               i++;
>               j++;
>       }
> 
> turning into this:
> 
>       main:
>               movl i,%eax
>               incl %eax
>               movl %eax,i
>               incl j
>               ret
> 
> Now, ask yourself why? The two _should_ be the same. Both do a
> read-modify-write cycle. But the fact is, that when you add "volatile" to
> the register, it really tells gcc "Be afraid.  Be very afraid. This user
> expects some random behaviour that is not actually covered by any
> standard, so just don't ever use this variable for any optimizations, even
> if they are obviously correct. That way he can't complain".

This is a too simple case, this is maybe better :

        mov homer, %edx
        ...
        ...
        ...
        ... ( 101 asm ins )
loop:
        cmp %edx, ...
        ja out
        ...
        inc %edx
        ...
        jmp loop

You're right, it might be optimized with a barrier() but it's all kind of how
much times you're going to need one behaviour or the other.
When I'll need most of my access to be "strict" I'd like to have a way that avoid
me to spread the code with barries()s.


> Also note how the "incl j" instruction is actually _better_ from a
> "atomicity" standpoint than the "load+inc+store" instruction. In this
> case, adding a "volatile" actually made the accesses to "i" be _less_
> likely to be correct - you could have had an interrupt happen in between
> that also updated "i", and got lost when we wrote the value back.

Not that much if you look at how incl is "decomposed" internally ( w/o LOCK )
by the CPU. If you really care about  j  you need an atomic op here, in any case.





- Davide

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Tue, 24 Jul 2001, Davide Libenzi wrote:
>
> Not that much if you look at how incl is "decomposed" internally ( w/o LOCK )
> by the CPU. If you really care about  j  you need an atomic op here, in any case.

Yes, but the "inc" is atomic at least on a UP system. So here "volatile"
might actually _show_ bugs instead of hiding them.

The real isssue, though, is that that is all volatile ever does. It can
show or hide bugs, but it can't fix them.

Of course, some people consider hidden bugs to _be_ fixed. I don't believe
in that particulat philosophy myself.

		Linus

Re: user-mode port 0.44-2.4.7

[Anton Altaparmakov]

At 17:52 24/07/2001, Davide Libenzi wrote:
>On 24-Jul-2001 Alexander Viro wrote:
> > On Tue, 24 Jul 2001, Davide Libenzi wrote:
>You're simply telling the compiler the way it has to ( not ) optimize the 
>code.
>This is IMHO a declaration time issue.
>Looking at this code :
>
>while (jiffies < ...) {
>         ...
>}
>
>the "natural" behaviour that a reader expects is that the "content" of the 
>memory pointed by  jiffied  is loaded and compared.

Well, that depends on your definition of "natural". In my definition, it 
would be absolutely normal in this example for the compiler to cache 
jiffies because it considers it as a non-changing variable if none of the 
code inside the while loop refers to jiffies again. But that's just me...

>If you like this code more :
>
>for (;;) {
>         barrier();
>         if (jiffies >= ...)
>                 break;
>         ...
>}

Er, what is wrong with:

while (barrier(), jiffies < ...) {
         ...
}

It is just as clean as the starting point but tells both the compiler at 
compile time and _me_ when reading the code that jiffies is expected to 
change under me.

That is _way_ better than declaring it volatile in some obsure header file 
which, chances are, I have never looked at, or looked at and long forgotten 
about...

Just my 2p.

Anton


-- 
   "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/

Re: user-mode port 0.44-2.4.7

[Chris Friesen]

Linus Torvalds wrote:
> 
> On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
> >
> > gcc can assume 'state' stays constant in memory not just during the
> > 'case'.
> 
> The point is that if the kernel has _any_ algorithm where it cares, it's a
> kernel bug. With volatile or without.
> 
> SHOW ME THE CASE WHERE IT CARES. Let's fix it. Let's not just hide it with
> "volatile".

If I understand correctly, xtime is updated asynchronously.  If it isn't, then
ignore this message totally.  However, if it is, then *not* specifying it as
volatile could easily cause problems in technically correct but poorly written
code.

Suppose I loop against xtime reaching a particular value.  While this is
definately not good practice, if xtime is not specified as volatile then since I
never modify it within the loop the compiler is free to move the initial load
out of the loop when optimizing.  In this example the case where it is marked as
volatile will run (though inefficiently), but the non-volatile case can hang
totally.

Do we want to get ourselves into something like this?

Chris

-- 
Chris Friesen                    | MailStop: 043/33/F10  
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 04:44:06PM -0400, Chris Friesen wrote:
> Linus Torvalds wrote:
> > 
> > On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
> > >
> > > gcc can assume 'state' stays constant in memory not just during the
> > > 'case'.
> > 
> > The point is that if the kernel has _any_ algorithm where it cares, it's a
> > kernel bug. With volatile or without.
> > 
> > SHOW ME THE CASE WHERE IT CARES. Let's fix it. Let's not just hide it with
> > "volatile".
> 
> If I understand correctly, xtime is updated asynchronously.  If it isn't, then
> ignore this message totally.  However, if it is, then *not* specifying it as
> volatile could easily cause problems in technically correct but poorly written
> code.
> 
> Suppose I loop against xtime reaching a particular value.  While this is
> definately not good practice, if xtime is not specified as volatile then since I
> never modify it within the loop the compiler is free to move the initial load
> out of the loop when optimizing.  In this example the case where it is marked as
> volatile will run (though inefficiently), but the non-volatile case can hang
> totally.
> 
> Do we want to get ourselves into something like this?

This is actually another issue, not really the problem I was talking
about. The problem I was talking about was just about the C language and
about writing correct C code (whith correct C code I mean something that
cannot break by using a future release of gcc). Without my patch you
never know. About the loop problem you mentioned you can know it won't
break instead if you write it carefully. Of course I understand in most
cases if the code breaks in the actual usages of xtime it is likely that
gcc is doing something stupid in terms of performance. but GCC if it
wants to is allowed to compile this code:

	printf("%lx\n", xtime.tv_sec);

as:

	unsigned long sec = xtime.tv_sec;
	if (sec != xtime.tv_sec)
		BUG();
	printf("%lx\n", sec);

Andrea

Re: user-mode port 0.44-2.4.7

[Richard Gooch]

Andrea Arcangeli writes:
> cases if the code breaks in the actual usages of xtime it is likely that
> gcc is doing something stupid in terms of performance. but GCC if it
> wants to is allowed to compile this code:
> 
> 	printf("%lx\n", xtime.tv_sec);
> 
> as:
> 
> 	unsigned long sec = xtime.tv_sec;
> 	if (sec != xtime.tv_sec)
> 		BUG();
> 	printf("%lx\n", sec);

And if it does that, it's stupid. Why on earth would GCC add extra
code to check if a value hasn't changed? I want it to produce
efficient code. What's next? Wrap checking?
    printk ("You've just wrapped an integer: press [ENTER] to confirm,
	    [NT] to ignore   ");

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 03:50:54PM -0600, Richard Gooch wrote:
> Andrea Arcangeli writes:
> > cases if the code breaks in the actual usages of xtime it is likely that
> > gcc is doing something stupid in terms of performance. but GCC if it
> > wants to is allowed to compile this code:
> > 
> > 	printf("%lx\n", xtime.tv_sec);
> > 
> > as:
> > 
> > 	unsigned long sec = xtime.tv_sec;
> > 	if (sec != xtime.tv_sec)
> > 		BUG();
> > 	printf("%lx\n", sec);
> 
> And if it does that, it's stupid. Why on earth would GCC add extra
> code to check if a value hasn't changed? I want it to produce

GCC will obviously _never_ introduce a BUG(), I never said that, the
above example is only meant to show what GCC is _allowed_ to do and what
we have to do to write correct C code.

The real life case of the BUG() is when gcc optimize `case' with a jump
table the equivalent of BUG() will be you derferencing a dangling
pointer at runtime. Same can happen in other cases but don't ask me the
other cases as I'm not a gcc developer and I've no idea what they plans
to do for other things (ask Honza for those details).

> efficient code. What's next? Wrap checking?
>     printk ("You've just wrapped an integer: press [ENTER] to confirm,
> 	    [NT] to ignore   ");
> 
> 				Regards,
> 
> 					Richard....
> Permanent: rgooch@atnf.csiro.au
> Current:   rgooch@ras.ucalgary.ca


Andrea

Re: user-mode port 0.44-2.4.7

[Rob Landley]

On Monday 23 July 2001 18:09, Andrea Arcangeli wrote:

> GCC will obviously _never_ introduce a BUG(), I never said that, the
> above example is only meant to show what GCC is _allowed_ to do and what
> we have to do to write correct C code.

"Correct" C code as in portable C code?  Standards compliant so it's portable 
to other compilers?

The linux kernel is compiled on gcc.  It always has been.  Specific versions 
of the kernel have a specific range of "known working" gcc versions that in 
the past have produced a functional result.  gcc is -ALLOWED- to compile code 
the way Borland, Watcom, or Microsoft C compilers do.  Assuming somebody 
wants to rewrite gcc from scratch and still call the result gcc...

Standards are fun, but we don't even thread posixly yet...

Rob

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 09:20:32AM -0400, Rob Landley wrote:
> On Monday 23 July 2001 18:09, Andrea Arcangeli wrote:
> 
> > GCC will obviously _never_ introduce a BUG(), I never said that, the
> > above example is only meant to show what GCC is _allowed_ to do and what
> > we have to do to write correct C code.
> 
> "Correct" C code as in portable C code?  Standards compliant so it's portable 

correct C code _mainly_ for gcc which is very aggressive.

Andrea

Re: user-mode port 0.44-2.4.7

[Rob Landley]

On Monday 23 July 2001 18:27, Andrea Arcangeli wrote:
> On Mon, Jul 23, 2001 at 09:20:32AM -0400, Rob Landley wrote:
> > On Monday 23 July 2001 18:09, Andrea Arcangeli wrote:
> > > GCC will obviously _never_ introduce a BUG(), I never said that, the
> > > above example is only meant to show what GCC is _allowed_ to do and
> > > what we have to do to write correct C code.
> >
> > "Correct" C code as in portable C code?  Standards compliant so it's
> > portable
>
> correct C code _mainly_ for gcc which is very aggressive.
>
> Andrea

Very aggressive and very known.

If we know how GCC is going to behave in a given situation (and have a fairly 
good idea of how it's going to behave in the future), we don't have to worry 
about theoretical stardards loopholes like it calling BUG(), do we?

Is there something in the current behavior that's causing trouble (in which 
case, what exactly is it), or are the gcc guys (who you mentioned earlier*) 
warning that the behavior of upcoming versions of gcc is likely to break 
known constructs in the current kernel?

Rob

(Has the english language officially held a funeral for the word "whom" yet?)

Re: user-mode port 0.44-2.4.7

[Richard Gooch]

Andrea Arcangeli writes:
> On Mon, Jul 23, 2001 at 03:50:54PM -0600, Richard Gooch wrote:
> > Andrea Arcangeli writes:
> > > cases if the code breaks in the actual usages of xtime it is likely that
> > > gcc is doing something stupid in terms of performance. but GCC if it
> > > wants to is allowed to compile this code:
> > > 
> > > 	printf("%lx\n", xtime.tv_sec);
> > > 
> > > as:
> > > 
> > > 	unsigned long sec = xtime.tv_sec;
> > > 	if (sec != xtime.tv_sec)
> > > 		BUG();
> > > 	printf("%lx\n", sec);
> > 
> > And if it does that, it's stupid. Why on earth would GCC add extra
> > code to check if a value hasn't changed? I want it to produce
> 
> GCC will obviously _never_ introduce a BUG(), I never said that, the
> above example is only meant to show what GCC is _allowed_ to do and
> what we have to do to write correct C code.

I don't think it should be allowed to do that. That's a whipping
offence. I think that example is a red herring.

> The real life case of the BUG() is when gcc optimize `case' with a
> jump table the equivalent of BUG() will be you derferencing a
> dangling pointer at runtime. Same can happen in other cases but
> don't ask me the other cases as I'm not a gcc developer and I've no
> idea what they plans to do for other things (ask Honza for those
> details).

So grab a local snapshot of the variable, as Linus suggested. In fact,
the switch example is interesting, because one could argue the
opposite way, that declaring the switch variable as "volatile" means
that if GCC needs to internally re-"get" the variable, it should grab
it from memory, and thus definately fail. Without "volatile", GCC is
implicitely allowed to cache the variable (which is of course safe).

So, really, the switch example requires an "antivolatile"
declaration. Oh, wait! I get the same with " ".

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Mon, Jul 23, 2001 at 05:47:04PM -0600, Richard Gooch wrote:
> I don't think it should be allowed to do that. That's a whipping

it is allowed to do that, period. This is not your choice or my choice.
You may ask gcc folks not to do that and I think they just do.

> So grab a local snapshot of the variable, as Linus suggested. In fact,
> the switch example is interesting, because one could argue the
> opposite way, that declaring the switch variable as "volatile" means
> that if GCC needs to internally re-"get" the variable, it should grab
> it from memory, and thus definately fail. Without "volatile", GCC is
> implicitely allowed to cache the variable (which is of course safe).

If gcc caches there's no problem indeed, the problem is when it doesn't
cache it which can happen, with volatile it will understand it must not
make assumption for the variable to not change under it. Anyways as just
said in another email in this thread I'm been told it wasn't just for
'case'. I think tomorrow Honza will comment this since he's the gcc
developer who asked those kernel bugs to be fixed for gcc.

Andrea

Re: user-mode port 0.44-2.4.7

[Jan Hubicka]

> If gcc caches there's no problem indeed, the problem is when it doesn't
> cache it which can happen, with volatile it will understand it must not
> make assumption for the variable to not change under it. Anyways as just
> said in another email in this thread I'm been told it wasn't just for
> 'case'. I think tomorrow Honza will comment this since he's the gcc
> developer who asked those kernel bugs to be fixed for gcc.
What I was concerned about is for example scenario:
1) cse realizes that given variable is constnat in some region of program
   (by seeing an conjump).
2) it replaces occurences of other same constants in the program by that variable
   (as costs says register is cheaper)
3) register allocator runs out of registers
4) reload optimizes and re-loads the variable from original place instead of
   spilling it.

This way you can effectivly change one constant to another for some region
of program.

At the moment gcc rarely exploits knowledge about the variable value, but
this is going to improve, as gcc envolve and we are riscing hard to debug
problems in kernel.

I am not sure volatile is sensible solution tought.

For instance gcc 3.1 now does load store motion that makes gcc to keep
global variables in registers much longer than it did originally making
such risk greater.

Honza
> 
> Andrea

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Tue, 24 Jul 2001, Jan Hubicka wrote:
>
> What I was concerned about is for example scenario:
> 1) cse realizes that given variable is constnat in some region of program
>    (by seeing an conjump).

Note that just about _every_ single global in the whole kernel is
"volatile" in this sense.

In fact, I can't think of a single (interesting) global that cannot be
changed by another thread at any time.

What makes this even more interesting, of course, is that with SMP you
also have various memory ordering models, so many classic algorithms
simply will not work: one classic algorithm is to know about update order
and avoid locking by using optimistic algorithms:

	do {
		x = (volatile) value1;
		y = (volatile) value2;
	} while (x != (volatile) value1);

the above is a completely valid algorithm in a strictly ordered system,
but it simply WILL NOT WORK on SMP.

How do we handle globals in the kernel? We use locking.

It's really that simple. We _always_ use locking to a first approximation.
And the locking code takes care to tell gcc that gcc cannot optimize
memory movements of assume that memory is constant.

This is just a basic fact of life.

There are other valid algorithms, but they have to be secondary if only
because they are a lot harder to think about and validate.

One is to use memory ordering constraints. Note that in this one
"volatile" simply doesn't help. Even with "volatile" maybe (and this is
debatable - gcc does not actually have a very good definition of what
volatile means even for a purely gcc implementation standpoint) forcing
ordering with the compiler, it doesn't force any ordering in the hardware.

In fact, there have been CPU's at least designed that simply will not be
guaranteed to pick up changes by another CPU without some kind of memory
barrier. On such a CPU it is simply not _possible_ to use

	while ((volatile) jiffies < x)
		/* do nothing */;

because if "jiffies" is updated on another CPU, the first CPU will not
necessarily see it AT ALL. Even though the compiler causes the code to
re-load the value constantly.

An example of this kind of machine? Imagine a P4 that didn't have some
backwards compatibility cruft.. And wonder about some of the reasons that
Intel _really_ wants people to start using "rep ; nop" in these kinds of
loops.

Do we potentially have these bugs? Yes. In the case of "jiffies", we'd be
ok on these kinds of machines simply because all places that do this do so
with interrupts enabled (it wouldn't work on UP otherwise), and interrupts
would end up working as memory barriers in any case.

Ok, so memory ordering is one valid algorithm. And clearly "volatile" is
useless for memory ordering.

Another valid algorithm: "we don't care what the value is, old or new".
This is the one most often used for "jiffies". We simply don't care
enough. We know that jiffies can change at any time, and the code had
better not do anything strange about it anyway. And the code had better be
such that even the compiler cannot do anything really strange with it
either.

"xtime" simply isn't this kind of value. Why? Because it's not even
atomic. It's two 32-bit words, and the operations gcc would use on it
aren't things that we could improve with "volatile".

For "xtime", we're ok with (for example) only looking at one of the fields
(usually xtime.sec), and just not caring _what_ the value is, because we
just copy it into "inode->i_mtime" or something.

But if we have an algorithm that really _does_ care, then "volatile"
really doesn't help. See above on memory ordering etc constraints anyway.

> For instance gcc 3.1 now does load store motion that makes gcc to keep
> global variables in registers much longer than it did originally making
> such risk greater.

I bet that we'll find kernel bugs thanks to it. And I look forward to it,
as that is just a sign of gcc doing a better job. And we'll have to make
sure that we add the locks or the memory ordering things that we missed.

Looking back a few years, we used to have lots of code of the type

	while (!SCpnt->ready) {
		.. do some waiting ..
	}

and gcc was simply too stupid to notice that it could hoist the load of
"SCpnt->ready" outside the loop because the loop did nothing.

Then gcc improved, and for a while we had a _lot_ of "barrier()" calls
added. And we've been fine with this ever since.

Maybe we'll need to add some more. Maybe for xtime. Maybe for something we
haven't even thought about. Bugs are nothing new, and nothing to be really
afraid of.

		Linus

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Tue, 24 Jul 2001, Andrea Arcangeli wrote:
> On Mon, Jul 23, 2001 at 05:47:04PM -0600, Richard Gooch wrote:
> > I don't think it should be allowed to do that. That's a whipping
>
> it is allowed to do that, period. This is not your choice or my choice.
> You may ask gcc folks not to do that and I think they just do.

Stop this stupid argument.

A C compiler is "allowed" to do just about anything. The introduction of
"volatile" does not change that in any really meaningful way. It doesn't
change the fact that gcc is "allowed" to do a really shitty job on _any_
code it is given.

>From a pure standards standpoint, gcc can change something like

	int i = *int_ptr;

into the equivalent of (assuming a little-endian machine with only byte
load/store instructions)

	unsigned char *tmp = (unsigned char *)int_ptr + 3;
	int j = 4;
	int i = 0;

	do {
		i <<= 8;
		i += *tmp;
		tmp--;
	} while (--j);

The fact that a C compiler is _allowed_ to create code like just about
anything is not an argument at all.

The above, btw, is NOT as ridiculous as it sounds. We've had the exact
opposite problem on alpha: byte stores are not "atomic", and would
"corrupt" the bytes around it due to the non-atomic nature of having to do

	load word
	mask value
	insert byte
	store word

Did it help to mark things "volatile" there? No. We had to change the code
to (a) either use locking so that nobody would ever touch adjacent bytes
concurrently or (b) stop using byte values.

			Linus

Re: user-mode port 0.44-2.4.7

[Andrea Arcangeli]

On Tue, Jul 24, 2001 at 09:04:28AM -0700, Linus Torvalds wrote:
> 
> On Tue, 24 Jul 2001, Andrea Arcangeli wrote:
> > On Mon, Jul 23, 2001 at 05:47:04PM -0600, Richard Gooch wrote:
> > > I don't think it should be allowed to do that. That's a whipping
> >
> > it is allowed to do that, period. This is not your choice or my choice.
> > You may ask gcc folks not to do that and I think they just do.
> 
> Stop this stupid argument.

I will if Honza assures me that no future version of gcc will cause me to
crash if I don't declare xtime volatile and I play with it while it can
change under me (which seems not the case from his last email).

Of course I know that this is more a theorical thing and that if we
consider that a bug we have also many other bugs of the same kind.

Honza? Do you assure me that? In case you don't, could you suggest
another way besides volatile and spinlocks around the access to the
variable to avoid gcc to get confused?

Andrea

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Thu, 26 Jul 2001, Andrea Arcangeli wrote:
>
> I will if Honza assures me that no future version of gcc will cause me to
> crash if I don't declare xtime volatile and I play with it while it can
> change under me (which seems not the case from his last email).

WHY DO YOU NOT ADD THE "VOLATILE" TO THE PLACES THAT _CARE_?

This is not a gcc issue. Even if gcc _were_ to generate bad code, the
global volatile _still_ wouldn't be the correct answer.

		Linus

Re: user-mode port 0.44-2.4.7

[Chris Friesen]

Linus Torvalds wrote:
> 
> On Thu, 26 Jul 2001, Andrea Arcangeli wrote:
> >
> > I will if Honza assures me that no future version of gcc will cause me to
> > crash if I don't declare xtime volatile and I play with it while it can
> > change under me (which seems not the case from his last email).
> 
> WHY DO YOU NOT ADD THE "VOLATILE" TO THE PLACES THAT _CARE_?
> 
> This is not a gcc issue. Even if gcc _were_ to generate bad code, the
> global volatile _still_ wouldn't be the correct answer.

I think his worry is the pedantic reason that without the volatile gcc is
allowed to write code that chokes and dies if xtime happens to change in a
volatile manner.  The example given earlier was:

code as written:
	kprintf("%d\n",xtime.tv_usec);

code as compiled by gcc (with xtime not specified as volatile):
	suseconds_t temp = xtime.tv_usec;
	if (temp != xtime.tv_usec)
		BUG();
	kprintf"%d\n",temp);

I hope that gcc would not do such a thing, but I think a case could be made that
it could do it and still comply with the language standard.

Of course, since the linux kernel is an important thing for gcc to compile, I
can't imagine them doing something that would break it on purpose without a
really good reason.

Chris


-- 
Chris Friesen                    | MailStop: 043/33/F10  
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: user-mode port 0.44-2.4.7

[Jan Hubicka]

> Honza? Do you assure me that? In case you don't, could you suggest
> another way besides volatile and spinlocks around the access to the
> variable to avoid gcc to get confused?
Looking at the code, it really looks as perfect candidate for volatile.
GCC has definitly right to assume that the memory location won't change
and use it for optimization.  On the other hand, from usual usage of
time I guess gcc won't be able to do so, at least not today.

Basically most optimizations comes from that compiler recognizes that
value is equivalent to something (constant) at given code path and
promotes it futher.

So it is probably up to kernel folks if you want to follow C standard
and blame gcc developers for breaking it, or stay in the unsafe ground
and fix kernel each time gcc will introduce new nasty optimizations.
This may become tricky later - for instance as making kernel codebase
-fstrict-aliasing ready.

Honza

Re: user-mode port 0.44-2.4.7

[Alan Cox]

> and blame gcc developers for breaking it, or stay in the unsafe ground
> and fix kernel each time gcc will introduce new nasty optimizations.
> This may become tricky later - for instance as making kernel codebase
> -fstrict-aliasing ready.

Until the C language improves its ability to describe aliasing safety I
don't think its going to be practical to fix the issue. I've seen patches to
make slhc.c strict aliasing safe for example, and they are too ugly to live

Re: user-mode port 0.44-2.4.7

[Linus Torvalds]

On Mon, 23 Jul 2001, Chris Friesen wrote:
>
> If I understand correctly, xtime is updated asynchronously.  If it isn't, then
> ignore this message totally.  However, if it is, then *not* specifying it as
> volatile could easily cause problems in technically correct but poorly written
> code.

Yes.

Adding "volatile" often helps poorly written code.

In fact, the one AND ONLY reason to add volatile to data structures is in
fact poorly written code.

Now, think about that for a minute, and maybe you'll understand why I
don't want more volatiles in the kernel.

		Linus

PS. This has come up before. The old pre-Alan networking code had
"volatile" on just about every single network data structure. Every damn
single one of them was a bug. Without exception.

Re: user-mode port 0.44-2.4.7

[Alan Cox]

> PS. This has come up before. The old pre-Alan networking code had
> "volatile" on just about every single network data structure. Every damn
> single one of them was a bug. Without exception.

With due respect to Fred and Ross most of them were there because the
gcc 2.4.5 compiler was buggy.

Nowdays we still have some volatile users - notably jiffies, and one or two
of them make sense, but not many

Alan

Re: user-mode port 0.44-2.4.7

[Jeff Dike]

andrea@suse.de said:
> in my tree I did some further cleanup, here the ones that you can
> interested about: 

Thanks, it's in my queue.

Although I think I'll hold off on the xtime change until you and Linus have 
decided whether it needs volatile or locking :-)

				Jeff

Re: user-mode port 0.44-2.4.7

[Alan Cox]

> Suppose I loop against xtime reaching a particular value.  While this is

xtime isnt used this way that I can see. jiffies however is. There are good
arguments for getting rid of most [ab]use of jiffies however. For one its
pretty important to scaling on both big mainframes and beowulf setups doing
heavy computation to reduce timer ticks

Re: user-mode port 0.44-2.4.7

[Chris Friesen]

Alan Cox wrote:
> 
> > Suppose I loop against xtime reaching a particular value.  While this is
> 
> xtime isnt used this way that I can see. jiffies however is. There are good
> arguments for getting rid of most [ab]use of jiffies however. For one its
> pretty important to scaling on both big mainframes and beowulf setups doing
> heavy computation to reduce timer ticks

jiffies is (as of 2.4.7 anyways) marked as volatile, so we're safe there.  My
point is this--should someone writing badly designed (but technically correct)
code be able to totally hose the system?

The only difference between volatile and normal is that if it is marked as
volatile it must be accessed every time rather than being pre-cached.  If we
never spin on accessing xtime, then the fact that we can't optimize it shouldn't
hurt. (Am I wrong here?  If I am then please explain because I'm missing
something...)  If someone ever *does* spin on xtime, then we really don't want
to optimize that access out of the loop, because doing so could cause nasty
problems.


-- 
Chris Friesen                    | MailStop: 043/33/F10  
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: user-mode port 0.44-2.4.7

[Alan Cox]

> Should head and tail be volatile in the definition, or should they be
> accessed with:
> int head = (volatile)myqueue.head;
> or with barrier() around the read/write?

The best way is to use barrier calls. It makes your assumptions about
ordering absolutely explicit. However you should still be careful - you
can't be sure that head will be read atomically or written atomically on
all processors eg if it was

	struct
	{
		unsigned char head;
		unsigned char tail;
		char buf[256];
	}

you would get some suprisingly unpleasant suprises on SMP Alpha. Currently
"int" is probably safe for all processors.

So unless this is a precision tuned fast path it is better to play safe with
this and use atomic_t or locking. The spinlock cost on an Athlon or a later
PIII is pretty good in most cases. Using the -ac prefetch stuff can make it
good in almost all cases, but thats probably a 2.5 thing for the generic
case.

Basically locks are getting cheaper on x86, the suprises are getting more
interesting on non-x86

Alan

Re: user-mode port 0.44-2.4.7

[Alan Cox]

> > This is not a gcc issue. Even if gcc _were_ to generate bad code, the
> > global volatile _still_ wouldn't be the correct answer.
> 
> I think his worry is the pedantic reason that without the volatile gcc is
> allowed to write code that chokes and dies if xtime happens to change in a
> volatile manner.  The example given earlier was:

Make the volatility explicit where it is needed, either to express a barrier
with barrier() or an assignment as in

	foo = (volatile)xtime

This makes it clear where the barriers are and avoids unpleasant
optimisation hits elsewhere.

Alan

RE: user-mode port 0.44-2.4.7

[James W. Lake]

What about a case when you have a struct
struct queue
{
	int head;
	int tail;
	...
} myqueue;
You update head in an isr and tail in one singular open fop_read() call.
Either don't mind getting old values of the one they don't update, want
to get the latest version of the variable as often as possible.
Should head and tail be volatile in the definition, or should they be
accessed with:
int head = (volatile)myqueue.head;
or with barrier() around the read/write?
Jim Lake