2005-04-05 16:59:49

by Greg KH

[permalink] [raw]
Subject: [00/11] -stable review

This is the start of the stable review cycle for the 2.6.11.7 release.
There are 8 patches in this series, all will be posted as a response to
this one. If anyone has any issues with these being applied, please let
us know. If anyone is a maintainer of the proper subsystem, and wants
to add a Signed-off-by: line to the patch, please respond with it.

These patches are sent out with a number of different people on the Bcc:
line. If you wish to be a reviewer, please email [email protected] to
add your name to the list. If you want to be off the reviewer list,
also email us.

Responses should be made by Thursday, April 7, 17:00 UTC. Anything
received after that time, might be too late.

thanks,

the -stable release team


2005-04-05 16:49:03

by Greg KH

[permalink] [raw]
Subject: [07/08] [TCP] Fix BIC congestion avoidance algorithm error


-stable review patch. If anyone has any objections, please let us know.

------------------

Since BIC is the default congestion control algorithm
enabled in every 2.6.x kernel out there, fixing errors
in it becomes quite critical.

A flaw in the loss handling caused it to not perform
the binary search regimen of the BIC algorithm
properly.

The fix below from Stephen Hemminger has been heavily
verified.

[TCP]: BIC not binary searching correctly

While redoing BIC for the split up version, I discovered that the existing
2.6.11 code doesn't really do binary search. It ends up being just a slightly
modified version of Reno. See attached graphs to see the effect over simulated
1mbit environment.

The problem is that BIC is supposed to reset the cwnd to the last loss value
rather than ssthresh when loss is detected. The correct code (from the BIC
TCP code for Web100) is in this patch.

Signed-off-by: Stephen Hemminger <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

--- 1.92/net/ipv4/tcp_input.c 2005-02-22 10:45:31 -08:00
+++ edited/net/ipv4/tcp_input.c 2005-03-23 10:55:18 -08:00
@@ -1653,7 +1653,10 @@
static void tcp_undo_cwr(struct tcp_sock *tp, int undo)
{
if (tp->prior_ssthresh) {
- tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh<<1);
+ if (tcp_is_bic(tp))
+ tp->snd_cwnd = max(tp->snd_cwnd, tp->bictcp.last_max_cwnd);
+ else
+ tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh<<1);

if (undo && tp->prior_ssthresh > tp->snd_ssthresh) {
tp->snd_ssthresh = tp->prior_ssthresh;

2005-04-05 16:51:53

by Greg KH

[permalink] [raw]
Subject: [05/08] [IPSEC]: Do not hold state lock while checking size

-stable review patch. If anyone has any objections, please let us know.

------------------

This patch from Herbert Xu fixes a deadlock with IPsec.
When an ICMP frag. required is sent and the ICMP message
needs the same SA as the packet that caused it the state
will be locked twice.

[IPSEC]: Do not hold state lock while checking size.

This can elicit ICMP message output and thus result in a
deadlock.

Signed-off-by: Herbert Xu <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

diff -Nru a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
--- a/net/ipv4/xfrm4_output.c 2005-03-20 16:53:05 +01:00
+++ b/net/ipv4/xfrm4_output.c 2005-03-20 16:53:05 +01:00
@@ -103,16 +103,16 @@
goto error_nolock;
}

- spin_lock_bh(&x->lock);
- err = xfrm_state_check(x, skb);
- if (err)
- goto error;
-
if (x->props.mode) {
err = xfrm4_tunnel_check_size(skb);
if (err)
- goto error;
+ goto error_nolock;
}
+
+ spin_lock_bh(&x->lock);
+ err = xfrm_state_check(x, skb);
+ if (err)
+ goto error;

xfrm4_encap(skb);

diff -Nru a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
--- a/net/ipv6/xfrm6_output.c 2005-03-20 16:53:05 +01:00
+++ b/net/ipv6/xfrm6_output.c 2005-03-20 16:53:05 +01:00
@@ -103,16 +103,16 @@
goto error_nolock;
}

- spin_lock_bh(&x->lock);
- err = xfrm_state_check(x, skb);
- if (err)
- goto error;
-
if (x->props.mode) {
err = xfrm6_tunnel_check_size(skb);
if (err)
- goto error;
+ goto error_nolock;
}
+
+ spin_lock_bh(&x->lock);
+ err = xfrm_state_check(x, skb);
+ if (err)
+ goto error;

xfrm6_encap(skb);

2005-04-05 16:51:54

by Greg KH

[permalink] [raw]
Subject: [04/08] I2C: Fix oops in eeprom driver

-stable review patch. If anyone has any objections, please let us know.

------------------

This fixes an oops in the eeprom driver. It was first reported here:
http://bugzilla.kernel.org/show_bug.cgi?id=4347

It was additionally discussed here (while tracking a completely
different bug):
http://archives.andrew.net.au/lm-sensors/msg30021.html

The patch is already in 2.6.12-rc1:
http://linux.bkbits.net:8080/linux-2.5/[email protected]

The oops happens when one reads data from the sysfs interface file such
that (off < 16) and (count < 16 - off). For example "sensors" from
lm_sensors 2.9.0 does this, and causes the oops.

Signed-off-by: Jean Delvare <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

--- linux-2.6.11.4/drivers/i2c/chips/eeprom.c.orig 2005-03-13 10:00:01.000000000 +0100
+++ linux-2.6.11.4/drivers/i2c/chips/eeprom.c 2005-03-17 19:54:07.000000000 +0100
@@ -130,7 +130,8 @@

/* Hide Vaio security settings to regular users (16 first bytes) */
if (data->nature == VAIO && off < 16 && !capable(CAP_SYS_ADMIN)) {
- int in_row1 = 16 - off;
+ size_t in_row1 = 16 - off;
+ in_row1 = min(in_row1, count);
memset(buf, 0, in_row1);
if (count - in_row1 > 0)
memcpy(buf + in_row1, &data->data[16], count - in_row1);


2005-04-05 16:52:19

by Greg KH

[permalink] [raw]
Subject: [01/08] Fix Oops with ALSA timer event notification

-stable review patch. If anyone has any objections, please let us know.

------------------

the patch below fixes the bug of ALSA timer notification, which is
used in the recent ALSA dmix plugin.

- fixed Oops in read()
- fixed wake-up polls and signals with new events

Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

--- linux/sound/core/timer.c 20 Jan 2005 17:37:00 -0000 1.50
+++ linux/sound/core/timer.c 14 Mar 2005 22:07:32 -0000
@@ -1117,7 +1117,8 @@
if (tu->qused >= tu->queue_size) {
tu->overrun++;
} else {
- memcpy(&tu->queue[tu->qtail++], tread, sizeof(*tread));
+ memcpy(&tu->tqueue[tu->qtail++], tread, sizeof(*tread));
+ tu->qtail %= tu->queue_size;
tu->qused++;
}
}
@@ -1140,6 +1141,8 @@
spin_lock(&tu->qlock);
snd_timer_user_append_to_tqueue(tu, &r1);
spin_unlock(&tu->qlock);
+ kill_fasync(&tu->fasync, SIGIO, POLL_IN);
+ wake_up(&tu->qchange_sleep);
}

static void snd_timer_user_tinterrupt(snd_timer_instance_t *timeri,

2005-04-05 16:56:03

by Greg KH

[permalink] [raw]
Subject: [08/08] uml: va_copy fix

-stable review patch. If anyone has any objections, please let us know.

------------------

Uses __va_copy instead of va_copy since some old versions of gcc (2.95.4
for instance) don't accept va_copy.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---

clean-linux-2.6.11-paolo/arch/um/kernel/skas/uaccess.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)

diff -puN arch/um/kernel/skas/uaccess.c~uml-va_copy_fix arch/um/kernel/skas/uaccess.c
--- clean-linux-2.6.11/arch/um/kernel/skas/uaccess.c~uml-va_copy_fix 2005-04-01 22:37:11.000000000 +0200
+++ clean-linux-2.6.11-paolo/arch/um/kernel/skas/uaccess.c 2005-04-01 22:37:11.000000000 +0200
@@ -61,7 +61,8 @@ static void do_buffer_op(void *jmpbuf, v
void *arg;
int *res;

- va_copy(args, *(va_list *)arg_ptr);
+ /* Some old gccs recognize __va_copy, but not va_copy */
+ __va_copy(args, *(va_list *)arg_ptr);
addr = va_arg(args, unsigned long);
len = va_arg(args, int);
is_write = va_arg(args, int);
_

_______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable

2005-04-05 16:59:50

by Greg KH

[permalink] [raw]
Subject: [02/08] Prevent race condition in jbd


-stable review patch. If anyone has any objections, please let us know.

------------------


This patch from Stephen Tweedie which fixes a race in jbd code (it
demonstrated itself as more or less random NULL dereferences in the
journal code).

Acked-by: Jan Kara <[email protected]>
Acked-by: Chris Mason <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

--- linux-2.6-ext3/fs/jbd/transaction.c.=K0000=.orig
+++ linux-2.6-ext3/fs/jbd/transaction.c
@@ -1775,10 +1775,10 @@ static int journal_unmap_buffer(journal_
JBUFFER_TRACE(jh, "checkpointed: add to BJ_Forget");
ret = __dispose_buffer(jh,
journal->j_running_transaction);
+ journal_put_journal_head(jh);
spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);
spin_unlock(&journal->j_state_lock);
- journal_put_journal_head(jh);
return ret;
} else {
/* There is no currently-running transaction. So the
@@ -1789,10 +1789,10 @@ static int journal_unmap_buffer(journal_
JBUFFER_TRACE(jh, "give to committing trans");
ret = __dispose_buffer(jh,
journal->j_committing_transaction);
+ journal_put_journal_head(jh);
spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);
spin_unlock(&journal->j_state_lock);
- journal_put_journal_head(jh);
return ret;
} else {
/* The orphan record's transaction has
@@ -1813,10 +1813,10 @@ static int journal_unmap_buffer(journal_
journal->j_running_transaction);
jh->b_next_transaction = NULL;
}
+ journal_put_journal_head(jh);
spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh);
spin_unlock(&journal->j_state_lock);
- journal_put_journal_head(jh);
return 0;
} else {
/* Good, the buffer belongs to the running transaction.

2005-04-05 16:59:48

by Greg KH

[permalink] [raw]
Subject: [03/08] fix ia64 syscall auditing

-stable review patch. If anyone has any objections, please let us know.

------------------

Attached is a patch against David's audit.17 kernel that adds checks
for the TIF_SYSCALL_AUDIT thread flag to the ia64 system call and
signal handling code paths. The patch enables auditing of system
calls set up via fsys_bubble_down, as well as ensuring that
audit_syscall_exit() is called on return from sigreturn.

Neglecting to check for TIF_SYSCALL_AUDIT at these points results in
incorrect information in audit_context, causing frequent system panics
when system call auditing is enabled on an ia64 system.

I have tested this patch and have seen no problems with it.

[Original patch from Amy Griffis ported to current kernel by David Woodhouse]

From: Amy Griffis <[email protected]>
From: David Woodhouse <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

--- 1.34/arch/ia64/kernel/fsys.S 2005-01-22 22:19:11 +00:00
+++ edited/arch/ia64/kernel/fsys.S 2005-04-01 00:20:32 +01:00
@@ -611,8 +611,10 @@
movl r2=ia64_ret_from_syscall
;;
mov rp=r2 // set the real return addr
- tbit.z p8,p0=r3,TIF_SYSCALL_TRACE
+ and r3=_TIF_SYSCALL_TRACEAUDIT,r3
;;
+ cmp.eq p8,p0=r3,r0
+
(p10) br.cond.spnt.many ia64_ret_from_syscall // p10==true means out registers are more than 8
(p8) br.call.sptk.many b6=b6 // ignore this return addr
br.cond.sptk ia64_trace_syscall
===== arch/ia64/kernel/signal.c 1.49 vs edited =====
--- 1.49/arch/ia64/kernel/signal.c 2005-01-25 20:23:45 +00:00
+++ edited/arch/ia64/kernel/signal.c 2005-04-01 00:18:29 +01:00
@@ -224,7 +224,8 @@
* could be corrupted.
*/
retval = (long) &ia64_leave_kernel;
- if (test_thread_flag(TIF_SYSCALL_TRACE))
+ if (test_thread_flag(TIF_SYSCALL_TRACE)
+ || test_thread_flag(TIF_SYSCALL_AUDIT))
/*
* strace expects to be notified after sigreturn returns even though the
* context to which we return may not be in the middle of a syscall.

2005-04-05 16:59:49

by Greg KH

[permalink] [raw]
Subject: [06/08] rwsem fix

-stable review patch. If anyone has any objections, please let us know.

------------------

We should merge this backport - it's needed to prevent deadlocks when
dio_complete() does up_read() from IRQ context. And perhaps other places.

From: David Howells <[email protected]>

[PATCH] rwsem: Make rwsems use interrupt disabling spinlocks

The attached patch makes read/write semaphores use interrupt disabling
spinlocks in the slow path, thus rendering the up functions and trylock
functions available for use in interrupt context. This matches the
regular semaphore behaviour.

I've assumed that the normal down functions must be called with interrupts
enabled (since they might schedule), and used the irq-disabling spinlock
variants that don't save the flags.

Signed-Off-By: David Howells <[email protected]>
Tested-by: Badari Pulavarty <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

diff -Nru a/lib/rwsem-spinlock.c b/lib/rwsem-spinlock.c
--- a/lib/rwsem-spinlock.c 2005-04-01 23:22:40 -08:00
+++ b/lib/rwsem-spinlock.c 2005-04-01 23:22:40 -08:00
@@ -140,12 +140,12 @@

rwsemtrace(sem, "Entering __down_read");

- spin_lock(&sem->wait_lock);
+ spin_lock_irq(&sem->wait_lock);

if (sem->activity >= 0 && list_empty(&sem->wait_list)) {
/* granted */
sem->activity++;
- spin_unlock(&sem->wait_lock);
+ spin_unlock_irq(&sem->wait_lock);
goto out;
}

@@ -160,7 +160,7 @@
list_add_tail(&waiter.list, &sem->wait_list);

/* we don't need to touch the semaphore struct anymore */
- spin_unlock(&sem->wait_lock);
+ spin_unlock_irq(&sem->wait_lock);

/* wait to be given the lock */
for (;;) {
@@ -181,10 +181,12 @@
*/
int fastcall __down_read_trylock(struct rw_semaphore *sem)
{
+ unsigned long flags;
int ret = 0;
+
rwsemtrace(sem, "Entering __down_read_trylock");

- spin_lock(&sem->wait_lock);
+ spin_lock_irqsave(&sem->wait_lock, flags);

if (sem->activity >= 0 && list_empty(&sem->wait_list)) {
/* granted */
@@ -192,7 +194,7 @@
ret = 1;
}

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irqrestore(&sem->wait_lock, flags);

rwsemtrace(sem, "Leaving __down_read_trylock");
return ret;
@@ -209,12 +211,12 @@

rwsemtrace(sem, "Entering __down_write");

- spin_lock(&sem->wait_lock);
+ spin_lock_irq(&sem->wait_lock);

if (sem->activity == 0 && list_empty(&sem->wait_list)) {
/* granted */
sem->activity = -1;
- spin_unlock(&sem->wait_lock);
+ spin_unlock_irq(&sem->wait_lock);
goto out;
}

@@ -229,7 +231,7 @@
list_add_tail(&waiter.list, &sem->wait_list);

/* we don't need to touch the semaphore struct anymore */
- spin_unlock(&sem->wait_lock);
+ spin_unlock_irq(&sem->wait_lock);

/* wait to be given the lock */
for (;;) {
@@ -250,10 +252,12 @@
*/
int fastcall __down_write_trylock(struct rw_semaphore *sem)
{
+ unsigned long flags;
int ret = 0;
+
rwsemtrace(sem, "Entering __down_write_trylock");

- spin_lock(&sem->wait_lock);
+ spin_lock_irqsave(&sem->wait_lock, flags);

if (sem->activity == 0 && list_empty(&sem->wait_list)) {
/* granted */
@@ -261,7 +265,7 @@
ret = 1;
}

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irqrestore(&sem->wait_lock, flags);

rwsemtrace(sem, "Leaving __down_write_trylock");
return ret;
@@ -272,14 +276,16 @@
*/
void fastcall __up_read(struct rw_semaphore *sem)
{
+ unsigned long flags;
+
rwsemtrace(sem, "Entering __up_read");

- spin_lock(&sem->wait_lock);
+ spin_lock_irqsave(&sem->wait_lock, flags);

if (--sem->activity == 0 && !list_empty(&sem->wait_list))
sem = __rwsem_wake_one_writer(sem);

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irqrestore(&sem->wait_lock, flags);

rwsemtrace(sem, "Leaving __up_read");
}
@@ -289,15 +295,17 @@
*/
void fastcall __up_write(struct rw_semaphore *sem)
{
+ unsigned long flags;
+
rwsemtrace(sem, "Entering __up_write");

- spin_lock(&sem->wait_lock);
+ spin_lock_irqsave(&sem->wait_lock, flags);

sem->activity = 0;
if (!list_empty(&sem->wait_list))
sem = __rwsem_do_wake(sem, 1);

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irqrestore(&sem->wait_lock, flags);

rwsemtrace(sem, "Leaving __up_write");
}
@@ -308,15 +316,17 @@
*/
void fastcall __downgrade_write(struct rw_semaphore *sem)
{
+ unsigned long flags;
+
rwsemtrace(sem, "Entering __downgrade_write");

- spin_lock(&sem->wait_lock);
+ spin_lock_irqsave(&sem->wait_lock, flags);

sem->activity = 1;
if (!list_empty(&sem->wait_list))
sem = __rwsem_do_wake(sem, 0);

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irqrestore(&sem->wait_lock, flags);

rwsemtrace(sem, "Leaving __downgrade_write");
}
diff -Nru a/lib/rwsem.c b/lib/rwsem.c
--- a/lib/rwsem.c 2005-04-01 23:22:40 -08:00
+++ b/lib/rwsem.c 2005-04-01 23:22:40 -08:00
@@ -150,7 +150,7 @@
set_task_state(tsk, TASK_UNINTERRUPTIBLE);

/* set up my own style of waitqueue */
- spin_lock(&sem->wait_lock);
+ spin_lock_irq(&sem->wait_lock);
waiter->task = tsk;
get_task_struct(tsk);

@@ -163,7 +163,7 @@
if (!(count & RWSEM_ACTIVE_MASK))
sem = __rwsem_do_wake(sem, 0);

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irq(&sem->wait_lock);

/* wait to be given the lock */
for (;;) {
@@ -219,15 +219,17 @@
*/
struct rw_semaphore fastcall *rwsem_wake(struct rw_semaphore *sem)
{
+ unsigned long flags;
+
rwsemtrace(sem, "Entering rwsem_wake");

- spin_lock(&sem->wait_lock);
+ spin_lock_irqsave(&sem->wait_lock, flags);

/* do nothing if list empty */
if (!list_empty(&sem->wait_list))
sem = __rwsem_do_wake(sem, 0);

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irqrestore(&sem->wait_lock, flags);

rwsemtrace(sem, "Leaving rwsem_wake");

@@ -241,15 +243,17 @@
*/
struct rw_semaphore fastcall *rwsem_downgrade_wake(struct rw_semaphore *sem)
{
+ unsigned long flags;
+
rwsemtrace(sem, "Entering rwsem_downgrade_wake");

- spin_lock(&sem->wait_lock);
+ spin_lock_irqsave(&sem->wait_lock, flags);

/* do nothing if list empty */
if (!list_empty(&sem->wait_list))
sem = __rwsem_do_wake(sem, 1);

- spin_unlock(&sem->wait_lock);
+ spin_unlock_irqrestore(&sem->wait_lock, flags);

rwsemtrace(sem, "Leaving rwsem_downgrade_wake");
return sem;

2005-04-05 17:50:17

by Greg KH

[permalink] [raw]
Subject: Re: [00/11] -stable review


Hm, sorry about the wrong subject, there were only 8 patches.

And here's a diffstat of all of them, just to make this email worth
reading and not just an apology:

lib/rwsem-spinlock.c | 42 ++++++++++++++++++++++++++----------------
lib/rwsem.c | 16 ++++++++++------
net/ipv4/xfrm4_output.c | 12 ++++++------
net/ipv6/xfrm6_output.c | 12 ++++++------
arch/um/kernel/skas/uaccess.c | 3 ++-
arch/ia64/kernel/fsys.S | 4 +++-
arch/ia64/kernel/signal.c | 3 ++-
net/ipv4/tcp_input.c | 5 ++++-
fs/jbd/transaction.c | 6 +++---
drivers/i2c/chips/eeprom.c | 3 ++-
sound/core/timer.c | 5 ++++-
11 files changed, 68 insertions(+), 43 deletions(-)

thanks,

greg k-h

2005-04-05 18:32:27

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [07/08] [TCP] Fix BIC congestion avoidance algorithm error

On Tue, Apr 05, 2005 at 09:47:59AM -0700, Greg KH wrote:
>
> -stable review patch. If anyone has any objections, please let us know.
>
> While redoing BIC for the split up version, I discovered that the
> existing 2.6.11 code doesn't really do binary search. It ends up
> being just a slightly modified version of Reno. See attached graphs
> to see the effect over simulated 1mbit environment.

I hate to be a stickler for the rules, but does this really meet this
criteria?

- It must fix a real bug that bothers people (not a, "This could be a
problem..." type thing.)

If the congestion control alogirthm is "Reno-like", what is
user-visible impact to users? There are OS's out there with TCP/IP
stacks that are still using Reno, aren't there?

Knowing the answer to the question, "How does this bug `bother' either
users or network administrators?" would probably be really helpful.

- Ted

2005-04-05 18:32:26

by David Miller

[permalink] [raw]
Subject: Re: [07/08] [TCP] Fix BIC congestion avoidance algorithm error

On Tue, 5 Apr 2005 14:22:02 -0400
Theodore Ts'o <[email protected]> wrote:

> If the congestion control alogirthm is "Reno-like", what is
> user-visible impact to users? There are OS's out there with TCP/IP
> stacks that are still using Reno, aren't there?

An incorrect implementation of any congestion control algorithm
has ramifications not considered when the congestion control
author verified the design of his algorithm.

This has a large impact on every user on the internet, not just
Linux machines.

Perhaps on a microscopic scale "this" part of the BIC algorithm
was just behaving Reno-like due to the bug, but what implications
does that error have as applied to the other heuristics in BIC?
This is what I'm talking about. BIC operates in several modes,
one of which is a pseudo binary search mode, and another is a
less aggressive slower increase mode.

Therefore I think fixes to congestion control algorithms which
are enabled by default always should take a high priority in
the stable kernels.

2005-04-05 18:45:56

by Renate Meijer

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix


On Apr 5, 2005, at 6:48 PM, Greg KH wrote:

> -stable review patch. If anyone has any objections, please let us
> know.
>
> ------------------
>
> Uses __va_copy instead of va_copy since some old versions of gcc
> (2.95.4
> for instance) don't accept va_copy.

Are there many kernels still being built with 2.95.4? It's quite
antiquated, as far as
i'm aware.

The use of '__' violates compiler namespace. If 2.95.4 were not easily
replaced by
a much better version (3.3.x? 3.4.x) I would see a reason to disregard
this, but a fix
merely to satisfy an obsolete compiler?

In my humblest of opinions you are fixing a bug that is better solved
by downloading
a more recent version of gcc.


Regards,

Renate Meijer.

2005-04-05 18:52:59

by Blaisorblade

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix

On Tuesday 05 April 2005 20:47, Renate Meijer wrote:
> On Apr 5, 2005, at 6:48 PM, Greg KH wrote:
> > -stable review patch. If anyone has any objections, please let us
> > know.
> >
> > ------------------
> >
> > Uses __va_copy instead of va_copy since some old versions of gcc
> > (2.95.4
> > for instance) don't accept va_copy.
>
> Are there many kernels still being built with 2.95.4? It's quite
> antiquated, as far as
> i'm aware.
>
> The use of '__' violates compiler namespace.
Why? The symbol is defined by the compiler itself.
> If 2.95.4 were not easily
> replaced by
> a much better version (3.3.x? 3.4.x) I would see a reason to disregard
> this, but a fix
> merely to satisfy an obsolete compiler?

Let's not flame, Linus Torvalds said "we support GCC 2.95.3, because the newer
versions are worse compilers in most cases". One user complained, even
because he uses Debian, and I cannot do less than make sure that we comply
with the requirements we have choosen (compiling with that GCC).

Please let's not start a flame on this. Consider me as having no opinion on
this except not wanting to break on purpose Debian users. If you want, submit
a patch removing Gcc 2.95.3 from supported versions, and get ready to fight
for it (and probably loose).

Also, that GCC has discovered some syscall table errors in UML - I sent a
separate patch, which was a bit big sadly (in the reduced version, about 70
lines + description).
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade

2005-04-05 18:36:24

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [07/08] [TCP] Fix BIC congestion avoidance algorithm error

On Tue, 5 Apr 2005 11:26:08 -0700
"David S. Miller" <[email protected]> wrote:

> On Tue, 5 Apr 2005 14:22:02 -0400
> Theodore Ts'o <[email protected]> wrote:
>
> > If the congestion control alogirthm is "Reno-like", what is
> > user-visible impact to users? There are OS's out there with TCP/IP
> > stacks that are still using Reno, aren't there?
>
> An incorrect implementation of any congestion control algorithm
> has ramifications not considered when the congestion control
> author verified the design of his algorithm.
>
> This has a large impact on every user on the internet, not just
> Linux machines.
>
> Perhaps on a microscopic scale "this" part of the BIC algorithm
> was just behaving Reno-like due to the bug, but what implications
> does that error have as applied to the other heuristics in BIC?
> This is what I'm talking about. BIC operates in several modes,
> one of which is a pseudo binary search mode, and another is a
> less aggressive slower increase mode.

> Therefore I think fixes to congestion control algorithms which
> are enabled by default always should take a high priority in
> the stable kernels.

Also, hopefully distro vendors will pick up 2.6.11.X fixes and update their customers.

2005-04-05 20:34:37

by David Mosberger

[permalink] [raw]
Subject: Re: [03/08] fix ia64 syscall auditing

>>>>> On Tue, 5 Apr 2005 09:46:48 -0700, Greg KH <[email protected]> said:

Greg> -stable review patch. If anyone has any objections, please
Greg> let us know.

Nitpick: the patch introduces trailing whitespace.

Why doesn't everybody use emacs and enable show-trailing-whitespace? ;-)

--david

2005-04-05 20:34:49

by Renate Meijer

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix


On Apr 5, 2005, at 8:53 PM, Blaisorblade wrote:

> On Tuesday 05 April 2005 20:47, Renate Meijer wrote:
>> On Apr 5, 2005, at 6:48 PM, Greg KH wrote:
>>> -stable review patch. If anyone has any objections, please let us
>>> know.
>>>
>>> ------------------
>>>
>>> Uses __va_copy instead of va_copy since some old versions of gcc
>>> (2.95.4
>>> for instance) don't accept va_copy.
>>
>> Are there many kernels still being built with 2.95.4? It's quite
>> antiquated, as far as
>> i'm aware.
>>
>> The use of '__' violates compiler namespace.
> Why? The symbol is defined by the compiler itself.

If a function is prefixed with a double underscore, this implies the
function is internal to
the compiler, and may change at any time, since it's not governed by
some sort of standard.
Hence that code may start suffering from bitrot and complaining to the
compiler guys won't help.

They'll just tell you to RTFM.

>> If 2.95.4 were not easily
>> replaced by
>> a much better version (3.3.x? 3.4.x) I would see a reason to disregard
>> this, but a fix
>> merely to satisfy an obsolete compiler?
>
> Let's not flame, Linus Torvalds said "we support GCC 2.95.3, because
> the newer
> versions are worse compilers in most cases".

You make it sound as if you were reciting Ye Holy Scribings. When did
Linus Thorvalds say this? In the Redhat-2.96 debacle? Before or after
3.3? I have searched for that quote, but could not find it, and having
suffered under 3.1.1, I can well understand his wearyness for the
earlier versions.

See

http://kerneltrap.org/node/4126, halfway down.

For the cold, hard facts...

http://www.suse.de/~aj/SPEC/

<snip>

> Consider me as having no opinion on this except not wanting to break
> on purpose Debian users.

If Debian users are stuck with a pretty outdated compiler, i'd
seriously suggest migrating to some
other distro which allows more freedom. If linux itself is holding them
back, there's a need for some serious patching. If there are serious
issues in the gcc compiler, which hinder migration to a more up-to-date
version our efforts should be directed at solving them in that project,
not this.

> If you want, submit a patch removing Gcc 2.95.3 from supported
> versions, and get ready to fight
> for it (and probably loose).

I don't fight over things like that, i'm not interested in politics. I
merely point out the problem. And yes.
I do think support for obsolete compiler should be dumped in favor of a
more modern version. Especially if that compiler requires invasions of
compiler-namespace. The patch, as presented, is not guaranteed to be
portable over versions, and may thus introduce another problem with
future versions of GCC.

> Also, that GCC has discovered some syscall table errors in UML - I
> sent a
> separate patch, which was a bit big sadly (in the reduced version,
> about 70
> lines + description).

I am not quite sure what is intended here... Please explain.

timeo hominem unius libri

Thomas van Aquino

2005-04-05 20:51:03

by Greg KH

[permalink] [raw]
Subject: Re: [03/08] fix ia64 syscall auditing

On Tue, 2005-04-05 at 13:27 -0700, David Mosberger wrote:
> >>>>> On Tue, 5 Apr 2005 09:46:48 -0700, Greg KH <[email protected]> said:
>
> Greg> -stable review patch. If anyone has any objections, please
> Greg> let us know.
>
> Nitpick: the patch introduces trailing whitespace.

Sorry about that, I've removed it from the patch now.

> Why doesn't everybody use emacs and enable show-trailing-whitespace? ;-)

Because some of us use vim and ":set list" to see it, when we remember
to... :)

thanks,

greg k-h

2005-04-05 21:16:21

by Randy.Dunlap

[permalink] [raw]
Subject: Re: [03/08] fix ia64 syscall auditing

Greg KH wrote:
> On Tue, 2005-04-05 at 13:27 -0700, David Mosberger wrote:
>
>>>>>>>On Tue, 5 Apr 2005 09:46:48 -0700, Greg KH <[email protected]> said:
>>
>> Greg> -stable review patch. If anyone has any objections, please
>> Greg> let us know.
>>
>>Nitpick: the patch introduces trailing whitespace.
>
>
> Sorry about that, I've removed it from the patch now.
>
>
>>Why doesn't everybody use emacs and enable show-trailing-whitespace? ;-)
>
>
> Because some of us use vim and ":set list" to see it, when we remember
> to... :)

others check received patches with a script instead of....
no, let's not debate $EDITOR.

--
~Randy

2005-04-05 23:49:05

by Ryan Anderson

[permalink] [raw]
Subject: Re: [03/08] fix ia64 syscall auditing

On Tue, Apr 05, 2005 at 01:49:18PM -0700, Greg KH wrote:
> On Tue, 2005-04-05 at 13:27 -0700, David Mosberger wrote:
> > >>>>> On Tue, 5 Apr 2005 09:46:48 -0700, Greg KH <[email protected]> said:
> >
> > Greg> -stable review patch. If anyone has any objections, please
> > Greg> let us know.
> >
> > Nitpick: the patch introduces trailing whitespace.
>
> Sorry about that, I've removed it from the patch now.
>
> > Why doesn't everybody use emacs and enable show-trailing-whitespace? ;-)
>
> Because some of us use vim and ":set list" to see it, when we remember
> to... :)

Try adding this to your .vimrc:

highlight WhitespaceEOL ctermbg=red guibg=red
match WhitespaceEOL /\s\+$/

Then you'll have to resist the urge to fix whitespace issues instead of
not seeing them at all.

--

Ryan Anderson
sometimes Pug Majere

2005-04-06 00:05:19

by Greg KH

[permalink] [raw]
Subject: Re: [03/08] fix ia64 syscall auditing

On Tue, 2005-04-05 at 19:46 -0400, Ryan Anderson wrote:
> highlight WhitespaceEOL ctermbg=red guibg=red
> match WhitespaceEOL /\s\+$/

Very nice, thanks a lot for that.

greg k-h

2005-04-06 00:50:41

by Dave Jones

[permalink] [raw]
Subject: Re: [03/08] fix ia64 syscall auditing

On Tue, Apr 05, 2005 at 05:05:04PM -0700, Greg KH wrote:
> On Tue, 2005-04-05 at 19:46 -0400, Ryan Anderson wrote:
> > highlight WhitespaceEOL ctermbg=red guibg=red
> > match WhitespaceEOL /\s\+$/
>
> Very nice, thanks a lot for that.

let c_space_errors=1 also works great.

Dave

2005-04-06 11:32:58

by Jörn Engel

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix

On Tue, 5 April 2005 22:18:26 +0200, Renate Meijer wrote:
>
> If a function is prefixed with a double underscore, this implies the
> function is internal to
> the compiler, and may change at any time, since it's not governed by
> some sort of standard.
> Hence that code may start suffering from bitrot and complaining to the
> compiler guys won't help.

You did read include/linux/compiler.h, didn't you?
And you did read this thread as well, right?
http://kerneltrap.org/node/4126

J?rn

--
Time? What's that? Time is only worth what you do with it.
-- Theo de Raadt

2005-04-06 12:05:27

by Renate Meijer

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix


On Apr 6, 2005, at 1:32 PM, J?rn Engel wrote:

> On Tue, 5 April 2005 22:18:26 +0200, Renate Meijer wrote:
>>
>> If a function is prefixed with a double underscore, this implies the
>> function is internal to
>> the compiler, and may change at any time, since it's not governed by
>> some sort of standard.
>> Hence that code may start suffering from bitrot and complaining to the
>> compiler guys won't help.
>
> You did read include/linux/compiler.h, didn't you?

Yes. It seems a good place to hide compiler-internal stuff as the one
this patch
implements. And I know linux does not exactly have a great track record
on this issue. But just because the dependency is there, does not imply
it is a
GOOD THING to have it.

So instead of applying this patch, simply

#ifdef VERSION_MINOR < WHATEVER
#define va_copy __va_copy
#endif

in include/linux/compiler_gcc2.h

Thus solving the problem without having to invade compiler namespace all
over the place, but doing so in *one* place only.

> And you did read this thread as well, right?
> http://kerneltrap.org/node/4126

<quote>
Things seem to have improved a bit lately. The gcc-3.x series was
basically not worth it for plain C until 3.3 or so.
</quote>

Yes. You did read the actual data as produced by that guy from Suse,
did you? In the past,
people may have justly stuck to (e.g.) 2.95.3, however, support for
that version now starts to
require dependencies on compiler internals. This is one argument in
favor of dropping support
for that version, or at least not to spread compiler dependent stuff
all over the code.


2005-04-06 12:27:45

by Jörn Engel

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix

On Wed, 6 April 2005 14:04:39 +0200, Renate Meijer wrote:
>
> >And you did read this thread as well, right?
> >http://kerneltrap.org/node/4126
>
> <quote>
> Things seem to have improved a bit lately. The gcc-3.x series was
> basically not worth it for plain C until 3.3 or so.
> </quote>
>
> Yes. You did read the actual data as produced by that guy from Suse,
> did you? In the past,
> people may have justly stuck to (e.g.) 2.95.3, however, support for
> that version now starts to
> require dependencies on compiler internals. This is one argument in
> favor of dropping support
> for that version, or at least not to spread compiler dependent stuff
> all over the code.

Fyi, another fact that was missing from the quoted thread: gcc 2.95
catches bugs that 3.x compilers simply miss. Support for the old
compiler is more work, no doubt, and at times requires to work around
plain compiler bugs as well. But there is some return on investment.

Is it worth the effort? Not sure. But the "it's old, drop support
for it" argument just doesn't cut it and it doesn't get any better by
repetition.

J?rn

--
Schr?dinger's cat is <BLINK>not</BLINK> dead.
-- Illiad

2005-04-06 15:47:34

by Greg KH

[permalink] [raw]
Subject: Re: [stable] Re: [08/08] uml: va_copy fix

On Wed, Apr 06, 2005 at 02:27:51PM +0200, J?rn Engel wrote:
>
> Is it worth the effort? Not sure. But the "it's old, drop support
> for it" argument just doesn't cut it and it doesn't get any better by
> repetition.

Exactly, that's why this patch is valid.

thanks,

greg k-h

2005-04-06 17:24:07

by Renate Meijer

[permalink] [raw]
Subject: Re: [stable] Re: [08/08] uml: va_copy fix


On Apr 6, 2005, at 5:46 PM, Greg KH wrote:

> On Wed, Apr 06, 2005 at 02:27:51PM +0200, J?rn Engel wrote:
>>
>> Is it worth the effort? Not sure. But the "it's old, drop support
>> for it" argument just doesn't cut it and it doesn't get any better by
>> repetition.

However, the argument gets better every time "a workaround" is needed.
If there are still serious issues
open (like a failure to catch bugs the old version did), they are
issues which need resolving in the
compiler. Patching the wrong project, is introducing two imperfections.

I think its worth the time and trouble to take this up with the gcc
crowd. So if you could provide a list of things 3.3 misses, i'm sure
the gcc-crowd would like it.

> Exactly, that's why this patch is valid.

At the very least, it's at the wrong place, since it should be patched
in ./include/linux/compiler.h. And I do not exactly argue "it's old,
drop support for it", but rely on the "dont rely on compiler internals
or at least stick them on one place where everyone can find them
easily, instead of peppering the entire codebase with them" argument.

Regards,

Renate Meijer.

2005-04-06 17:33:29

by Jörn Engel

[permalink] [raw]
Subject: Re: [stable] Re: [08/08] uml: va_copy fix

On Wed, 6 April 2005 19:29:46 +0200, Renate Meijer wrote:
>
> I think its worth the time and trouble to take this up with the gcc
> crowd. So if you could provide a list of things 3.3 misses, i'm sure
> the gcc-crowd would like it.

If you volunteer to do work with the gcc-crowd, I can dig up some old
stuff and send you testcases. Sure.

J?rn

--
The strong give up and move away, while the weak give up and stay.
-- unknown

2005-04-06 17:52:30

by Renate Meijer

[permalink] [raw]
Subject: Re: [stable] Re: [08/08] uml: va_copy fix


On Apr 6, 2005, at 7:33 PM, J?rn Engel wrote:

> On Wed, 6 April 2005 19:29:46 +0200, Renate Meijer wrote:
>>
>> I think its worth the time and trouble to take this up with the gcc
>> crowd. So if you could provide a list of things 3.3 misses, i'm sure
>> the gcc-crowd would like it.
>
> If you volunteer to do work with the gcc-crowd, I can dig up some old
> stuff and send you testcases. Sure.

I'll volunteer. As I said, i think it's worth the time and trouble. But
i can't do it on my own,
at least i need some backup from thee development community around
here. You telling
me what's up, for one. I found one mail by Greg KH spelling out some of
the problems, but you
suggest there's more to worry about.

Problem is, i'll be spending 5 weeks prety much scattered around
europe, starting next friday and have
no idea on my online-ness yet. As it is, my trusty mac is my only
digital companion, and my linux-box is in
storage for the time being.

So don't expect any results before May 17. But hey... Somebody has to
do it. Just don't be surprised if the
folks at gcc do not agree with you "a prima vista". They may have a
different idea on what exactly constitutes
a bug.

Upside is, i'll be taking my Mac anyway, so at least i'll have the
sources handy. I'll start downloading tonight, so if you have data,
please, lets have it.

Nevertheless, the points made in previous posts stand.


Regards,

Renate Meijer.
--

timeo hominem unius libri

Thomas van Aquino

2005-04-06 18:12:59

by Jörn Engel

[permalink] [raw]
Subject: Re: [stable] Re: [08/08] uml: va_copy fix

On Wed, 6 April 2005 19:58:06 +0200, Renate Meijer wrote:
> On Apr 6, 2005, at 7:33 PM, J?rn Engel wrote:
> >On Wed, 6 April 2005 19:29:46 +0200, Renate Meijer wrote:
> >>
> >>I think its worth the time and trouble to take this up with the gcc
> >>crowd. So if you could provide a list of things 3.3 misses, i'm sure
> >>the gcc-crowd would like it.
> >
> >If you volunteer to do work with the gcc-crowd, I can dig up some old
> >stuff and send you testcases. Sure.
>
> I'll volunteer. [...]

Thanks!

> Problem is, i'll be spending 5 weeks prety much scattered around
> europe, [...]

Have fun!

Ok, here is the first testcase. It was a real bug, even though never
submitted to Linus:

#define ASM_MACRO \
op; \
#ifdef FOO \
op; \
#else \
op; \
#endif \
op; \
op;


The thing occurred in some entry.S or head.S, don't remember exactly
which one. Gcc people might tell you unfriendly things about using
the _C_ preprocessor for _ASM_ code, but that's just how the kernel
code is written.

With gcc 2.95, the old preprocessor errored out on the correct line
and we had a look at the code. With 3.x, preprocessor chewed things
and starting from "#ifdef", everything was interpreted as a comment
and ignored.

Code never worked and the real bug was papered over by more ugliness,
but stayed that way for a year until someone (me) tried to compile it
with 2.95.

J?rn

--
You cannot suppose that Moliere ever troubled himself to be original in the
matter of ideas. You cannot suppose that the stories he tells in his plays
have never been told before. They were culled, as you very well know.
-- Andre-Louis Moreau in Scarabouche

2005-04-06 19:06:17

by Blaisorblade

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix

For J?rn Engel and the issue he opened: at the end of this mail I describe
another bug caught by 2.95 and not by 3.x.

On Tuesday 05 April 2005 22:18, Renate Meijer wrote:
> On Apr 5, 2005, at 8:53 PM, Blaisorblade wrote:
> > On Tuesday 05 April 2005 20:47, Renate Meijer wrote:
> >> On Apr 5, 2005, at 6:48 PM, Greg KH wrote:

> >> The use of '__' violates compiler namespace.
> >
> > Why? The symbol is defined by the compiler itself.

> If a function is prefixed with a double underscore, this implies the
> function is internal to
> the compiler, and may change at any time, since it's not governed by
> some sort of standard.
> Hence that code may start suffering from bitrot and complaining to the
> compiler guys won't help.

> They'll just tell you to RTFM.
Ok, agreed in general. However, the -stable tree is for "current" GCC. Your
objections would better refer to the fact that the same patch has already
been merged into the main trunk.

Also, they have no point in doing this, probably. And the __va_copy name was
used in the draft C99 standard so it's widespread (I've read this on "man 3
va_copy").
> >> If 2.95.4 were not easily
> >> replaced by
> >> a much better version (3.3.x? 3.4.x) I would see a reason to disregard
> >> this, but a fix
> >> merely to satisfy an obsolete compiler?
> >
> > Let's not flame, Linus Torvalds said "we support GCC 2.95.3, because
> > the newer
> > versions are worse compilers in most cases".

> You make it sound as if you were reciting Ye Holy Scribings. When did
> Linus Thorvalds say this? In the Redhat-2.96 debacle? Before or after
> 3.3? I have searched for that quote,
Sorry for the quote marks, it was a resume of what he said (and from
re-reading, it's still a correct resume).
> but could not find it, and having
> suffered under 3.1.1, I can well understand his wearyness for the
> earlier versions.
I've read the same kerneltrap article you quote.
> See
>
> http://kerneltrap.org/node/4126, halfway down.
Ok, read.
> For the cold, hard facts...
>
> http://www.suse.de/~aj/SPEC/
Linus pointed out that SPEC performances are not a good test case for the
kernel compilation in that article. Point out a kernel compilation case.
> <snip>
>
> > Consider me as having no opinion on this except not wanting to break
> > on purpose Debian users.
>
> If Debian users are stuck with a pretty outdated compiler, i'd
> seriously suggest migrating to some
> other distro which allows more freedom.
I guess they can, if they want, upgrade some selected packages from newer
trees, maybe by recompiling (at least, on Gentoo it's trivial, maybe on a
binary distro like Debian it's harder).
> If linux itself is holding them
> back, there's a need for some serious patching.

> If there are serious
> issues in the gcc compiler, which hinder migration to a more up-to-date
> version our efforts should be directed at solving them in that project,
> not this.
Linus spoke about the compiler speed, which isn't such a bad reason. He's
unfair in saying that GCC 3.x does not optimize better than older releases,
probably; I guess that the compilation flags (I refer to
-fno-strict-aliasing, which disables some optimizations) make some
difference, as do the memory barriers (as pointed in the comments).

> > If you want, submit a patch removing Gcc 2.95.3 from supported
> > versions, and get ready to fight
> > for it (and probably loose).

> I don't fight over things like that, i'm not interested in politics. I
> merely point out the problem. And yes.
> I do think support for obsolete compiler should be dumped in favor of a
> more modern version. Especially if that compiler requires invasions of
> compiler-namespace. The patch, as presented, is not guaranteed to be
> portable over versions, and may thus introduce another problem with
> future versions of GCC.
When and if that will happen, I'll come with an hack. UML already has need for
some GCC - version specific behaviour (see arch/um/kernel/gmon_syms.c on a
recent BitKeeper snapshot, even -rc1-bk5 has this code).

> > Also, that GCC has discovered some syscall table errors in UML - I
> > sent a
> > separate patch, which was a bit big sadly (in the reduced version,
> > about 70
> > lines + description).

> I am not quite sure what is intended here... Please explain.
I'm reattaching the patch, so that you can look at the changelog (I'm also
resending it as a separate email so that it is reviewed and possibly merged).
Basically this is an error in GCC 2 and not in GCC 3:

int [] list = {
[0] = 1,
[0] = 1
}
(I've not tested the above itself, but this should be a stripped down version
of one of the bugs fixed in the patch).

That sort of code in the UML syscall table is not the safer one - in fact,
apart this patch for the stable tree, I'm refactoring the UML syscall table
completely (for 2.6.12 / 2.6.13).

Btw: I've not investigated which one of the two behaviours is the buggy one -
if you know, maybe you or I can report it.
> timeo hominem unius libri

> Thomas van Aquino

--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


Attachments:
(No filename) (5.02 kB)
uml-quick-fix-syscall-table-for-stable.patch (7.05 kB)
Download all attachments

2005-04-06 19:09:05

by Blaisorblade

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix

On Wednesday 06 April 2005 14:04, Renate Meijer wrote:
> On Apr 6, 2005, at 1:32 PM, J?rn Engel wrote:
> > On Tue, 5 April 2005 22:18:26 +0200, Renate Meijer wrote:

> >
> > You did read include/linux/compiler.h, didn't you?

> So instead of applying this patch, simply
>
> #ifdef VERSION_MINOR < WHATEVER
> #define va_copy __va_copy
> #endif
>
> in include/linux/compiler_gcc2.h
>
> Thus solving the problem without having to invade compiler namespace all
> over the place, but doing so in *one* place only.
About this one: thanks for suggesting this and being constructive, I'll do
ASAP (if I don't forget) this for the -bk tree. However, I think that Greg KH
for the stable tree would prefer a local tested patch rather than a global
one with possible side effects, right Greg?

Also, I hope this discussion does not count as a vote for the -stable tree
inclusion (since dropping GCC 2 support in the -stable tree is exactly the
purpose of this tree, right ;-) ? ).
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade

2005-04-06 19:30:01

by Jörn Engel

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix

On Wed, 6 April 2005 21:09:50 +0200, Blaisorblade wrote:
>
> I'm reattaching the patch, so that you can look at the changelog (I'm also
> resending it as a separate email so that it is reviewed and possibly merged).
> Basically this is an error in GCC 2 and not in GCC 3:
>
> int [] list = {
> [0] = 1,
> [0] = 1
> }
> (I've not tested the above itself, but this should be a stripped down version
> of one of the bugs fixed in the patch).
>
> That sort of code in the UML syscall table is not the safer one - in fact,
> apart this patch for the stable tree, I'm refactoring the UML syscall table
> completely (for 2.6.12 / 2.6.13).
>
> Btw: I've not investigated which one of the two behaviours is the buggy one -
> if you know, maybe you or I can report it.

Your code is at best redundant. And I'd bet beer that it is not what
its author intended to write. So the bug is in GCC 3, imo.

J?rn

--
The cost of changing business rules is much more expensive for software
than for a secretaty.
-- unknown

2005-04-06 22:50:22

by Andrew Morton

[permalink] [raw]
Subject: Re: [03/08] fix ia64 syscall auditing

Ryan Anderson <[email protected]> wrote:
>
> On Tue, Apr 05, 2005 at 01:49:18PM -0700, Greg KH wrote:
> > On Tue, 2005-04-05 at 13:27 -0700, David Mosberger wrote:
> > > >>>>> On Tue, 5 Apr 2005 09:46:48 -0700, Greg KH <[email protected]> said:
> > >
> > > Greg> -stable review patch. If anyone has any objections, please
> > > Greg> let us know.
> > >
> > > Nitpick: the patch introduces trailing whitespace.
> >
> > Sorry about that, I've removed it from the patch now.
> >
> > > Why doesn't everybody use emacs and enable show-trailing-whitespace? ;-)
> >
> > Because some of us use vim and ":set list" to see it, when we remember
> > to... :)
>
> Try adding this to your .vimrc:
>
> highlight WhitespaceEOL ctermbg=red guibg=red
> match WhitespaceEOL /\s\+$/
>
> Then you'll have to resist the urge to fix whitespace issues instead of
> not seeing them at all.
>

Yeah, that's a risk. But gratuitous trailing whitespace changes shouldn't
cause a lot of downstream problems due to `patch -l'.

What I do is to ensure that we never _add_ trailing whitespace. So
anything which matches

^+.*[tab or space]$

gets trimmed. My theory is that after 10 years of this, all the trailing
whitespace will be gone. Problem is, I also see the hundreds of lines of
code in the bk patches which add trailing whitespace :(

Larry sent me a little bk script which would spam the user if they tried to
commit something which adds trailing whitespace, but maybe that's a bit
academic right now.

2005-04-07 09:54:22

by Renate Meijer

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix


On Apr 6, 2005, at 9:09 PM, Blaisorblade wrote:

> For J?rn Engel and the issue he opened: at the end of this mail I
> describe
> another bug caught by 2.95 and not by 3.x.
>
> On Tuesday 05 April 2005 22:18, Renate Meijer wrote:
>> On Apr 5, 2005, at 8:53 PM, Blaisorblade wrote:
>>> On Tuesday 05 April 2005 20:47, Renate Meijer wrote:
>>>> On Apr 5, 2005, at 6:48 PM, Greg KH wrote:
>
>>>> The use of '__' violates compiler namespace.
>>>
>>> Why? The symbol is defined by the compiler itself.
>
>> If a function is prefixed with a double underscore, this implies the
>> function is internal to
>> the compiler, and may change at any time, since it's not governed by
>> some sort of standard.
>> Hence that code may start suffering from bitrot and complaining to the
>> compiler guys won't help.
>
>> They'll just tell you to RTFM.
> Ok, agreed in general. However, the -stable tree is for "current" GCC.
> Your
> objections would better refer to the fact that the same patch has
> already
> been merged into the main trunk.
>
> Also, they have no point in doing this, probably. And the __va_copy
> name was
> used in the draft C99 standard so it's widespread (I've read this on
> "man 3
> va_copy").


>>>> If 2.95.4 were not easily
>>>> replaced by
>>>> a much better version (3.3.x? 3.4.x) I would see a reason to
>>>> disregard
>>>> this, but a fix
>>>> merely to satisfy an obsolete compiler?
>>>
>>> Let's not flame, Linus Torvalds said "we support GCC 2.95.3, because
>>> the newer
>>> versions are worse compilers in most cases".
>
>> You make it sound as if you were reciting Ye Holy Scribings. When did
>> Linus Thorvalds say this? In the Redhat-2.96 debacle? Before or after
>> 3.3? I have searched for that quote,

> Sorry for the quote marks, it was a resume of what he said (and from
> re-reading, it's still a correct resume).


>> but could not find it, and having
>> suffered under 3.1.1, I can well understand his wearyness for the
>> earlier versions.
>

> I've read the same kerneltrap article you quote.
>> See
>>
>> http://kerneltrap.org/node/4126, halfway down.
> Ok, read.
>> For the cold, hard facts...
>>
>> http://www.suse.de/~aj/SPEC/
> Linus pointed out that SPEC performances are not a good test case for
> the
> kernel compilation in that article. Point out a kernel compilation
> case.


>> <snip>
>>
>>> Consider me as having no opinion on this except not wanting to break
>>> on purpose Debian users.
>>
>> If Debian users are stuck with a pretty outdated compiler, i'd
>> seriously suggest migrating to some
>> other distro which allows more freedom.
> I guess they can, if they want, upgrade some selected packages from
> newer
> trees, maybe by recompiling (at least, on Gentoo it's trivial, maybe
> on a
> binary distro like Debian it's harder).

On a binary distro they can recompile, too. Althoughg i'll admit it's
not something
a newbie should do.
>> If linux itself is holding them
>> back, there's a need for some serious patching.
>
>> If there are serious
>> issues in the gcc compiler, which hinder migration to a more
>> up-to-date
>> version our efforts should be directed at solving them in that
>> project,
>> not this.
> Linus spoke about the compiler speed, which isn't such a bad reason.

It may be a reason for folks who do a lot of development. But that can
hardly serve
as a "main" reason. Speed of compilation, after all, is a one-time
thing. Howver, as i've
understood, there are more pressing reasons.

> He's
> unfair in saying that GCC 3.x does not optimize better than older
> releases,
> probably; I guess that the compilation flags (I refer to
> -fno-strict-aliasing, which disables some optimizations) make some
> difference, as do the memory barriers (as pointed in the comments).
>
>>> If you want, submit a patch removing Gcc 2.95.3 from supported
>>> versions, and get ready to fight
>>> for it (and probably loose).
>
>> I don't fight over things like that, i'm not interested in politics. I
>> merely point out the problem. And yes.
>> I do think support for obsolete compiler should be dumped in favor of
>> a
>> more modern version. Especially if that compiler requires invasions of
>> compiler-namespace. The patch, as presented, is not guaranteed to be
>> portable over versions, and may thus introduce another problem with
>> future versions of GCC.

> When and if that will happen, I'll come with an hack.

Ok. And a couple of hacks down the road, and the code will look nice
and cryptic and a newbie trying to understand what's going on, will
have a nice set of puzzles to solve.

> UML already has need for some GCC - version specific behaviour
> (see arch/um/kernel/gmon_syms.c on a recent BitKeeper snapshot,
> even -rc1-bk5 has this code).

Perhaps. But i think you'll agree it's not "The way to go".

>>> Also, that GCC has discovered some syscall table errors in UML - I
>>> sent a
>>> separate patch, which was a bit big sadly (in the reduced version,
>>> about 70
>>> lines + description).
>
>> I am not quite sure what is intended here... Please explain.
> I'm reattaching the patch, so that you can look at the changelog (I'm
> also
> resending it as a separate email so that it is reviewed and possibly
> merged).
> Basically this is an error in GCC 2 and not in GCC 3:
>
> int [] list = {
> [0] = 1,
> [0] = 1
> }
> (I've not tested the above itself, but this should be a stripped down
> version
> of one of the bugs fixed in the patch).
>
> That sort of code in the UML syscall table is not the safer one - in
> fact,
> apart this patch for the stable tree, I'm refactoring the UML syscall
> table
> completely (for 2.6.12 / 2.6.13).
>
> Btw: I've not investigated which one of the two behaviours is the
> buggy one -
> if you know, maybe you or I can report it.

From a strict ISO-C point of view, both are. It's a gcc-specific
"feature" which (agreed) does come in handy sometimes. However it makes
it quite hard to say which is the buggy version, since the
"appropriate" behavior
is a question of definition (by the gcc-folks). They may even argue
that, having changed their minds about it, neither is buggy, but both
conform to the specifications (for that specific functionality).

That's pretty much the trouble with relying on gcc-extensions: since
there's no standard, it's difficult to tell what's wrong and what's
right. I'll dive into it.

Regards,

Renate Meijer.


timeo hominem unius libri

Thomas van Aquino

2005-04-07 18:19:27

by Blaisorblade

[permalink] [raw]
Subject: Re: [08/08] uml: va_copy fix

On Thursday 07 April 2005 11:16, Renate Meijer wrote:
> On Apr 6, 2005, at 9:09 PM, Blaisorblade wrote:

> > Btw: I've not investigated which one of the two behaviours is the
> > buggy one -
> > if you know, maybe you or I can report it.
>
> From a strict ISO-C point of view, both are. It's a gcc-specific
> "feature" which (agreed) does come in handy sometimes.

Well, for "range" assignments GCC mustn't complain, but for the rest the
double assignment laziness is not very useful. Could they at least add a
-Wsomething inside -Wall or -W for this problem?

> However it makes
> it quite hard to say which is the buggy version, since the
> "appropriate" behavior
> is a question of definition (by the gcc-folks). They may even argue
> that, having changed their minds about it, neither is buggy, but both
> conform to the specifications (for that specific functionality).
>
> That's pretty much the trouble with relying on gcc-extensions: since
> there's no standard, it's difficult to tell what's wrong and what's
> right. I'll dive into it.
>
> Regards,
>
> Renate Meijer.

--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade