Re: [3/4] 2.6.22-rc3: known regressions

Re: [3/4] 2.6.22-rc3: known regressions

[Michal Piotrowski]

Hi all,

Here is a list of some known regressions in 2.6.22-rc3.

Feel free to add new regressions/remove fixed etc.
http://kernelnewbies.org/known_regressions



Suspend

Subject    : 2.6.22-rc1 suspend to RAM problem
References : http://permalink.gmane.org/gmane.linux.power-management.general/5819
Submitter  : Marcus Better <marcus@better.se>
Handled-By : Stefan Richter <stefanr@s5r6.in-berlin.de>
             Kristian Høgsberg <krh@bitplanet.net>
Status     : caused by fw-ohci module



Timers

Subject    : hrtimer overflow bug on 64-bit systems
References : http://lkml.org/lkml/2007/5/24/391
Submitter  : David Miller <davem@davemloft.net>
Status     : problem is being debugged



TTY

Subject    : tty-related oops in latest kernel(s)
References : http://lkml.org/lkml/2007/5/27/104
Submitter  : Tero Roponen <teanropo@jyu.fi>
Status     : problem is being debugged



USB

Subject    : usb hotplug/udev cannot correctly register usb/scanners
References : http://lkml.org/lkml/2007/5/15/205
Submitter  : art@usfltd.com <art@usfltd.com>
Status     : Unknown



Regards,
Michal

--
"Najbardziej brakowało mi twojego milczenia."
-- Andrzej Sapkowski "Coś więcej"

[PATCH] NOHZ: prevent multiplication overflow - stop timer for huge timeouts

[Thomas Gleixner]

get_next_timer_interrupt() returns a delta of (LONG_MAX > 1) in case
there is no timer pending. On 64 bit machines this results in a
multiplication overflow in tick_nohz_stop_sched_tick(). 

Reported by: Dave Miller <davem@davemloft.net>

Make the return value a constant and limit the return value to a 32 bit
value.

When the max timeout value is returned, we can safely stop the tick
timer device. The max jiffies delta results in a 12 days timeout for
HZ=1000.

In the long term the get_next_timer_interrupt() code needs to be
reworked to return ktime instead of jiffies, but we have to wait until
the last users of the original NO_IDLE_HZ code are converted.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/timer.h    |    6 ++++++
 kernel/time/tick-sched.c |   16 +++++++++++++++-
 kernel/timer.c           |   10 +++++++++-
 3 files changed, 30 insertions(+), 2 deletions(-)

Index: linux-2.6.21/include/linux/timer.h
===================================================================
--- linux-2.6.21.orig/include/linux/timer.h	2007-05-29 17:58:04.000000000 +0200
+++ linux-2.6.21/include/linux/timer.h	2007-05-29 17:58:06.000000000 +0200
@@ -68,6 +68,12 @@
 extern int mod_timer(struct timer_list *timer, unsigned long expires);
 
 /*
+ * The jiffies value which is added to now, when there is no timer
+ * in the timer wheel:
+ */
+#define NEXT_TIMER_MAX_DELTA	((1UL << 30) - 1)
+
+/*
  * Return when the next timer-wheel timeout occurs (in absolute jiffies),
  * locks the timer base:
  */
Index: linux-2.6.21/kernel/time/tick-sched.c
===================================================================
--- linux-2.6.21.orig/kernel/time/tick-sched.c	2007-05-29 17:58:04.000000000 +0200
+++ linux-2.6.21/kernel/time/tick-sched.c	2007-05-29 17:58:06.000000000 +0200
@@ -233,6 +233,21 @@
 		if (cpu == tick_do_timer_cpu)
 			tick_do_timer_cpu = -1;
 
+		ts->idle_sleeps++;
+
+		/*
+		 * delta_jiffies >= NEXT_TIMER_MAX_DELTA signals that
+		 * there is no timer pending or at least extremly far
+		 * into the future (12 days for HZ=1000). In this case
+		 * we simply stop the tick timer:
+		 */
+		if (unlikely(delta_jiffies >= NEXT_TIMER_MAX_DELTA)) {
+			ts->idle_expires.tv64 = KTIME_MAX;
+			if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
+				hrtimer_cancel(&ts->sched_timer);
+			goto out;
+		}
+
 		/*
 		 * calculate the expiry time for the next timer wheel
 		 * timer
@@ -240,7 +255,6 @@
 		expires = ktime_add_ns(last_update, tick_period.tv64 *
 				       delta_jiffies);
 		ts->idle_expires = expires;
-		ts->idle_sleeps++;
 
 		if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
 			hrtimer_start(&ts->sched_timer, expires,
Index: linux-2.6.21/kernel/timer.c
===================================================================
--- linux-2.6.21.orig/kernel/timer.c	2007-05-29 17:58:05.000000000 +0200
+++ linux-2.6.21/kernel/timer.c	2007-05-29 17:58:06.000000000 +0200
@@ -625,7 +625,7 @@
 static unsigned long __next_timer_interrupt(tvec_base_t *base)
 {
 	unsigned long timer_jiffies = base->timer_jiffies;
-	unsigned long expires = timer_jiffies + (LONG_MAX >> 1);
+	unsigned long expires = timer_jiffies + NEXT_TIMER_MAX_DELTA;
 	int index, slot, array, found = 0;
 	struct timer_list *nte;
 	tvec_t *varray[4];
@@ -708,6 +708,14 @@
 
 	tsdelta = ktime_to_timespec(hr_delta);
 	delta = timespec_to_jiffies(&tsdelta);
+
+	/*
+	 * Limit the delta to the max value, which is checked in
+	 * tick_nohz_stop_sched_tick():
+	 */
+	if (delta > NEXT_TIMER_MAX_DELTA)
+		delta = NEXT_TIMER_MAX_DELTA;
+
 	/*
 	 * Take rounding errors in to account and make sure, that it
 	 * expires in the next tick. Otherwise we go into an endless

Re: [PATCH] NOHZ: prevent multiplication overflow - stop timer for huge timeouts

[David Miller]

From: Thomas Gleixner <tglx@linutronix.de>
Date: Tue, 29 May 2007 23:47:39 +0200

> get_next_timer_interrupt() returns a delta of (LONG_MAX > 1) in case
> there is no timer pending. On 64 bit machines this results in a
> multiplication overflow in tick_nohz_stop_sched_tick(). 
> 
> Reported by: Dave Miller <davem@davemloft.net>
> 
> Make the return value a constant and limit the return value to a 32 bit
> value.
> 
> When the max timeout value is returned, we can safely stop the tick
> timer device. The max jiffies delta results in a 12 days timeout for
> HZ=1000.
> 
> In the long term the get_next_timer_interrupt() code needs to be
> reworked to return ktime instead of jiffies, but we have to wait until
> the last users of the original NO_IDLE_HZ code are converted.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-off-by: David S. Miller <davem@davemloft.net>

Re: [3/4] 2.6.22-rc3: known regressions

[Linus Torvalds]

On Tue, 29 May 2007, Michal Piotrowski wrote:
> 
> Subject    : hrtimer overflow bug on 64-bit systems
> References : http://lkml.org/lkml/2007/5/24/391
> Submitter  : David Miller <davem@davemloft.net>
> Status     : problem is being debugged

Should be fixed by commit eaad084bb.

> TTY
> 
> Subject    : tty-related oops in latest kernel(s)
> References : http://lkml.org/lkml/2007/5/27/104
> Submitter  : Tero Roponen <teanropo@jyu.fi>
> Status     : problem is being debugged

People seem to have debugged this to neofb palette handling, but I haven't 
seen a patch. Antonino?

		Linus

Re: [3/4] 2.6.22-rc3: known regressions

[Antonino A. Daplas]

On Wed, 2007-05-30 at 19:33 -0700, Linus Torvalds wrote:
> 
> On Tue, 29 May 2007, Michal Piotrowski wrote:
> > 

> > TTY
> > 
> > Subject    : tty-related oops in latest kernel(s)
> > References : http://lkml.org/lkml/2007/5/27/104
> > Submitter  : Tero Roponen <teanropo@jyu.fi>
> > Status     : problem is being debugged
> 
> People seem to have debugged this to neofb palette handling, but I haven't 
> seen a patch. Antonino?
> 

Already posted one for testing. I'm waiting for Tero to confirm.

Tony

Re: [3/4] 2.6.22-rc3: known regressions

[Tero Roponen]

On Thu, 31 May 2007, Antonino A. Daplas wrote:

> On Wed, 2007-05-30 at 19:33 -0700, Linus Torvalds wrote:
> > 
> > On Tue, 29 May 2007, Michal Piotrowski wrote:
> > > 
> 
> > > TTY
> > > 
> > > Subject    : tty-related oops in latest kernel(s)
> > > References : http://lkml.org/lkml/2007/5/27/104
> > > Submitter  : Tero Roponen <teanropo@jyu.fi>
> > > Status     : problem is being debugged
> > 
> > People seem to have debugged this to neofb palette handling, but I haven't 
> > seen a patch. Antonino?
> > 
> 
> Already posted one for testing. I'm waiting for Tero to confirm.
> 
> Tony
> 

Ok, I tested all the cases I have reported: no corruption
and nothing in slabinfo -v. This seems to be the right fix.

Thanks.

Acked-by: Tero Roponen <teanropo@jyu.fi>

[PATCH] neofb: Fix pseudo_palette array overrun in neofb_setcolreg

[Antonino A. Daplas]

The pseudo_palette has room for 16 entries only, but in truecolor mode, it
attempts to write 256.

Signed-off-by: Antonino Daplas <adaplas@gmail.com>
Acked-by: Tero Roponen <teanropo@jyu.fi>
---

Tero Roponen wrote:
> On Thu, 31 May 2007, Antonino A. Daplas wrote:
> 
>> On Wed, 2007-05-30 at 19:33 -0700, Linus Torvalds wrote:
>>> On Tue, 29 May 2007, Michal Piotrowski wrote:
>>>> TTY
>>>>
>>>> Subject    : tty-related oops in latest kernel(s)
>>>> References : http://lkml.org/lkml/2007/5/27/104
>>>> Submitter  : Tero Roponen <teanropo@jyu.fi>
>>>> Status     : problem is being debugged
>>> People seem to have debugged this to neofb palette handling, but I haven't 
>>> seen a patch. Antonino?
>>>
>> Already posted one for testing. I'm waiting for Tero to confirm.
>>
>> Tony
>>
> 
> Ok, I tested all the cases I have reported: no corruption
> and nothing in slabinfo -v. This seems to be the right fix.
> 

Okay, thanks for testing.

Tony

 drivers/video/neofb.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/video/neofb.c b/drivers/video/neofb.c
index bd30aba..731d7a5 100644
--- a/drivers/video/neofb.c
+++ b/drivers/video/neofb.c
@@ -1286,34 +1286,36 @@ static int neofb_setcolreg(u_int regno, 
 	if (regno >= fb->cmap.len || regno > 255)
 		return -EINVAL;
 
-	switch (fb->var.bits_per_pixel) {
-	case 8:
+	if (fb->var.bits_per_pixel <= 8) {
 		outb(regno, 0x3c8);
 
 		outb(red >> 10, 0x3c9);
 		outb(green >> 10, 0x3c9);
 		outb(blue >> 10, 0x3c9);
-		break;
-	case 16:
-		((u32 *) fb->pseudo_palette)[regno] =
+	} else if (regno < 16) {
+		switch (fb->var.bits_per_pixel) {
+		case 16:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((red & 0xf800)) | ((green & 0xfc00) >> 5) |
 				((blue & 0xf800) >> 11);
-		break;
-	case 24:
-		((u32 *) fb->pseudo_palette)[regno] =
+			break;
+		case 24:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((red & 0xff00) << 8) | ((green & 0xff00)) |
 				((blue & 0xff00) >> 8);
-		break;
+			break;
 #ifdef NO_32BIT_SUPPORT_YET
-	case 32:
-		((u32 *) fb->pseudo_palette)[regno] =
+		case 32:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((transp & 0xff00) << 16) | ((red & 0xff00) << 8) |
 				((green & 0xff00)) | ((blue & 0xff00) >> 8);
-		break;
+			break;
 #endif
-	default:
-		return 1;
+		default:
+			return 1;
+		}
 	}
+
 	return 0;
 }
 

[PATCH] neofb: Fix pseudo_palette array overrun in neofb_setcolreg

[Antonino A. Daplas]

The pseudo_palette has room for 16 entries only, but in truecolor mode, it
attempts to write 256.

Signed-off-by: Antonino Daplas <adaplas@gmail.com>
Acked-by: Tero Roponen <teanropo@jyu.fi>
---
This fixes the following regression/bug reported as follows:

Subject    : tty-related oops in latest kernel(s)
References : http://lkml.org/lkml/2007/5/27/104
Submitter  : Tero Roponen <teanropo@jyu.fi>
Status     : problem is being debugged

According to Tero, this is also reproducible with 2.6.21.3.

Tony

 drivers/video/neofb.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/video/neofb.c b/drivers/video/neofb.c
index bd30aba..731d7a5 100644
--- a/drivers/video/neofb.c
+++ b/drivers/video/neofb.c
@@ -1286,34 +1286,36 @@ static int neofb_setcolreg(u_int regno, 
 	if (regno >= fb->cmap.len || regno > 255)
 		return -EINVAL;
 
-	switch (fb->var.bits_per_pixel) {
-	case 8:
+	if (fb->var.bits_per_pixel <= 8) {
 		outb(regno, 0x3c8);
 
 		outb(red >> 10, 0x3c9);
 		outb(green >> 10, 0x3c9);
 		outb(blue >> 10, 0x3c9);
-		break;
-	case 16:
-		((u32 *) fb->pseudo_palette)[regno] =
+	} else if (regno < 16) {
+		switch (fb->var.bits_per_pixel) {
+		case 16:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((red & 0xf800)) | ((green & 0xfc00) >> 5) |
 				((blue & 0xf800) >> 11);
-		break;
-	case 24:
-		((u32 *) fb->pseudo_palette)[regno] =
+			break;
+		case 24:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((red & 0xff00) << 8) | ((green & 0xff00)) |
 				((blue & 0xff00) >> 8);
-		break;
+			break;
 #ifdef NO_32BIT_SUPPORT_YET
-	case 32:
-		((u32 *) fb->pseudo_palette)[regno] =
+		case 32:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((transp & 0xff00) << 16) | ((red & 0xff00) << 8) |
 				((green & 0xff00)) | ((blue & 0xff00) >> 8);
-		break;
+			break;
 #endif
-	default:
-		return 1;
+		default:
+			return 1;
+		}
 	}
+
 	return 0;
 }
 

[PATCH] [RESEND] neofb: Fix pseudo_palette array overrun in neofb_setcolreg

[Antonino A. Daplas]

The pseudo_palette has room for 16 entries only, but in truecolor mode, it
attempts to write 256.

Signed-off-by: Antonino Daplas <adaplas@gmail.com>
Acked-by: Tero Roponen <teanropo@jyu.fi>
---
This fixes the following regression/bug reported as follows:

Subject    : tty-related oops in latest kernel(s)
References : http://lkml.org/lkml/2007/5/27/104
Submitter  : Tero Roponen <teanropo@jyu.fi>
Status     : problem is being debugged

According to Tero, this is also reproducible with 2.6.21.3.

(Resending, wrong email address for stable@kernel.org)

Tony

 drivers/video/neofb.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/video/neofb.c b/drivers/video/neofb.c
index bd30aba..731d7a5 100644
--- a/drivers/video/neofb.c
+++ b/drivers/video/neofb.c
@@ -1286,34 +1286,36 @@ static int neofb_setcolreg(u_int regno, 
 	if (regno >= fb->cmap.len || regno > 255)
 		return -EINVAL;
 
-	switch (fb->var.bits_per_pixel) {
-	case 8:
+	if (fb->var.bits_per_pixel <= 8) {
 		outb(regno, 0x3c8);
 
 		outb(red >> 10, 0x3c9);
 		outb(green >> 10, 0x3c9);
 		outb(blue >> 10, 0x3c9);
-		break;
-	case 16:
-		((u32 *) fb->pseudo_palette)[regno] =
+	} else if (regno < 16) {
+		switch (fb->var.bits_per_pixel) {
+		case 16:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((red & 0xf800)) | ((green & 0xfc00) >> 5) |
 				((blue & 0xf800) >> 11);
-		break;
-	case 24:
-		((u32 *) fb->pseudo_palette)[regno] =
+			break;
+		case 24:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((red & 0xff00) << 8) | ((green & 0xff00)) |
 				((blue & 0xff00) >> 8);
-		break;
+			break;
 #ifdef NO_32BIT_SUPPORT_YET
-	case 32:
-		((u32 *) fb->pseudo_palette)[regno] =
+		case 32:
+			((u32 *) fb->pseudo_palette)[regno] =
 				((transp & 0xff00) << 16) | ((red & 0xff00) << 8) |
 				((green & 0xff00)) | ((blue & 0xff00) >> 8);
-		break;
+			break;
 #endif
-	default:
-		return 1;
+		default:
+			return 1;
+		}
 	}
+
 	return 0;
 }
 

Re: [4/4] 2.6.22-rc3: known regressions

[Michal Piotrowski]

Hi all,

Here is a list of some known regressions in 2.6.22-rc3.

Feel free to add new regressions/remove fixed etc.
http://kernelnewbies.org/known_regressions



V4L

Subject    : V4L ABI breakage
References : http://lkml.org/lkml/2007/5/14/42
Submitter  : Robert Fitzsimons <robfitz@273k.net>
Caused-By  : Hans Verkuil <hverkuil@xs4all.nl>
             Mauro Carvalho Chehab <mchehab@infradead.org>
             commit 206ebaf32795cf1582b1e2ff2ec6a560c9e986b8
Workaround : http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=15ac2d293b08b6975c6ca1a80eb839d4cb0dddbf
Status     : problem is being debugged


x86-64

Subject    : x86-64 2.6.22-rc2 random segfaults
References : http://lkml.org/lkml/2007/5/24/275
Submitter  : Ioan Ionita <opslynx@gmail.com>
Status     : Unknown



Regards,
Michal

--
"Najbardziej brakowało mi twojego milczenia."
-- Andrzej Sapkowski "Coś więcej"

Re: [4/4] 2.6.22-rc3: known regressions

[Mauro Carvalho Chehab]

Hi Michal,

> Subject    : V4L ABI breakage
> References : http://lkml.org/lkml/2007/5/14/42
> Submitter  : Robert Fitzsimons <robfitz@273k.net>
> Caused-By  : Hans Verkuil <hverkuil@xs4all.nl>
>              Mauro Carvalho Chehab <mchehab@infradead.org>
>              commit 206ebaf32795cf1582b1e2ff2ec6a560c9e986b8
> Workaround : http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=15ac2d293b08b6975c6ca1a80eb839d4cb0dddbf
> Status     : problem is being debugged

I've just sent today a pull request to Linus with the proper fixes for
this issue. Some changes were done at the ABI to avoid the breakage,
while keeping the capability to control TV Out on ivtv driver. The
changes are being tested since Jun, 2.

The related changesets are those:

http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=987e00ba5cf667beed2b88bd1d01150334cdb6dc
http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=1f137600cacf9a2908529c7d544de82672226a98

-- 
Cheers,
Mauro

Re: [2/4] 2.6.22-rc3: known regressions

[Michal Piotrowski]

Hi all,

Here is a list of some known regressions in 2.6.22-rc3.

Feel free to add new regressions/remove fixed etc.
http://kernelnewbies.org/known_regressions



Networking

Subject    : Network card not usable - sky2
References : http://bugzilla.kernel.org/show_bug.cgi?id=8539
Submitter  : Ruben <nahoo82@terra.es>
Status     : Unknown



PCI

Subject    : Oops on 2.6.22-rc2 when unloading the cciss driver
References : http://lkml.org/lkml/2007/5/24/172
Submitter  : Mike Miller (OS Dev) <mikem@beardog.cca.cpqcorp.net>
Status     : Unknown



PCMCIA

Subject    : libata and legacy ide pcmcia failure
References : http://lkml.org/lkml/2007/5/17/305
Submitter  : Robert de Rooy <robert.de.rooy@gmail.com>
Status     : Unknown



SATA/PATA

Subject    : 22-rc3 broke the CDROM in Dell notebook
References : http://lkml.org/lkml/2007/5/27/63
Submitter  : Gregor Jasny <gjasny@googlemail.com>
Handled-By : Tejun Heo <htejun@gmail.com>
             Jeff Garzik <jeff@garzik.org>
Caused-By  : Tejun Heo <htejun@gmail.com>
             commit d4b2bab4f26345ea1803feb23ea92fbe3f6b77bc
Status     : problem is being debugged



Sparc64

Subject    : 2.6.22-rc broke X on Ultra5
References : http://lkml.org/lkml/2007/5/22/78
Submitter  : Mikael Pettersson <mikpe@it.uu.se>
Status     : Unknown



Regards,
Michal

--
"Najbardziej brakowało mi twojego milczenia."
-- Andrzej Sapkowski "Coś więcej"

Re: [2/4] 2.6.22-rc3: known regressions

[Stephen Hemminger]

On Tue, 29 May 2007 14:56:20 +0200
Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:

> Hi all,
> 
> Here is a list of some known regressions in 2.6.22-rc3.
> 
> Feel free to add new regressions/remove fixed etc.
> http://kernelnewbies.org/known_regressions
> 
> 
> 
> Networking
> 
> Subject    : Network card not usable - sky2
> References : http://bugzilla.kernel.org/show_bug.cgi?id=8539
> Submitter  : Ruben <nahoo82@terra.es>
> Status     : Unknown
> 


Not a regression. Read the bug report:
	Most recent kernel where this bug did *NOT* occur:

	It's been happening for a long time (I'd say always).



There is a regression since 2.6.22-rc1 where the b44 driver has
receive performance/interrupt lockup issues.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [2/4] 2.6.22-rc3: known regressions

[Linus Torvalds]

On Tue, 29 May 2007, Michal Piotrowski wrote:
> 
> Subject    : Oops on 2.6.22-rc2 when unloading the cciss driver
> References : http://lkml.org/lkml/2007/5/24/172
> Submitter  : Mike Miller (OS Dev) <mikem@beardog.cca.cpqcorp.net>
> Status     : Unknown

I thought this one should be fixed by commit e9ca75b53. Not so?

Mike?

			Linus

Re: [2/4] 2.6.22-rc3: known regressions

[Mike Miller (OS Dev)]

On Wed, May 30, 2007 at 07:22:14PM -0700, Linus Torvalds wrote:
> 
> 
> On Tue, 29 May 2007, Michal Piotrowski wrote:
> > 
> > Subject    : Oops on 2.6.22-rc2 when unloading the cciss driver
> > References : http://lkml.org/lkml/2007/5/24/172
> > Submitter  : Mike Miller (OS Dev) <mikem@beardog.cca.cpqcorp.net>
> > Status     : Unknown
> 
> I thought this one should be fixed by commit e9ca75b53. Not so?
> 
> Mike?
> 
> 			Linus

I apologize for the slow response. I also apologize that I don't know enough
about git to figure out what commit e9ca75b53 does. I submitted a fix that
was blessed by Eric B. that fixed that Oops. 

mikem

Re: [2/4] 2.6.22-rc3: known regressions

[Linus Torvalds]

On Thu, 31 May 2007, Mike Miller (OS Dev) wrote:

> On Wed, May 30, 2007 at 07:22:14PM -0700, Linus Torvalds wrote:
> > 
> > 
> > On Tue, 29 May 2007, Michal Piotrowski wrote:
> > > 
> > > Subject    : Oops on 2.6.22-rc2 when unloading the cciss driver
> > > References : http://lkml.org/lkml/2007/5/24/172
> > > Submitter  : Mike Miller (OS Dev) <mikem@beardog.cca.cpqcorp.net>
> > > Status     : Unknown
> > 
> > I thought this one should be fixed by commit e9ca75b53. Not so?
> 
> I apologize for the slow response. I also apologize that I don't know enough
> about git to figure out what commit e9ca75b53 does.

Even without git, you can use the kernel.org gitweb install:

	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e9ca75b53

(where the "h=...." is the magic part - pick any commit SHA1 you want, 
including short-hand ones like the one I gave).

With git, you just do

	git show e9ca75b53

to see a particular named commit.

> I submitted a fix that was blessed by Eric B. that fixed that Oops.

Ok, I don't think I have anything like that. The one I pointed to is the 
one by Gerald Britton, acked by you ..

But I now realize that that commit was already in -rc2. In fact, it's just 
before the -rc2 release. So while it claims to fix one oops on shutdown, 
it may be the _cause_ of the oops on mudule unload.

		Linus

Re: [2/4] 2.6.22-rc3: known regressions

[Mike Miller (OS Dev)]

On Thu, May 31, 2007 at 03:22:07PM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 31 May 2007, Mike Miller (OS Dev) wrote:
> 
> > On Wed, May 30, 2007 at 07:22:14PM -0700, Linus Torvalds wrote:
> > > 
> > > 
> > > On Tue, 29 May 2007, Michal Piotrowski wrote:
> > > > 
> > > > Subject    : Oops on 2.6.22-rc2 when unloading the cciss driver
> > > > References : http://lkml.org/lkml/2007/5/24/172
> > > > Submitter  : Mike Miller (OS Dev) <mikem@beardog.cca.cpqcorp.net>
> > > > Status     : Unknown
> > > 
> > > I thought this one should be fixed by commit e9ca75b53. Not so?
> > 
> > I apologize for the slow response. I also apologize that I don't know enough
> > about git to figure out what commit e9ca75b53 does.
> 
> Even without git, you can use the kernel.org gitweb install:
> 
> 	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e9ca75b53
> 
> (where the "h=...." is the magic part - pick any commit SHA1 you want, 
> including short-hand ones like the one I gave).
> 
> With git, you just do
> 
> 	git show e9ca75b53
> 
> to see a particular named commit.
> 
> > I submitted a fix that was blessed by Eric B. that fixed that Oops.
> 
> Ok, I don't think I have anything like that. The one I pointed to is the 
> one by Gerald Britton, acked by you ..
> 
> But I now realize that that commit was already in -rc2. In fact, it's just 
> before the -rc2 release. So while it claims to fix one oops on shutdown, 
> it may be the _cause_ of the oops on mudule unload.
> 
> 		Linus
Linus,
The fix from Gerald was a different Oops and is not related to this problem.
This is the patch I submitted for the rmmod Oops:

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index e01380b..6632150 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -558,12 +558,12 @@ static int msi_free_irqs(struct pci_dev* dev)

        list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
                if (entry->msi_attrib.type == PCI_CAP_ID_MSIX) {
-                       if (list_is_last(&entry->list, &dev->msi_list))
-                               iounmap(entry->mask_base);
-
                        writel(1, entry->mask_base + entry->msi_attrib.entry_nr
                                  * PCI_MSIX_ENTRY_SIZE
                                  + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
+
+                       if (list_is_last(&entry->list, &dev->msi_list))
+                               iounmap(entry->mask_base);
                }
                list_del(&entry->list);
                kfree(entry);

Reference:
http://groups.google.com/group/linux.kernel/browse_frm/thread/ed0949e9d42cfdef/5953daaa00ea5bf7?lnk=gst&q=cciss&rnum=3&hl=en#5953daaa00ea5bf7

I'm not sure what the status is right now.

Thanks,
mikem

Re: [2/4] 2.6.22-rc3: known regressions

[Andrew Morton]

On Thu, 31 May 2007 17:39:08 -0500
"Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote:

> On Thu, May 31, 2007 at 03:22:07PM -0700, Linus Torvalds wrote:
> > 
> > 
> > On Thu, 31 May 2007, Mike Miller (OS Dev) wrote:
> > 
> > > On Wed, May 30, 2007 at 07:22:14PM -0700, Linus Torvalds wrote:
> > > > 
> > > > 
> > > > On Tue, 29 May 2007, Michal Piotrowski wrote:
> > > > > 
> > > > > Subject    : Oops on 2.6.22-rc2 when unloading the cciss driver
> > > > > References : http://lkml.org/lkml/2007/5/24/172
> > > > > Submitter  : Mike Miller (OS Dev) <mikem@beardog.cca.cpqcorp.net>
> > > > > Status     : Unknown
> > > > 
> > > > I thought this one should be fixed by commit e9ca75b53. Not so?
> > > 
> > > I apologize for the slow response. I also apologize that I don't know enough
> > > about git to figure out what commit e9ca75b53 does.
> > 
> > Even without git, you can use the kernel.org gitweb install:
> > 
> > 	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e9ca75b53
> > 
> > (where the "h=...." is the magic part - pick any commit SHA1 you want, 
> > including short-hand ones like the one I gave).
> > 
> > With git, you just do
> > 
> > 	git show e9ca75b53
> > 
> > to see a particular named commit.
> > 
> > > I submitted a fix that was blessed by Eric B. that fixed that Oops.
> > 
> > Ok, I don't think I have anything like that. The one I pointed to is the 
> > one by Gerald Britton, acked by you ..
> > 
> > But I now realize that that commit was already in -rc2. In fact, it's just 
> > before the -rc2 release. So while it claims to fix one oops on shutdown, 
> > it may be the _cause_ of the oops on mudule unload.
> > 
> > 		Linus
> Linus,
> The fix from Gerald was a different Oops and is not related to this problem.
> This is the patch I submitted for the rmmod Oops:
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index e01380b..6632150 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -558,12 +558,12 @@ static int msi_free_irqs(struct pci_dev* dev)
> 
>         list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
>                 if (entry->msi_attrib.type == PCI_CAP_ID_MSIX) {
> -                       if (list_is_last(&entry->list, &dev->msi_list))
> -                               iounmap(entry->mask_base);
> -
>                         writel(1, entry->mask_base + entry->msi_attrib.entry_nr
>                                   * PCI_MSIX_ENTRY_SIZE
>                                   + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> +
> +                       if (list_is_last(&entry->list, &dev->msi_list))
> +                               iounmap(entry->mask_base);
>                 }
>                 list_del(&entry->list);
>                 kfree(entry);
> 
> Reference:
> http://groups.google.com/group/linux.kernel/browse_frm/thread/ed0949e9d42cfdef/5953daaa00ea5bf7?lnk=gst&q=cciss&rnum=3&hl=en#5953daaa00ea5bf7
> 
> I'm not sure what the status is right now.
> 

The status is "sitting in my queue for 2.6.22".  I'll be sending it up
today or tomorrow.  Was hoping to get an ack from Greg &/| Andi on it, but
those are not easy to come by.

Re: [2/4] 2.6.22-rc3: known regressions

[Linus Torvalds]

On Thu, 31 May 2007, Andrew Morton wrote:
> 
> The status is "sitting in my queue for 2.6.22".  I'll be sending it up
> today or tomorrow.  Was hoping to get an ack from Greg &/| Andi on it, but
> those are not easy to come by.

Well, it looks "Obviously Correct(tm)", since clearly the iounmap() should 
be done _after_ the last use. So I don't think it needs any more acking.

But tomorrow is fine..

		Linus

Re: [2/4] 2.6.22-rc3: known regressions

[Roland Dreier]

What about the changes to fix the order that MSI-X irqs are returned
in (iirc, list_add had to be changed to list_add_tail in a couple of
places).  Without that change, multiple MSI-X interrupts seem to be
broken: the kernel programs the MSI-X table in the opposite order that
it gives the irq numbers to the driver.  The net effect is that if I
request, say, 3 MSI-X interrupts for a device, then when the device
generates the first interrupt, the driver thinks it generated the
third interrupt, and things go fairly haywire.

 - R.

Re: [2/4] 2.6.22-rc3: known regressions

[Andrew Morton]

On Thu, 31 May 2007 16:17:00 -0700
Roland Dreier <rdreier@cisco.com> wrote:

> What about the changes to fix the order that MSI-X irqs are returned
> in (iirc, list_add had to be changed to list_add_tail in a couple of
> places).  Without that change, multiple MSI-X interrupts seem to be
> broken: the kernel programs the MSI-X table in the opposite order that
> it gives the irq numbers to the driver.  The net effect is that if I
> request, say, 3 MSI-X interrupts for a device, then when the device
> generates the first interrupt, the driver thinks it generated the
> third interrupt, and things go fairly haywire.
> 

That's msi-fix-the-ordering-of-msix-irqs.patch, which is also queued for
the next batch.


"next batch" == around 40 patches atm.  Some of these need maintainer acks
so they will be further delayed, or more likely just merged without the
appropriate ack.



From: "Eric W. Biederman" <ebiederm@xmission.com>

"Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> writes:

Found what seems the problem with our vectors being listed backward.  In
drivers/pci/msi.c we should be using list_add_tail rather than list_add to
preserve the ordering across various kernels.  Please consider this for
inclusion.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Screwed-up-by: Michael Ellerman <michael@ellerman.id.au>
Cc: "Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net>
Cc: Andi Kleen <ak@suse.de>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/pci/msi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -puN drivers/pci/msi.c~msi-fix-the-ordering-of-msix-irqs drivers/pci/msi.c
--- a/drivers/pci/msi.c~msi-fix-the-ordering-of-msix-irqs
+++ a/drivers/pci/msi.c
@@ -333,7 +333,7 @@ static int msi_capability_init(struct pc
 			msi_mask_bits_reg(pos, is_64bit_address(control)),
 			maskbits);
 	}
-	list_add(&entry->list, &dev->msi_list);
+	list_add_tail(&entry->list, &dev->msi_list);
 
 	/* Configure MSI capability structure */
 	ret = arch_setup_msi_irqs(dev, 1, PCI_CAP_ID_MSI);
@@ -404,7 +404,7 @@ static int msix_capability_init(struct p
 		entry->dev = dev;
 		entry->mask_base = base;
 
-		list_add(&entry->list, &dev->msi_list);
+		list_add_tail(&entry->list, &dev->msi_list);
 	}
 
 	ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
_