[1/2] 2.6.23-rc1: known regressions

[1/2] 2.6.23-rc1: known regressions

[Michal Piotrowski]

Hi all,

Here is a list of some known regressions in 2.6.23-rc1.

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

List of Aces

Name                    Regressions fixed since 21-Jun-2007
Andi Kleen                             4
Linus Torvalds                         4
Adrian Bunk                            3
Andrew Morton                          3
Jens Axboe                             3
Al Viro                                2
David Woodhouse                        2
Hugh Dickins                           2
Tejun Heo                              2



Unclassified

Subject         : /usr/bin/ld: section .text [ffffffffff700500 -> ffffffffff7007e3] overlaps section .gnu.version_d [ffffffffff7004d8 -> ffffffffff70050f]
References      : http://lkml.org/lkml/2007/7/22/239
Last known good : ?
Submitter       : Andre Noll <maan@systemlinux.org>
Caused-By       : ?
Handled-By      : Andi Kleen <ak@suse.de>
Status          : problem is being debugged

Subject         : pcwd_init_module(): WARNING: at lib/kref.c:33 kref_get()
References      : http://lkml.org/lkml/2007/7/22/94
Last known good : ?
Submitter       : Ingo Molnar <mingo@elte.hu>
Caused-By       : ?
Handled-By      : ?
Status          : problem is being debugged

Subject         : konqueror suddenly vanishing, "konqueror: Fatal IO error: client killed"
References      : http://lkml.org/lkml/2007/7/22/86
Last known good : ?
Submitter       : Markus <lists4me@web.de>
Caused-By       : ?
Handled-By      : Ingo Molnar <mingo@elte.hu>
Status          : problem is being debugged



ACPI

Subject         : drivers/misc/asus-laptop.c:*: error: 'struct led_classdev' has no member named 'class_dev'
References      : http://lkml.org/lkml/2007/7/22/299
Last known good : ?
Submitter       : Gabriel C <nix.or.die@googlemail.com>
Caused-By       : ?
Handled-By      : ?
Status          : unknown



Networking

Subject         : New wake ups from sky2
References      : http://lkml.org/lkml/2007/7/20/386
Last known good : ?
Submitter       : Thomas Meyer <thomas@m3y3r.de>
Caused-By       : Stephen Hemminger <shemminger@osdl.org>
                  commit eb35cf60e462491249166182e3e755d3d5d91a28
Handled-By      : Stephen Hemminger <shemminger@osdl.org>
Status          : unknown



Regards,
Michal

--
LOG
http://www.stardust.webpages.pl/log/

Re: [1/2] 2.6.23-rc1: known regressions

[Al Viro]

On Mon, Jul 23, 2007 at 11:47:30AM +0200, Michal Piotrowski wrote:
> Subject         : drivers/misc/asus-laptop.c:*: error: 'struct led_classdev' has no member named 'class_dev'
> References      : http://lkml.org/lkml/2007/7/22/299
> Last known good : ?
> Submitter       : Gabriel C <nix.or.die@googlemail.com>
> Caused-By       : ?
> Handled-By      : ?
> Status          : unknown

>From 2a7e1148a9d3ee860dc2650c9a45288b120e250f Mon Sep 17 00:00:00 2001
From: Al Viro <viro@zeniv.linux.org.uk>
Date: Mon, 23 Jul 2007 06:20:22 -0400
Subject: [PATCH] Fix failure exits in asus-laptop

Fallout from f8a7c6fe14f556ca8eeddce258cb21392d0c3a2f.  However, looking
at it shows that checks done in ASUS_LED_UNREGISTER() can't trigger
at all (we never get to asus_led_exit() if registration fails) and
if that registration fails, we actually leak stuff.  IOW, it's worse
than just replacing class_dev with dev in there - the tests themselves
had been papering over the lousy cleanup logics.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/misc/asus-laptop.c |   32 +++++++++++++++++++++-----------
 1 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index f753060..6b89854 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -1067,19 +1067,16 @@ static void asus_backlight_exit(void)
 }
 
 #define  ASUS_LED_UNREGISTER(object)				\
-	if(object##_led.class_dev				\
-	   && !IS_ERR(object##_led.class_dev))			\
-		led_classdev_unregister(&object##_led)
+	led_classdev_unregister(&object##_led)
 
 static void asus_led_exit(void)
 {
+	destroy_workqueue(led_workqueue);
 	ASUS_LED_UNREGISTER(mled);
 	ASUS_LED_UNREGISTER(tled);
 	ASUS_LED_UNREGISTER(pled);
 	ASUS_LED_UNREGISTER(rled);
 	ASUS_LED_UNREGISTER(gled);
-
-	destroy_workqueue(led_workqueue);
 }
 
 static void __exit asus_laptop_exit(void)
@@ -1135,29 +1132,42 @@ static int asus_led_init(struct device *dev)
 
 	rv = ASUS_LED_REGISTER(mled, dev);
 	if (rv)
-		return rv;
+		goto out;
 
 	rv = ASUS_LED_REGISTER(tled, dev);
 	if (rv)
-		return rv;
+		goto out1;
 
 	rv = ASUS_LED_REGISTER(rled, dev);
 	if (rv)
-		return rv;
+		goto out2;
 
 	rv = ASUS_LED_REGISTER(pled, dev);
 	if (rv)
-		return rv;
+		goto out3;
 
 	rv = ASUS_LED_REGISTER(gled, dev);
 	if (rv)
-		return rv;
+		goto out4;
 
 	led_workqueue = create_singlethread_workqueue("led_workqueue");
 	if (!led_workqueue)
-		return -ENOMEM;
+		goto out5;
 
 	return 0;
+out5:
+	rv = -ENOMEM;
+	ASUS_LED_UNREGISTER(gled);
+out4:
+	ASUS_LED_UNREGISTER(pled);
+out3:
+	ASUS_LED_UNREGISTER(rled);
+out2:
+	ASUS_LED_UNREGISTER(tled);
+out1:
+	ASUS_LED_UNREGISTER(mled);
+out:
+	return rv;
 }
 
 static int __init asus_laptop_init(void)
-- 
1.5.3.GIT

Re: [1/2] 2.6.23-rc1: known regressions

[Trent Piepho]

On Mon, 23 Jul 2007, Michal Piotrowski wrote:
> ACPI
>
> Subject         : drivers/misc/asus-laptop.c:*: error: 'struct led_classdev' has no member named 'class_dev'
> References      : http://lkml.org/lkml/2007/7/22/299
> Last known good : ?
> Submitter       : Gabriel C <nix.or.die@googlemail.com>
> Caused-By       : ?
> Handled-By      : ?
> Status          : unknown

Here's a trivial patch for this one.
------------------------------------------------------------------------
asus-laptop: Sync with changes to led class

Driver was broken by commit f8a7c6fe14f556ca8eeddce258cb21392d0c3a2f
	leds: Convert from struct class_device to struct device

	Convert the LEDs class from struct class_device to struct device
	since class_device is scheduled for removal.

Use (struct led_classdev).dev instead of (struct led_classdev).class_dev

diff -r 3f21143ca44c drivers/misc/asus-laptop.c
--- a/drivers/misc/asus-laptop.c	Sun Jul 22 21:30:24 2007 +0000
+++ b/drivers/misc/asus-laptop.c	Mon Jul 23 03:47:55 2007 -0700
@@ -1067,8 +1067,8 @@ static void asus_backlight_exit(void)
 }

 #define  ASUS_LED_UNREGISTER(object)				\
-	if(object##_led.class_dev				\
-	   && !IS_ERR(object##_led.class_dev))			\
+	if(object##_led.dev					\
+	   && !IS_ERR(object##_led.dev))			\
 		led_classdev_unregister(&object##_led)

 static void asus_led_exit(void)

Re: [1/2] 2.6.23-rc1: known regressions

[Al Viro]

On Mon, Jul 23, 2007 at 04:17:05AM -0700, Trent Piepho wrote:
 
> Here's a trivial patch for this one.
> ------------------------------------------------------------------------
> asus-laptop: Sync with changes to led class
> 
> Driver was broken by commit f8a7c6fe14f556ca8eeddce258cb21392d0c3a2f
> 	leds: Convert from struct class_device to struct device
> 
> 	Convert the LEDs class from struct class_device to struct device
> 	since class_device is scheduled for removal.
> 
> Use (struct led_classdev).dev instead of (struct led_classdev).class_dev

It doesn't fix the real bug in there - if you look carefully at the code,
you'll see that we don't get to these checks if allocation fails halfway
through (and we leak in that case) *and* these checks are not needed at
all if failure happens elsewhere.

Re: [1/2] 2.6.23-rc1: known regressions

[Richard Purdie]

On Mon, 2007-07-23 at 12:25 +0100, Al Viro wrote:
> On Mon, Jul 23, 2007 at 04:17:05AM -0700, Trent Piepho wrote:
>  
> > Here's a trivial patch for this one.
> > ------------------------------------------------------------------------
> > asus-laptop: Sync with changes to led class
> > 
> > Driver was broken by commit f8a7c6fe14f556ca8eeddce258cb21392d0c3a2f
> > 	leds: Convert from struct class_device to struct device
> > 
> > 	Convert the LEDs class from struct class_device to struct device
> > 	since class_device is scheduled for removal.
> > 
> > Use (struct led_classdev).dev instead of (struct led_classdev).class_dev
> 
> It doesn't fix the real bug in there - if you look carefully at the code,
> you'll see that we don't get to these checks if allocation fails halfway
> through (and we leak in that case) *and* these checks are not needed at
> all if failure happens elsewhere.

leds: Allow led_classdev_unregister() to ignore unregistered drivers

Allow led_classdev_unregister() to return for devices that were already
unregistered, never registered or failed to register to simplify
drivers. 

Fix error leftover from LED class struct device conversion (after the
above, drivers don't need to look at the device structure).

Fix the leak where asus-led registration fails half way through.

Signed-off-by: Richard Purdie <rpurdie@rpsys.net>

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 4211293..68d26b8 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -95,8 +95,10 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 
 	led_cdev->dev = device_create(leds_class, parent, 0, "%s",
 					    led_cdev->name);
-	if (unlikely(IS_ERR(led_cdev->dev)))
-		return PTR_ERR(led_cdev->dev);
+	if (unlikely(IS_ERR(led_cdev->dev))) {
+		rc = PTR_ERR(led_cdev->dev);
+		goto create_err;
+	}
 
 	dev_set_drvdata(led_cdev->dev, led_cdev);
 
@@ -132,6 +134,8 @@ err_out_led_list:
 #endif
 err_out:
 	device_unregister(led_cdev->dev);
+create_err:
+	led_cdev->dev = NULL;
 	return rc;
 }
 EXPORT_SYMBOL_GPL(led_classdev_register);
@@ -144,6 +148,9 @@ EXPORT_SYMBOL_GPL(led_classdev_register);
  */
 void led_classdev_unregister(struct led_classdev *led_cdev)
 {
+	if (!led_cdev->dev)
+		return;
+
 	device_remove_file(led_cdev->dev, &dev_attr_brightness);
 #ifdef CONFIG_LEDS_TRIGGERS
 	device_remove_file(led_cdev->dev, &dev_attr_trigger);
diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index f753060..2bc4f6e 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -1066,18 +1066,13 @@ static void asus_backlight_exit(void)
 		backlight_device_unregister(asus_backlight_device);
 }
 
-#define  ASUS_LED_UNREGISTER(object)				\
-	if(object##_led.class_dev				\
-	   && !IS_ERR(object##_led.class_dev))			\
-		led_classdev_unregister(&object##_led)
-
 static void asus_led_exit(void)
 {
-	ASUS_LED_UNREGISTER(mled);
-	ASUS_LED_UNREGISTER(tled);
-	ASUS_LED_UNREGISTER(pled);
-	ASUS_LED_UNREGISTER(rled);
-	ASUS_LED_UNREGISTER(gled);
+	led_classdev_unregister(&mled_led);
+	led_classdev_unregister(&tled_led);
+	led_classdev_unregister(&pled_led);
+	led_classdev_unregister(&rled_led);
+	led_classdev_unregister(&gled_led);
 
 	destroy_workqueue(led_workqueue);
 }
@@ -1226,9 +1221,8 @@ static int __init asus_laptop_init(void)
 	platform_driver_unregister(&asuspf_driver);
 
       fail_platform_driver:
-	asus_led_exit();
-
       fail_led:
+	asus_led_exit();
 	asus_backlight_exit();
 
       fail_backlight:

Re: [1/2] 2.6.23-rc1: known regressions

[Al Viro]

On Mon, Jul 23, 2007 at 02:12:07PM +0100, Richard Purdie wrote:
> Fix the leak where asus-led registration fails half way through.
 
Frankly, I don't think that any post-factum checks are needed.  We
should just bail out at the point of failure.  See the patch posted
earlier...  BTW, your variant does destroy_workqueue() when no
create_singlethreaded_workqueue() had been called.  FWIW, here's
the patch I'm talking about:

diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index f753060..6b89854 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -1067,19 +1067,16 @@ static void asus_backlight_exit(void)
 }
 
 #define  ASUS_LED_UNREGISTER(object)				\
-	if(object##_led.class_dev				\
-	   && !IS_ERR(object##_led.class_dev))			\
-		led_classdev_unregister(&object##_led)
+	led_classdev_unregister(&object##_led)
 
 static void asus_led_exit(void)
 {
+	destroy_workqueue(led_workqueue);
 	ASUS_LED_UNREGISTER(mled);
 	ASUS_LED_UNREGISTER(tled);
 	ASUS_LED_UNREGISTER(pled);
 	ASUS_LED_UNREGISTER(rled);
 	ASUS_LED_UNREGISTER(gled);
-
-	destroy_workqueue(led_workqueue);
 }
 
 static void __exit asus_laptop_exit(void)
@@ -1135,29 +1132,42 @@ static int asus_led_init(struct device *dev)
 
 	rv = ASUS_LED_REGISTER(mled, dev);
 	if (rv)
-		return rv;
+		goto out;
 
 	rv = ASUS_LED_REGISTER(tled, dev);
 	if (rv)
-		return rv;
+		goto out1;
 
 	rv = ASUS_LED_REGISTER(rled, dev);
 	if (rv)
-		return rv;
+		goto out2;
 
 	rv = ASUS_LED_REGISTER(pled, dev);
 	if (rv)
-		return rv;
+		goto out3;
 
 	rv = ASUS_LED_REGISTER(gled, dev);
 	if (rv)
-		return rv;
+		goto out4;
 
 	led_workqueue = create_singlethread_workqueue("led_workqueue");
 	if (!led_workqueue)
-		return -ENOMEM;
+		goto out5;
 
 	return 0;
+out5:
+	rv = -ENOMEM;
+	ASUS_LED_UNREGISTER(gled);
+out4:
+	ASUS_LED_UNREGISTER(pled);
+out3:
+	ASUS_LED_UNREGISTER(rled);
+out2:
+	ASUS_LED_UNREGISTER(tled);
+out1:
+	ASUS_LED_UNREGISTER(mled);
+out:
+	return rv;
 }
 
 static int __init asus_laptop_init(void)
-- 
1.5.3.GIT

Re: [1/2] 2.6.23-rc1: known regressions

[Richard Purdie]

On Mon, 2007-07-23 at 14:36 +0100, Al Viro wrote:
> On Mon, Jul 23, 2007 at 02:12:07PM +0100, Richard Purdie wrote:
> > Fix the leak where asus-led registration fails half way through.
>  
> Frankly, I don't think that any post-factum checks are needed.  We
> should just bail out at the point of failure.  See the patch posted
> earlier...  BTW, your variant does destroy_workqueue() when no
> create_singlethreaded_workqueue() had been called.  FWIW, here's
> the patch I'm talking about:

I saw that after I'd posted mine. I don't mind which we use... 

I'm pleased the IS_ERR(object##_led.class_dev) is gone as that makes
nasty assumptions about what the LED class does internally...

Cheers,

Richard