[REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[trapDoor]

Hello,
The merge window is closed so it's time to hunt for bugs. I've got
one, please find details below.
-----------------------

Working kernels: from 2.6.35 - to 2.6.35.2
Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
-----------------------

Description of the problem(s):

1. First of all I confirm that the problem is consistent: suspend
works well every time on the 'good' kernels and it newer works (with
the symptoms) on any of the 'bad' kernels. I've tested it several
times on a couple of 'good' and 'bad' kernels to make sure it's not
some random issue.

2. On a bad kernel, this is what happens when I try to switch to suspend mode:
- screen goes blank ['No signal detected !'] - that's OK :)
- either the CPU fan or PSU fan or both are still running and they
won't stop (left it for about 30 min. once)
- the power light is on and still, where in suspend mode it should
blink - that won't change either
- pressing the power button obviously doesn't wake the system up; need
to hard reboot

3. Now, when booting again, after an unsuccessful suspend and hard
reboot, something happens to my network card (and it doesn't matter
what is the next kernel I boot from - it can be any 'good' or 'bad'
one):
- in Gnome, the gnome-panel network indicator shows my connection as
disabled (wired eth0 - the only one I use); enabling it doesn't bring
my network up

4. After re-booting from any of the 'good' or 'bad' kernels my network
connection is working again.
-----------------------

Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
results in a system that works.

commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Sun Jul 11 15:34:05 2010 +0200

    HID: hiddev: use usb_find_interface, get rid of BKL

    This removes the private hiddev_table in the usbhid
    driver and changes it to use usb_find_interface
    instead.

    The advantage is that we can avoid the race between
    usb_register_dev and usb_open and no longer need the
    big kernel lock.

    This doesn't introduce race condition -- the intf pointer could be
    invalidated only in hiddev_disconnect() through usb_deregister_dev(),
    but that will block on minor_rwsem and not actually remove the device
    until usb_open().
-----------------------

Despite mentioned issues with network (occurring every time after
unsuccessful suspend > hard restart) the bisecting turned up a commit
which isn't rather related to network. But I can confirm that
reverting only this one commit in 2.6.36-rc1
[da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
suspend works OK and I have no issues with my network after waking the
system up, neither after re-booting from the same or any other kernel.

Please let me know what further details should I provide. Any logs,
hardware specifications? I've also made a brief log recording the
bisect if anyone would need it.

--
Regards
Tomasz B. aka trapDoor

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[trapDoor]

Posted on Bugzilla as well, ref: 16615 -  [REGRESSION, bisected]
2.6.36-rc1 - suspend doesn't work and network issues afterwards

--
Thanks
Tomasz B.


On Tue, Aug 17, 2010 at 6:01 PM, trapDoor <trapdoor6@gmail.com> wrote:
> Hello,
> The merge window is closed so it's time to hunt for bugs. I've got
> one, please find details below.
> -----------------------
>
> Working kernels: from 2.6.35 - to 2.6.35.2
> Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
> -----------------------
>
> Description of the problem(s):
>
> 1. First of all I confirm that the problem is consistent: suspend
> works well every time on the 'good' kernels and it newer works (with
> the symptoms) on any of the 'bad' kernels. I've tested it several
> times on a couple of 'good' and 'bad' kernels to make sure it's not
> some random issue.
>
> 2. On a bad kernel, this is what happens when I try to switch to suspend mode:
> - screen goes blank ['No signal detected !'] - that's OK :)
> - either the CPU fan or PSU fan or both are still running and they
> won't stop (left it for about 30 min. once)
> - the power light is on and still, where in suspend mode it should
> blink - that won't change either
> - pressing the power button obviously doesn't wake the system up; need
> to hard reboot
>
> 3. Now, when booting again, after an unsuccessful suspend and hard
> reboot, something happens to my network card (and it doesn't matter
> what is the next kernel I boot from - it can be any 'good' or 'bad'
> one):
> - in Gnome, the gnome-panel network indicator shows my connection as
> disabled (wired eth0 - the only one I use); enabling it doesn't bring
> my network up
>
> 4. After re-booting from any of the 'good' or 'bad' kernels my network
> connection is working again.
> -----------------------
>
> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
> results in a system that works.
>
> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Sun Jul 11 15:34:05 2010 +0200
>
>     HID: hiddev: use usb_find_interface, get rid of BKL
>
>     This removes the private hiddev_table in the usbhid
>     driver and changes it to use usb_find_interface
>     instead.
>
>     The advantage is that we can avoid the race between
>     usb_register_dev and usb_open and no longer need the
>     big kernel lock.
>
>     This doesn't introduce race condition -- the intf pointer could be
>     invalidated only in hiddev_disconnect() through usb_deregister_dev(),
>     but that will block on minor_rwsem and not actually remove the device
>     until usb_open().
> -----------------------
>
> Despite mentioned issues with network (occurring every time after
> unsuccessful suspend > hard restart) the bisecting turned up a commit
> which isn't rather related to network. But I can confirm that
> reverting only this one commit in 2.6.36-rc1
> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
> suspend works OK and I have no issues with my network after waking the
> system up, neither after re-booting from the same or any other kernel.
>
> Please let me know what further details should I provide. Any logs,
> hardware specifications? I've also made a brief log recording the
> bisect if anyone would need it.
>
> --
> Regards
> Tomasz B. aka trapDoor
>



-- 
Regards
trapDoor

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Andrea Gelmini]

2010/8/17 trapDoor <trapdoor6@gmail.com>:
> Hello,
> The merge window is closed so it's time to hunt for bugs. I've got
> one, please find details below.

Same problem here...
But I've made no bisecting.

Thanks a lot,
Andrea

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[trapDoor]

Thanks Andrea.
Have you tried to revert the commit to see if it fixes the issue on your end?

I also have response from Gabriel Craciunescu on bugzilla
[https://bugzilla.kernel.org/show_bug.cgi?id=16615], who had the same
problem and confirmed that reverting the same commit solved it.

--
Regards
Tomasz B.



On Tue, Aug 17, 2010 at 9:54 PM, Andrea Gelmini
<andrea.gelmini@gmail.com> wrote:
> 2010/8/17 trapDoor <trapdoor6@gmail.com>:
>> Hello,
>> The merge window is closed so it's time to hunt for bugs. I've got
>> one, please find details below.
>
> Same problem here...
> But I've made no bisecting.
>
> Thanks a lot,
> Andrea
>

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Rafael J. Wysocki]

On Tuesday, August 17, 2010, trapDoor wrote:
> Hello,
> The merge window is closed so it's time to hunt for bugs. I've got
> one, please find details below.
> -----------------------
> 
> Working kernels: from 2.6.35 - to 2.6.35.2
> Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
> -----------------------
> 
> Description of the problem(s):
> 
> 1. First of all I confirm that the problem is consistent: suspend
> works well every time on the 'good' kernels and it newer works (with
> the symptoms) on any of the 'bad' kernels. I've tested it several
> times on a couple of 'good' and 'bad' kernels to make sure it's not
> some random issue.
> 
> 2. On a bad kernel, this is what happens when I try to switch to suspend mode:
> - screen goes blank ['No signal detected !'] - that's OK :)
> - either the CPU fan or PSU fan or both are still running and they
> won't stop (left it for about 30 min. once)
> - the power light is on and still, where in suspend mode it should
> blink - that won't change either
> - pressing the power button obviously doesn't wake the system up; need
> to hard reboot
> 
> 3. Now, when booting again, after an unsuccessful suspend and hard
> reboot, something happens to my network card (and it doesn't matter
> what is the next kernel I boot from - it can be any 'good' or 'bad'
> one):
> - in Gnome, the gnome-panel network indicator shows my connection as
> disabled (wired eth0 - the only one I use); enabling it doesn't bring
> my network up
> 
> 4. After re-booting from any of the 'good' or 'bad' kernels my network
> connection is working again.
> -----------------------
> 
> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
> results in a system that works.
> 
> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Sun Jul 11 15:34:05 2010 +0200
> 
>     HID: hiddev: use usb_find_interface, get rid of BKL
> 
>     This removes the private hiddev_table in the usbhid
>     driver and changes it to use usb_find_interface
>     instead.
> 
>     The advantage is that we can avoid the race between
>     usb_register_dev and usb_open and no longer need the
>     big kernel lock.
> 
>     This doesn't introduce race condition -- the intf pointer could be
>     invalidated only in hiddev_disconnect() through usb_deregister_dev(),
>     but that will block on minor_rwsem and not actually remove the device
>     until usb_open().
> -----------------------
> 
> Despite mentioned issues with network (occurring every time after
> unsuccessful suspend > hard restart) the bisecting turned up a commit
> which isn't rather related to network. But I can confirm that
> reverting only this one commit in 2.6.36-rc1
> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
> suspend works OK and I have no issues with my network after waking the
> system up, neither after re-booting from the same or any other kernel.
> 
> Please let me know what further details should I provide. Any logs,
> hardware specifications? I've also made a brief log recording the
> bisect if anyone would need it.

Well, we should let the HID maintainer know about the problem (CC added).

Thanks,
Rafael

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[trapDoor]

On Wed, Aug 18, 2010 at 2:25 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, August 17, 2010, trapDoor wrote:
>> Hello,
>> The merge window is closed so it's time to hunt for bugs. I've got
>> one, please find details below.
>> -----------------------
>>
>> Working kernels: from 2.6.35 - to 2.6.35.2
>> Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
>> -----------------------
>>
>> Description of the problem(s):
>>
>> 1. First of all I confirm that the problem is consistent: suspend
>> works well every time on the 'good' kernels and it newer works (with
>> the symptoms) on any of the 'bad' kernels. I've tested it several
>> times on a couple of 'good' and 'bad' kernels to make sure it's not
>> some random issue.
>>
>> 2. On a bad kernel, this is what happens when I try to switch to suspend mode:
>> - screen goes blank ['No signal detected !'] - that's OK :)
>> - either the CPU fan or PSU fan or both are still running and they
>> won't stop (left it for about 30 min. once)
>> - the power light is on and still, where in suspend mode it should
>> blink - that won't change either
>> - pressing the power button obviously doesn't wake the system up; need
>> to hard reboot
>>
>> 3. Now, when booting again, after an unsuccessful suspend and hard
>> reboot, something happens to my network card (and it doesn't matter
>> what is the next kernel I boot from - it can be any 'good' or 'bad'
>> one):
>> - in Gnome, the gnome-panel network indicator shows my connection as
>> disabled (wired eth0 - the only one I use); enabling it doesn't bring
>> my network up
>>
>> 4. After re-booting from any of the 'good' or 'bad' kernels my network
>> connection is working again.
>> -----------------------
>>
>> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
>> results in a system that works.
>>
>> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
>> Author: Arnd Bergmann <arnd@arndb.de>
>> Date:   Sun Jul 11 15:34:05 2010 +0200
>>
>>     HID: hiddev: use usb_find_interface, get rid of BKL
>>
>>     This removes the private hiddev_table in the usbhid
>>     driver and changes it to use usb_find_interface
>>     instead.
>>
>>     The advantage is that we can avoid the race between
>>     usb_register_dev and usb_open and no longer need the
>>     big kernel lock.
>>
>>     This doesn't introduce race condition -- the intf pointer could be
>>     invalidated only in hiddev_disconnect() through usb_deregister_dev(),
>>     but that will block on minor_rwsem and not actually remove the device
>>     until usb_open().
>> -----------------------
>>
>> Despite mentioned issues with network (occurring every time after
>> unsuccessful suspend > hard restart) the bisecting turned up a commit
>> which isn't rather related to network. But I can confirm that
>> reverting only this one commit in 2.6.36-rc1
>> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
>> suspend works OK and I have no issues with my network after waking the
>> system up, neither after re-booting from the same or any other kernel.
>>
>> Please let me know what further details should I provide. Any logs,
>> hardware specifications? I've also made a brief log recording the
>> bisect if anyone would need it.
>
> Well, we should let the HID maintainer know about the problem (CC added).
>
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Thanks Rafael,
Before I opened this thread I only searched on
http://vger.kernel.org/vger-lists.html for any appropriate mailing
list but nothing linke linux-*hid* there. Now I've found out that
there is a list of all maintainers included in the kernel source. I'll
keep that in mind ..

-- 
Regards
Tomasz

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Jiri Kosina]

On Wed, 18 Aug 2010, trapDoor wrote:

> >> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
> >> results in a system that works.
> >>
> >> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
> >> Author: Arnd Bergmann <arnd@arndb.de>
> >> Date:   Sun Jul 11 15:34:05 2010 +0200
> >>
> >>     HID: hiddev: use usb_find_interface, get rid of BKL
> >>
> >>     This removes the private hiddev_table in the usbhid
> >>     driver and changes it to use usb_find_interface
> >>     instead.
> >>
> >>     The advantage is that we can avoid the race between
> >>     usb_register_dev and usb_open and no longer need the
> >>     big kernel lock.
> >>
> >>     This doesn't introduce race condition -- the intf pointer could be
> >>     invalidated only in hiddev_disconnect() through usb_deregister_dev(),
> >>     but that will block on minor_rwsem and not actually remove the device
> >>     until usb_open().
> >> -----------------------
> >>
> >> Despite mentioned issues with network (occurring every time after
> >> unsuccessful suspend > hard restart) the bisecting turned up a commit
> >> which isn't rather related to network. But I can confirm that
> >> reverting only this one commit in 2.6.36-rc1
> >> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
> >> suspend works OK and I have no issues with my network after waking the
> >> system up, neither after re-booting from the same or any other kernel.
> >>
> >> Please let me know what further details should I provide. Any logs,
> >> hardware specifications? I've also made a brief log recording the
> >> bisect if anyone would need it.
> >
> > Well, we should let the HID maintainer know about the problem (CC added).
> 
> Thanks Rafael,
> Before I opened this thread I only searched on
> http://vger.kernel.org/vger-lists.html for any appropriate mailing
> list but nothing linke linux-*hid* there. Now I've found out that
> there is a list of all maintainers included in the kernel source. I'll
> keep that in mind ..

Yeah, MAINTAINERS file is always good to consult. Also CCing all people 
who have Signed-off-by or Acked-by the offending patch is usually a good 
idea.

Could you please verify whether the patch below (which is currently queued 
in my tree and I am planning to push it to Linus soon, as it fixes memory 
corruption) fixes your issue? Thanks.

commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
Author: Jiri Kosina <jkosina@suse.cz>
Date:   Fri Aug 13 12:19:45 2010 +0200

    HID: hiddev: fix memory corruption due to invalid intfdata
    
    Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
    get rid of BKL") introduced using of private intfdata in hiddev for
    purpose of storing hiddev pointer.
    
    This is a problem, because intf pointer is already being set to struct
    hid_device pointer by HID core. This obviously lead to memory corruptions
    at device disconnect time, such as
    
    WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
    kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
    
    Convert hiddev into accessing hiddev through struct hid_device which is
    in intfdata already.
    
    Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
    Reported-and-tested-by: Heinz Diehl <htd@fritha.org>
    Reported-and-tested-by: Alan Ott <alan@signal11.us>
    Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index f285017..0a29c51 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
 {
 	struct hiddev_list *list;
 	struct usb_interface *intf;
+	struct hid_device *hid;
 	struct hiddev *hiddev;
 	int res;
 
 	intf = usb_find_interface(&hiddev_driver, iminor(inode));
 	if (!intf)
 		return -ENODEV;
-	hiddev = usb_get_intfdata(intf);
+	hid = usb_get_intfdata(intf);
+	hiddev = hid->hiddev;
 
 	if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
 		return -ENOMEM;
@@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
 	hid->hiddev = hiddev;
 	hiddev->hid = hid;
 	hiddev->exist = 1;
-	usb_set_intfdata(usbhid->intf, usbhid);
 	retval = usb_register_dev(usbhid->intf, &hiddev_class);
 	if (retval) {
 		err_hid("Not able to get a minor for this device.");

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[trapDoor]

[adding Gabriel Craciunescu <osnix0@googlemail.com> and Andrea Gelmini
<andrea.gelmini@gelma.net> to CC lilst]

Hello,

On Wed, Aug 18, 2010 at 8:13 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 18 Aug 2010, trapDoor wrote:
>
>> >> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
>> >> results in a system that works.
>> >>
>> >> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
>> >> Author: Arnd Bergmann <arnd@arndb.de>
>> >> Date:   Sun Jul 11 15:34:05 2010 +0200
>> >>
>> >>     HID: hiddev: use usb_find_interface, get rid of BKL
>> >>
>> >>     This removes the private hiddev_table in the usbhid
>> >>     driver and changes it to use usb_find_interface
>> >>     instead.
>> >>
>> >>     The advantage is that we can avoid the race between
>> >>     usb_register_dev and usb_open and no longer need the
>> >>     big kernel lock.
>> >>
>> >>     This doesn't introduce race condition -- the intf pointer could be
>> >>     invalidated only in hiddev_disconnect() through usb_deregister_dev(),
>> >>     but that will block on minor_rwsem and not actually remove the device
>> >>     until usb_open().
>> >> -----------------------
>> >>
>> >> Despite mentioned issues with network (occurring every time after
>> >> unsuccessful suspend > hard restart) the bisecting turned up a commit
>> >> which isn't rather related to network. But I can confirm that
>> >> reverting only this one commit in 2.6.36-rc1
>> >> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
>> >> suspend works OK and I have no issues with my network after waking the
>> >> system up, neither after re-booting from the same or any other kernel.
>> >>
>> >> Please let me know what further details should I provide. Any logs,
>> >> hardware specifications? I've also made a brief log recording the
>> >> bisect if anyone would need it.
>> >
>> > Well, we should let the HID maintainer know about the problem (CC added).
>>
>> Thanks Rafael,
>> Before I opened this thread I only searched on
>> http://vger.kernel.org/vger-lists.html for any appropriate mailing
>> list but nothing linke linux-*hid* there. Now I've found out that
>> there is a list of all maintainers included in the kernel source. I'll
>> keep that in mind ..
>
> Yeah, MAINTAINERS file is always good to consult. Also CCing all people
> who have Signed-off-by or Acked-by the offending patch is usually a good
> idea.

Thanks for the tip.

> Could you please verify whether the patch below (which is currently queued
> in my tree and I am planning to push it to Linus soon, as it fixes memory
> corruption) fixes your issue? Thanks.
>
> commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
> Author: Jiri Kosina <jkosina@suse.cz>
> Date:   Fri Aug 13 12:19:45 2010 +0200
>
>    HID: hiddev: fix memory corruption due to invalid intfdata
>
>    Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
>    get rid of BKL") introduced using of private intfdata in hiddev for
>    purpose of storing hiddev pointer.
>
>    This is a problem, because intf pointer is already being set to struct
>    hid_device pointer by HID core. This obviously lead to memory corruptions
>    at device disconnect time, such as
>
>    WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
>    kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
>
>    Convert hiddev into accessing hiddev through struct hid_device which is
>    in intfdata already.
>
>    Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
>    Reported-and-tested-by: Heinz Diehl <htd@fritha.org>
>    Reported-and-tested-by: Alan Ott <alan@signal11.us>
>    Signed-off-by: Jiri Kosina <jkosina@suse.cz>
>
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index f285017..0a29c51 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
>  {
>        struct hiddev_list *list;
>        struct usb_interface *intf;
> +       struct hid_device *hid;
>        struct hiddev *hiddev;
>        int res;
>
>        intf = usb_find_interface(&hiddev_driver, iminor(inode));
>        if (!intf)
>                return -ENODEV;
> -       hiddev = usb_get_intfdata(intf);
> +       hid = usb_get_intfdata(intf);
> +       hiddev = hid->hiddev;
>
>        if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
>                return -ENOMEM;
> @@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
>        hid->hiddev = hiddev;
>        hiddev->hid = hid;
>        hiddev->exist = 1;
> -       usb_set_intfdata(usbhid->intf, usbhid);
>        retval = usb_register_dev(usbhid->intf, &hiddev_class);
>        if (retval) {
>                err_hid("Not able to get a minor for this device.");
>
> --
> Jiri Kosina
> SUSE Labs, Novell Inc.
>

Yes. That patch fixes my problem too. Tested on top of the current
tree [2.6.36-rc1-00051-g3b89f56]. Thanks!

One little thing though. There is a message about 2 lines offset when
patching. Succeeded anyway but I've never got such messages before so
I thought it would be better to mention it:

patch -p1 <../patches/hid.git-9c9e54a8df0be48aa359744f412377cc55c3b7d2.patch
patching file drivers/hid/usbhid/hiddev.c
Hunk #2 succeeded at 890 (offset -2 lines).

I downloaded the patch from
http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commitdiff;h=9c9e54a8df0be48aa359744f412377cc55c3b7d2
to make sure I had the original indentations etc. preserved (Gmail
tends to mess up with lines).

-- 
Regards
Tomasz B.

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Jiri Kosina]

On Wed, 18 Aug 2010, trapDoor wrote:

> > Could you please verify whether the patch below (which is currently queued
> > in my tree and I am planning to push it to Linus soon, as it fixes memory
> > corruption) fixes your issue? Thanks.
> >
> > commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
> > Author: Jiri Kosina <jkosina@suse.cz>
> > Date:   Fri Aug 13 12:19:45 2010 +0200
> >
> >    HID: hiddev: fix memory corruption due to invalid intfdata
> >
> >    Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
> >    get rid of BKL") introduced using of private intfdata in hiddev for
> >    purpose of storing hiddev pointer.
> >
> >    This is a problem, because intf pointer is already being set to struct
> >    hid_device pointer by HID core. This obviously lead to memory corruptions
> >    at device disconnect time, such as
> >
> >    WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
> >    kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
> >
> >    Convert hiddev into accessing hiddev through struct hid_device which is
> >    in intfdata already.
> >
> >    Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
> >    Reported-and-tested-by: Heinz Diehl <htd@fritha.org>
> >    Reported-and-tested-by: Alan Ott <alan@signal11.us>
> >    Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> >
> > diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> > index f285017..0a29c51 100644
> > --- a/drivers/hid/usbhid/hiddev.c
> > +++ b/drivers/hid/usbhid/hiddev.c
> > @@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
> >  {
> >        struct hiddev_list *list;
> >        struct usb_interface *intf;
> > +       struct hid_device *hid;
> >        struct hiddev *hiddev;
> >        int res;
> >
> >        intf = usb_find_interface(&hiddev_driver, iminor(inode));
> >        if (!intf)
> >                return -ENODEV;
> > -       hiddev = usb_get_intfdata(intf);
> > +       hid = usb_get_intfdata(intf);
> > +       hiddev = hid->hiddev;
> >
> >        if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
> >                return -ENOMEM;
> > @@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
> >        hid->hiddev = hiddev;
> >        hiddev->hid = hid;
> >        hiddev->exist = 1;
> > -       usb_set_intfdata(usbhid->intf, usbhid);
> >        retval = usb_register_dev(usbhid->intf, &hiddev_class);
> >        if (retval) {
> >                err_hid("Not able to get a minor for this device.");
> >
> 
> Yes. That patch fixes my problem too. Tested on top of the current
> tree [2.6.36-rc1-00051-g3b89f56]. Thanks!

Thanks a lot for prompt testing.

> One little thing though. There is a message about 2 lines offset when
> patching. Succeeded anyway but I've never got such messages before so
> I thought it would be better to mention it:
> 
> patch -p1 <../patches/hid.git-9c9e54a8df0be48aa359744f412377cc55c3b7d2.patch
> patching file drivers/hid/usbhid/hiddev.c
> Hunk #2 succeeded at 890 (offset -2 lines).
> 
> I downloaded the patch from
> http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commitdiff;h=9c9e54a8df0be48aa359744f412377cc55c3b7d2
> to make sure I had the original indentations etc. preserved (Gmail
> tends to mess up with lines).

Yeah, that's fine. It just means that the patch has been generated on a 
slightly different version of the file, which makes the line numbers be 
off a bit, but patch, git and friends can cope with this easily.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[trapDoor]

On Wed, Aug 18, 2010 at 9:46 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 18 Aug 2010, trapDoor wrote:
>
>> > Could you please verify whether the patch below (which is currently queued
>> > in my tree and I am planning to push it to Linus soon, as it fixes memory
>> > corruption) fixes your issue? Thanks.
>> >
>> > commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
>> > Author: Jiri Kosina <jkosina@suse.cz>
>> > Date:   Fri Aug 13 12:19:45 2010 +0200
>> >
>> >    HID: hiddev: fix memory corruption due to invalid intfdata
>> >
>> >    Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
>> >    get rid of BKL") introduced using of private intfdata in hiddev for
>> >    purpose of storing hiddev pointer.
>> >
>> >    This is a problem, because intf pointer is already being set to struct
>> >    hid_device pointer by HID core. This obviously lead to memory corruptions
>> >    at device disconnect time, such as
>> >
>> >    WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
>> >    kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
>> >
>> >    Convert hiddev into accessing hiddev through struct hid_device which is
>> >    in intfdata already.
>> >
>> >    Reported-and-tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
>> >    Reported-and-tested-by: Heinz Diehl <htd@fritha.org>
>> >    Reported-and-tested-by: Alan Ott <alan@signal11.us>
>> >    Signed-off-by: Jiri Kosina <jkosina@suse.cz>
>> >
>> > diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
>> > index f285017..0a29c51 100644
>> > --- a/drivers/hid/usbhid/hiddev.c
>> > +++ b/drivers/hid/usbhid/hiddev.c
>> > @@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
>> >  {
>> >        struct hiddev_list *list;
>> >        struct usb_interface *intf;
>> > +       struct hid_device *hid;
>> >        struct hiddev *hiddev;
>> >        int res;
>> >
>> >        intf = usb_find_interface(&hiddev_driver, iminor(inode));
>> >        if (!intf)
>> >                return -ENODEV;
>> > -       hiddev = usb_get_intfdata(intf);
>> > +       hid = usb_get_intfdata(intf);
>> > +       hiddev = hid->hiddev;
>> >
>> >        if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
>> >                return -ENOMEM;
>> > @@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
>> >        hid->hiddev = hiddev;
>> >        hiddev->hid = hid;
>> >        hiddev->exist = 1;
>> > -       usb_set_intfdata(usbhid->intf, usbhid);
>> >        retval = usb_register_dev(usbhid->intf, &hiddev_class);
>> >        if (retval) {
>> >                err_hid("Not able to get a minor for this device.");
>> >
>>
>> Yes. That patch fixes my problem too. Tested on top of the current
>> tree [2.6.36-rc1-00051-g3b89f56]. Thanks!
>
> Thanks a lot for prompt testing.
>
>> One little thing though. There is a message about 2 lines offset when
>> patching. Succeeded anyway but I've never got such messages before so
>> I thought it would be better to mention it:
>>
>> patch -p1 <../patches/hid.git-9c9e54a8df0be48aa359744f412377cc55c3b7d2.patch
>> patching file drivers/hid/usbhid/hiddev.c
>> Hunk #2 succeeded at 890 (offset -2 lines).
>>
>> I downloaded the patch from
>> http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commitdiff;h=9c9e54a8df0be48aa359744f412377cc55c3b7d2
>> to make sure I had the original indentations etc. preserved (Gmail
>> tends to mess up with lines).
>
> Yeah, that's fine. It just means that the patch has been generated on a
> slightly different version of the file, which makes the line numbers be
> off a bit, but patch, git and friends can cope with this easily.
>
Thanks for confirming.


Andrea, Gabriel
Would you please check if Jiri's patch solves the problem on your end
as well? Just to make 100% sure we can close related bugzilla call.


-- 
Regards
Tomasz B.

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Andrea Gelmini]

2010/8/18 trapDoor <trapdoor6@gmail.com>:
> Andrea, Gabriel
> Would you please check if Jiri's patch solves the problem on your end
> as well? Just to make 100% sure we can close related bugzilla call.

I hope to try it tomorrow.

Thanks a lot for your work,
Andrea

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Andrea Gelmini]

2010/8/18 trapDoor <trapdoor6@gmail.com>:
> Andrea, Gabriel
> Would you please check if Jiri's patch solves the problem on your end
> as well? Just to make 100% sure we can close related bugzilla call.

Well, I made the test, with ~10 cycle hibernation/resume.
Everything seems goes right.

Thanks a lot for your work,
Andrea

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Gabriel Craciunescu]

2010/8/18 Andrea Gelmini <andrea.gelmini@gelma.net>
>
> 2010/8/18 trapDoor <trapdoor6@gmail.com>:
> > Andrea, Gabriel
> > Would you please check if Jiri's patch solves the problem on your end
> > as well? Just to make 100% sure we can close related bugzilla call.
>
> Well, I made the test, with ~10 cycle hibernation/resume.
> Everything seems goes right.
>

Does work fine here also ..

Regards,

Gabriel

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[Jiri Kosina]

On Wed, 18 Aug 2010, Gabriel Craciunescu wrote:

> > > Andrea, Gabriel
> > > Would you please check if Jiri's patch solves the problem on your end
> > > as well? Just to make 100% sure we can close related bugzilla call.
> >
> > Well, I made the test, with ~10 cycle hibernation/resume.
> > Everything seems goes right.
> >
> 
> Does work fine here also ..

Thanks everyone for confirming. The fix is now in Linus' tree and thus 
will be in -rc2.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues

[trapDoor]

On Thu, Aug 19, 2010 at 8:40 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 18 Aug 2010, Gabriel Craciunescu wrote:
>
>> > > Andrea, Gabriel
>> > > Would you please check if Jiri's patch solves the problem on your end
>> > > as well? Just to make 100% sure we can close related bugzilla call.
>> >
>> > Well, I made the test, with ~10 cycle hibernation/resume.
>> > Everything seems goes right.
>> >
>>
>> Does work fine here also ..
>
> Thanks everyone for confirming. The fix is now in Linus' tree and thus
> will be in -rc2.
>
> --
> Jiri Kosina
> SUSE Labs, Novell Inc.
>

I saw Linus merged your HID pull request yesterday. I've got it now
and can confirm again that everything works OK.

-- 
Thanks
Tomasz B.