linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v3.11][v3.12][v3.13][Regression]  EISA: Initialize device before its resources
@ 2014-01-16 17:53 Joseph Salisbury
  2014-01-16 18:12 ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Joseph Salisbury @ 2014-01-16 17:53 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML

Hi Bjorn,

A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
was found the following commit introduced this bug:

commit 26abfeed4341872364386c6a52b9acef8c81a81a
Author: Bjorn Helgaas <bhelgaas@google.com>
Date: Mon Apr 15 14:34:02 2013 -0600

    EISA: Initialize device before its resources

The regression was introduced as of v3.10-rc1.

Reverting this commit has resolved the bug.  I wanted to get your
feedback, since you are the patch author.  Should we consider reverting
this commit in mainline and the stable trees?

Thanks,

Joe


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-16 17:53 [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources Joseph Salisbury
@ 2014-01-16 18:12 ` Bjorn Helgaas
  2014-01-16 18:14   ` Joseph Salisbury
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2014-01-16 18:12 UTC (permalink / raw)
  To: Joseph Salisbury; +Cc: LKML

On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
<joseph.salisbury@canonical.com> wrote:
> Hi Bjorn,
>
> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
> was found the following commit introduced this bug:

Sorry about that, and thanks for the report.  Did you mean to include
URL for the bug?

> commit 26abfeed4341872364386c6a52b9acef8c81a81a
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date: Mon Apr 15 14:34:02 2013 -0600
>
>     EISA: Initialize device before its resources
>
> The regression was introduced as of v3.10-rc1.
>
> Reverting this commit has resolved the bug.  I wanted to get your
> feedback, since you are the patch author.  Should we consider reverting
> this commit in mainline and the stable trees?
>
> Thanks,
>
> Joe
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-16 18:12 ` Bjorn Helgaas
@ 2014-01-16 18:14   ` Joseph Salisbury
  2014-01-17 17:02     ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Joseph Salisbury @ 2014-01-16 18:14 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML

On 01/16/2014 01:12 PM, Bjorn Helgaas wrote:
> On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
> <joseph.salisbury@canonical.com> wrote:
>> Hi Bjorn,
>>
>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
>> was found the following commit introduced this bug:
> Sorry about that, and thanks for the report.  Did you mean to include
> URL for the bug?
Yes, sorry about that:
http://pad.lv/1251816

>
>> commit 26abfeed4341872364386c6a52b9acef8c81a81a
>> Author: Bjorn Helgaas <bhelgaas@google.com>
>> Date: Mon Apr 15 14:34:02 2013 -0600
>>
>>     EISA: Initialize device before its resources
>>
>> The regression was introduced as of v3.10-rc1.
>>
>> Reverting this commit has resolved the bug.  I wanted to get your
>> feedback, since you are the patch author.  Should we consider reverting
>> this commit in mainline and the stable trees?
>>
>> Thanks,
>>
>> Joe
>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-16 18:14   ` Joseph Salisbury
@ 2014-01-17 17:02     ` Bjorn Helgaas
  2014-01-17 19:26       ` Joseph Salisbury
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2014-01-17 17:02 UTC (permalink / raw)
  To: Joseph Salisbury; +Cc: LKML

On Thu, Jan 16, 2014 at 01:14:01PM -0500, Joseph Salisbury wrote:
> On 01/16/2014 01:12 PM, Bjorn Helgaas wrote:
> > On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
> > <joseph.salisbury@canonical.com> wrote:
> >> Hi Bjorn,
> >>
> >> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
> >> was found the following commit introduced this bug:
> > Sorry about that, and thanks for the report.  Did you mean to include
> > URL for the bug?
> Yes, sorry about that:
> http://pad.lv/1251816

Hi Joseph,

Can you attach the 3.8.0-32-generic config (the one matching the successful
boot at https://launchpadlibrarian.net/156685076/BootDmesg.txt) to the bug?

The only way I can match up the output:

  EISA: Probing bus 0 at eisa.0
  Cannot allocate resource for EISA slot 1
  Cannot allocate resource for EISA slot 2
  Cannot allocate resource for EISA slot 4
  Cannot allocate resource for EISA slot 5
  EISA: Detected 0 cards.

with the code is if we have root->force_probe set, and the only way I see for
that to happen is if we're in the virtual_eisa_root_init() path and
CONFIG_EISA_VLB_PRIMING=y:

  #if defined(CONFIG_ALPHA_JENSEN) || defined(CONFIG_EISA_VLB_PRIMING)
  #define EISA_FORCE_PROBE_DEFAULT 1
  #else
  #define EISA_FORCE_PROBE_DEFAULT 0
  #endif

  static int force_probe = EISA_FORCE_PROBE_DEFAULT;

  virtual_eisa_root_init
    eisa_bus_root.force_probe = force_probe
    eisa_root_register(&eisa_bus_root)
      eisa_probe(root)
	printk("EISA: Probing bus %d at %s")
	if (eisa_request_resources)         
	  printk("EISA: Cannot allocate resource for mainboard")    # we don't see this
	if (eisa_init_device)               
	  eisa_release_resources
	  kfree
	  if (!root->force_probe)           
	    return -ENODEV
	    goto force_probe
	printk("EISA: Mainboard %s detected")       # we don't see this

	force_probe:
	for (i = 1; ...; i++)
	  if (eisa_request_resources(i))
	    printk("Cannot allocate resource for EISA slot %d", i)
	printk("EISA: Detected %d cards")

Bjorn

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-17 17:02     ` Bjorn Helgaas
@ 2014-01-17 19:26       ` Joseph Salisbury
  2014-01-17 22:19         ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Joseph Salisbury @ 2014-01-17 19:26 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML

On 01/17/2014 12:02 PM, Bjorn Helgaas wrote:
> On Thu, Jan 16, 2014 at 01:14:01PM -0500, Joseph Salisbury wrote:
>> On 01/16/2014 01:12 PM, Bjorn Helgaas wrote:
>>> On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
>>> <joseph.salisbury@canonical.com> wrote:
>>>> Hi Bjorn,
>>>>
>>>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
>>>> was found the following commit introduced this bug:
>>> Sorry about that, and thanks for the report.  Did you mean to include
>>> URL for the bug?
>> Yes, sorry about that:
>> http://pad.lv/1251816
> Hi Joseph,
>
> Can you attach the 3.8.0-32-generic config (the one matching the successful
> boot at https://launchpadlibrarian.net/156685076/BootDmesg.txt) to the bug?

I attached the config file to the bug:
https://launchpadlibrarian.net/162754666/config.common.ubuntu

I also attached a tar file with the complete config directory for that
kernel version.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/+attachment/3951156/+files/raring-config.tar

>
> The only way I can match up the output:
>
>   EISA: Probing bus 0 at eisa.0
>   Cannot allocate resource for EISA slot 1
>   Cannot allocate resource for EISA slot 2
>   Cannot allocate resource for EISA slot 4
>   Cannot allocate resource for EISA slot 5
>   EISA: Detected 0 cards.
>
> with the code is if we have root->force_probe set, and the only way I see for
> that to happen is if we're in the virtual_eisa_root_init() path and
> CONFIG_EISA_VLB_PRIMING=y:
>
>   #if defined(CONFIG_ALPHA_JENSEN) || defined(CONFIG_EISA_VLB_PRIMING)
>   #define EISA_FORCE_PROBE_DEFAULT 1
>   #else
>   #define EISA_FORCE_PROBE_DEFAULT 0
>   #endif
>
>   static int force_probe = EISA_FORCE_PROBE_DEFAULT;
>
>   virtual_eisa_root_init
>     eisa_bus_root.force_probe = force_probe
>     eisa_root_register(&eisa_bus_root)
>       eisa_probe(root)
> 	printk("EISA: Probing bus %d at %s")
> 	if (eisa_request_resources)         
> 	  printk("EISA: Cannot allocate resource for mainboard")    # we don't see this
> 	if (eisa_init_device)               
> 	  eisa_release_resources
> 	  kfree
> 	  if (!root->force_probe)           
> 	    return -ENODEV
> 	    goto force_probe
> 	printk("EISA: Mainboard %s detected")       # we don't see this
>
> 	force_probe:
> 	for (i = 1; ...; i++)
> 	  if (eisa_request_resources(i))
> 	    printk("Cannot allocate resource for EISA slot %d", i)
> 	printk("EISA: Detected %d cards")
>
> Bjorn


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-17 19:26       ` Joseph Salisbury
@ 2014-01-17 22:19         ` Bjorn Helgaas
  2014-01-18  1:35           ` Joseph Salisbury
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2014-01-17 22:19 UTC (permalink / raw)
  To: Joseph Salisbury; +Cc: LKML

On Fri, Jan 17, 2014 at 02:26:23PM -0500, Joseph Salisbury wrote:
> On 01/17/2014 12:02 PM, Bjorn Helgaas wrote:
> > On Thu, Jan 16, 2014 at 01:14:01PM -0500, Joseph Salisbury wrote:
> >> On 01/16/2014 01:12 PM, Bjorn Helgaas wrote:
> >>> On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
> >>> <joseph.salisbury@canonical.com> wrote:
> >>>> Hi Bjorn,
> >>>>
> >>>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
> >>>> was found the following commit introduced this bug:
> >>> Sorry about that, and thanks for the report.  Did you mean to include
> >>> URL for the bug?
> >> Yes, sorry about that:
> >> http://pad.lv/1251816
> > Hi Joseph,
> >
> > Can you attach the 3.8.0-32-generic config (the one matching the successful
> > boot at https://launchpadlibrarian.net/156685076/BootDmesg.txt) to the bug?
> 
> I attached the config file to the bug:
> https://launchpadlibrarian.net/162754666/config.common.ubuntu
> 
> I also attached a tar file with the complete config directory for that
> kernel version.
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/+attachment/3951156/+files/raring-config.tar

Thanks again.  I attached the following reverts to launchpad.  I screwed up
when doing those EISA changes.  I'd like to squeeze these into my v3.14
merge request (probably early next week), so please test and let me know
if this fixes the problem.  I'm really sorry for the inconvenience.

Bjorn


Revert "EISA: Log device resources in dmesg"

From: Bjorn Helgaas <bhelgaas@google.com>

This reverts commit a2080d0c561c546d73cb8b296d4b7ca414e6860b.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/eisa/eisa-bus.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
index 8842cde69177..1b86fe0c2e80 100644
--- a/drivers/eisa/eisa-bus.c
+++ b/drivers/eisa/eisa-bus.c
@@ -288,7 +288,6 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
 			edev->res[i].flags = IORESOURCE_IO | IORESOURCE_BUSY;
 		}
 
-		dev_printk(KERN_DEBUG, &edev->dev, "%pR\n", &edev->res[i]);
 		if (request_resource(root->res, &edev->res[i]))
 			goto failed;
 	}
Revert "EISA: Initialize device before its resources"

From: Bjorn Helgaas <bhelgaas@google.com>

This reverts commit 26abfeed4341872364386c6a52b9acef8c81a81a.

In the eisa_probe() force_probe path, if we were unable to request slot
resources (e.g., [io 0x800-0x8ff]), we skipped the slot with "Cannot
allocate resource for EISA slot %d" before reading the EISA signature in
eisa_init_device().

Commit 26abfeed4341 moved eisa_init_device() earlier, so we tried to read
the EISA signature before requesting the slot resources, and this caused
hangs during boot.

Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/eisa/eisa-bus.c |   26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
index 1b86fe0c2e80..612afeaec3cb 100644
--- a/drivers/eisa/eisa-bus.c
+++ b/drivers/eisa/eisa-bus.c
@@ -277,11 +277,13 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
 		}
 		
 		if (slot) {
+			edev->res[i].name  = NULL;
 			edev->res[i].start = SLOT_ADDRESS(root, slot)
 					     + (i * 0x400);
 			edev->res[i].end   = edev->res[i].start + 0xff;
 			edev->res[i].flags = IORESOURCE_IO;
 		} else {
+			edev->res[i].name  = NULL;
 			edev->res[i].start = SLOT_ADDRESS(root, slot)
 					     + EISA_VENDOR_ID_OFFSET;
 			edev->res[i].end   = edev->res[i].start + 3;
@@ -327,19 +329,20 @@ static int __init eisa_probe(struct eisa_root_device *root)
 		return -ENOMEM;
 	}
 		
-	if (eisa_init_device(root, edev, 0)) {
+	if (eisa_request_resources(root, edev, 0)) {
+		dev_warn(root->dev,
+		         "EISA: Cannot allocate resource for mainboard\n");
 		kfree(edev);
 		if (!root->force_probe)
-			return -ENODEV;
+			return -EBUSY;
 		goto force_probe;
 	}
 
-	if (eisa_request_resources(root, edev, 0)) {
-		dev_warn(root->dev,
-		         "EISA: Cannot allocate resource for mainboard\n");
+	if (eisa_init_device(root, edev, 0)) {
+		eisa_release_resources(edev);
 		kfree(edev);
 		if (!root->force_probe)
-			return -EBUSY;
+			return -ENODEV;
 		goto force_probe;
 	}
 
@@ -362,11 +365,6 @@ static int __init eisa_probe(struct eisa_root_device *root)
 			continue;
 		}
 
-		if (eisa_init_device(root, edev, i)) {
-			kfree(edev);
-			continue;
-		}
-
 		if (eisa_request_resources(root, edev, i)) {
 			dev_warn(root->dev,
 			         "Cannot allocate resource for EISA slot %d\n",
@@ -375,6 +373,12 @@ static int __init eisa_probe(struct eisa_root_device *root)
 			continue;
 		}
 
+		if (eisa_init_device(root, edev, i)) {
+			eisa_release_resources(edev);
+			kfree(edev);
+			continue;
+		}
+
 		if (edev->state == (EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED))
 			enabled_str = " (forced enabled)";
 		else if (edev->state == EISA_CONFIG_FORCED)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-17 22:19         ` Bjorn Helgaas
@ 2014-01-18  1:35           ` Joseph Salisbury
  2014-01-18 14:37             ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Joseph Salisbury @ 2014-01-18  1:35 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML

On 01/17/2014 05:19 PM, Bjorn Helgaas wrote:
> On Fri, Jan 17, 2014 at 02:26:23PM -0500, Joseph Salisbury wrote:
>> On 01/17/2014 12:02 PM, Bjorn Helgaas wrote:
>>> On Thu, Jan 16, 2014 at 01:14:01PM -0500, Joseph Salisbury wrote:
>>>> On 01/16/2014 01:12 PM, Bjorn Helgaas wrote:
>>>>> On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
>>>>> <joseph.salisbury@canonical.com> wrote:
>>>>>> Hi Bjorn,
>>>>>>
>>>>>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
>>>>>> was found the following commit introduced this bug:
>>>>> Sorry about that, and thanks for the report.  Did you mean to include
>>>>> URL for the bug?
>>>> Yes, sorry about that:
>>>> http://pad.lv/1251816
>>> Hi Joseph,
>>>
>>> Can you attach the 3.8.0-32-generic config (the one matching the successful
>>> boot at https://launchpadlibrarian.net/156685076/BootDmesg.txt) to the bug?
>> I attached the config file to the bug:
>> https://launchpadlibrarian.net/162754666/config.common.ubuntu
>>
>> I also attached a tar file with the complete config directory for that
>> kernel version.
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/+attachment/3951156/+files/raring-config.tar
> Thanks again.  I attached the following reverts to launchpad.  I screwed up
> when doing those EISA changes.  I'd like to squeeze these into my v3.14
> merge request (probably early next week), so please test and let me know
> if this fixes the problem.  I'm really sorry for the inconvenience.
>
> Bjorn
>
>
> Revert "EISA: Log device resources in dmesg"
>
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> This reverts commit a2080d0c561c546d73cb8b296d4b7ca414e6860b.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/eisa/eisa-bus.c |    1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
> index 8842cde69177..1b86fe0c2e80 100644
> --- a/drivers/eisa/eisa-bus.c
> +++ b/drivers/eisa/eisa-bus.c
> @@ -288,7 +288,6 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
>  			edev->res[i].flags = IORESOURCE_IO | IORESOURCE_BUSY;
>  		}
>  
> -		dev_printk(KERN_DEBUG, &edev->dev, "%pR\n", &edev->res[i]);
>  		if (request_resource(root->res, &edev->res[i]))
>  			goto failed;
>  	}
> Revert "EISA: Initialize device before its resources"
>
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> This reverts commit 26abfeed4341872364386c6a52b9acef8c81a81a.
>
> In the eisa_probe() force_probe path, if we were unable to request slot
> resources (e.g., [io 0x800-0x8ff]), we skipped the slot with "Cannot
> allocate resource for EISA slot %d" before reading the EISA signature in
> eisa_init_device().
>
> Commit 26abfeed4341 moved eisa_init_device() earlier, so we tried to read
> the EISA signature before requesting the slot resources, and this caused
> hangs during boot.
>
> Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/eisa/eisa-bus.c |   26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
> index 1b86fe0c2e80..612afeaec3cb 100644
> --- a/drivers/eisa/eisa-bus.c
> +++ b/drivers/eisa/eisa-bus.c
> @@ -277,11 +277,13 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
>  		}
>  		
>  		if (slot) {
> +			edev->res[i].name  = NULL;
>  			edev->res[i].start = SLOT_ADDRESS(root, slot)
>  					     + (i * 0x400);
>  			edev->res[i].end   = edev->res[i].start + 0xff;
>  			edev->res[i].flags = IORESOURCE_IO;
>  		} else {
> +			edev->res[i].name  = NULL;
>  			edev->res[i].start = SLOT_ADDRESS(root, slot)
>  					     + EISA_VENDOR_ID_OFFSET;
>  			edev->res[i].end   = edev->res[i].start + 3;
> @@ -327,19 +329,20 @@ static int __init eisa_probe(struct eisa_root_device *root)
>  		return -ENOMEM;
>  	}
>  		
> -	if (eisa_init_device(root, edev, 0)) {
> +	if (eisa_request_resources(root, edev, 0)) {
> +		dev_warn(root->dev,
> +		         "EISA: Cannot allocate resource for mainboard\n");
>  		kfree(edev);
>  		if (!root->force_probe)
> -			return -ENODEV;
> +			return -EBUSY;
>  		goto force_probe;
>  	}
>  
> -	if (eisa_request_resources(root, edev, 0)) {
> -		dev_warn(root->dev,
> -		         "EISA: Cannot allocate resource for mainboard\n");
> +	if (eisa_init_device(root, edev, 0)) {
> +		eisa_release_resources(edev);
>  		kfree(edev);
>  		if (!root->force_probe)
> -			return -EBUSY;
> +			return -ENODEV;
>  		goto force_probe;
>  	}
>  
> @@ -362,11 +365,6 @@ static int __init eisa_probe(struct eisa_root_device *root)
>  			continue;
>  		}
>  
> -		if (eisa_init_device(root, edev, i)) {
> -			kfree(edev);
> -			continue;
> -		}
> -
>  		if (eisa_request_resources(root, edev, i)) {
>  			dev_warn(root->dev,
>  			         "Cannot allocate resource for EISA slot %d\n",
> @@ -375,6 +373,12 @@ static int __init eisa_probe(struct eisa_root_device *root)
>  			continue;
>  		}
>  
> +		if (eisa_init_device(root, edev, i)) {
> +			eisa_release_resources(edev);
> +			kfree(edev);
> +			continue;
> +		}
> +
>  		if (edev->state == (EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED))
>  			enabled_str = " (forced enabled)";
>  		else if (edev->state == EISA_CONFIG_FORCED)
Thanks again for looking at this, Bjorn!

Yes, reverting commit 26abfeed4 does in fact resolve the bug.  I already
built a test kernel, noted in comment #34:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/comments/34

The original bug reporter tested the kernel to confirm it fixes the bug:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/comments/36

Would it be also possible for you to request the revert in the stable
trees, in addition to 3.14?

Thanks,

Joe



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-18  1:35           ` Joseph Salisbury
@ 2014-01-18 14:37             ` Bjorn Helgaas
  2014-01-22 18:00               ` Joseph Salisbury
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2014-01-18 14:37 UTC (permalink / raw)
  To: Joseph Salisbury; +Cc: LKML

On Fri, Jan 17, 2014 at 6:35 PM, Joseph Salisbury
<joseph.salisbury@canonical.com> wrote:
> On 01/17/2014 05:19 PM, Bjorn Helgaas wrote:
>> On Fri, Jan 17, 2014 at 02:26:23PM -0500, Joseph Salisbury wrote:
>>> On 01/17/2014 12:02 PM, Bjorn Helgaas wrote:
>>>> On Thu, Jan 16, 2014 at 01:14:01PM -0500, Joseph Salisbury wrote:
>>>>> On 01/16/2014 01:12 PM, Bjorn Helgaas wrote:
>>>>>> On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
>>>>>> <joseph.salisbury@canonical.com> wrote:
>>>>>>> Hi Bjorn,
>>>>>>>
>>>>>>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
>>>>>>> was found the following commit introduced this bug:
>>>>>> Sorry about that, and thanks for the report.  Did you mean to include
>>>>>> URL for the bug?
>>>>> Yes, sorry about that:
>>>>> http://pad.lv/1251816
>>>> Hi Joseph,
>>>>
>>>> Can you attach the 3.8.0-32-generic config (the one matching the successful
>>>> boot at https://launchpadlibrarian.net/156685076/BootDmesg.txt) to the bug?
>>> I attached the config file to the bug:
>>> https://launchpadlibrarian.net/162754666/config.common.ubuntu
>>>
>>> I also attached a tar file with the complete config directory for that
>>> kernel version.
>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/+attachment/3951156/+files/raring-config.tar
>> Thanks again.  I attached the following reverts to launchpad.  I screwed up
>> when doing those EISA changes.  I'd like to squeeze these into my v3.14
>> merge request (probably early next week), so please test and let me know
>> if this fixes the problem.  I'm really sorry for the inconvenience.
>>
>> Bjorn
>>
>>
>> Revert "EISA: Log device resources in dmesg"
>>
>> From: Bjorn Helgaas <bhelgaas@google.com>
>>
>> This reverts commit a2080d0c561c546d73cb8b296d4b7ca414e6860b.
>>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>  drivers/eisa/eisa-bus.c |    1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
>> index 8842cde69177..1b86fe0c2e80 100644
>> --- a/drivers/eisa/eisa-bus.c
>> +++ b/drivers/eisa/eisa-bus.c
>> @@ -288,7 +288,6 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
>>                       edev->res[i].flags = IORESOURCE_IO | IORESOURCE_BUSY;
>>               }
>>
>> -             dev_printk(KERN_DEBUG, &edev->dev, "%pR\n", &edev->res[i]);
>>               if (request_resource(root->res, &edev->res[i]))
>>                       goto failed;
>>       }
>> Revert "EISA: Initialize device before its resources"
>>
>> From: Bjorn Helgaas <bhelgaas@google.com>
>>
>> This reverts commit 26abfeed4341872364386c6a52b9acef8c81a81a.
>>
>> In the eisa_probe() force_probe path, if we were unable to request slot
>> resources (e.g., [io 0x800-0x8ff]), we skipped the slot with "Cannot
>> allocate resource for EISA slot %d" before reading the EISA signature in
>> eisa_init_device().
>>
>> Commit 26abfeed4341 moved eisa_init_device() earlier, so we tried to read
>> the EISA signature before requesting the slot resources, and this caused
>> hangs during boot.
>>
>> Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>  drivers/eisa/eisa-bus.c |   26 +++++++++++++++-----------
>>  1 file changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
>> index 1b86fe0c2e80..612afeaec3cb 100644
>> --- a/drivers/eisa/eisa-bus.c
>> +++ b/drivers/eisa/eisa-bus.c
>> @@ -277,11 +277,13 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
>>               }
>>
>>               if (slot) {
>> +                     edev->res[i].name  = NULL;
>>                       edev->res[i].start = SLOT_ADDRESS(root, slot)
>>                                            + (i * 0x400);
>>                       edev->res[i].end   = edev->res[i].start + 0xff;
>>                       edev->res[i].flags = IORESOURCE_IO;
>>               } else {
>> +                     edev->res[i].name  = NULL;
>>                       edev->res[i].start = SLOT_ADDRESS(root, slot)
>>                                            + EISA_VENDOR_ID_OFFSET;
>>                       edev->res[i].end   = edev->res[i].start + 3;
>> @@ -327,19 +329,20 @@ static int __init eisa_probe(struct eisa_root_device *root)
>>               return -ENOMEM;
>>       }
>>
>> -     if (eisa_init_device(root, edev, 0)) {
>> +     if (eisa_request_resources(root, edev, 0)) {
>> +             dev_warn(root->dev,
>> +                      "EISA: Cannot allocate resource for mainboard\n");
>>               kfree(edev);
>>               if (!root->force_probe)
>> -                     return -ENODEV;
>> +                     return -EBUSY;
>>               goto force_probe;
>>       }
>>
>> -     if (eisa_request_resources(root, edev, 0)) {
>> -             dev_warn(root->dev,
>> -                      "EISA: Cannot allocate resource for mainboard\n");
>> +     if (eisa_init_device(root, edev, 0)) {
>> +             eisa_release_resources(edev);
>>               kfree(edev);
>>               if (!root->force_probe)
>> -                     return -EBUSY;
>> +                     return -ENODEV;
>>               goto force_probe;
>>       }
>>
>> @@ -362,11 +365,6 @@ static int __init eisa_probe(struct eisa_root_device *root)
>>                       continue;
>>               }
>>
>> -             if (eisa_init_device(root, edev, i)) {
>> -                     kfree(edev);
>> -                     continue;
>> -             }
>> -
>>               if (eisa_request_resources(root, edev, i)) {
>>                       dev_warn(root->dev,
>>                                "Cannot allocate resource for EISA slot %d\n",
>> @@ -375,6 +373,12 @@ static int __init eisa_probe(struct eisa_root_device *root)
>>                       continue;
>>               }
>>
>> +             if (eisa_init_device(root, edev, i)) {
>> +                     eisa_release_resources(edev);
>> +                     kfree(edev);
>> +                     continue;
>> +             }
>> +
>>               if (edev->state == (EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED))
>>                       enabled_str = " (forced enabled)";
>>               else if (edev->state == EISA_CONFIG_FORCED)
> Thanks again for looking at this, Bjorn!
>
> Yes, reverting commit 26abfeed4 does in fact resolve the bug.  I already
> built a test kernel, noted in comment #34:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/comments/34
>
> The original bug reporter tested the kernel to confirm it fixes the bug:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/comments/36

OK.  I don't know exactly what patch you applied to revert this.  I
assume that you reverted both a2080d0c561 and 26abfeed43.

> Would it be also possible for you to request the revert in the stable
> trees, in addition to 3.14?

Oh, of course, thanks for the reminder.  I added a stable tag and re-pushed.

Bjorn

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources
  2014-01-18 14:37             ` Bjorn Helgaas
@ 2014-01-22 18:00               ` Joseph Salisbury
  0 siblings, 0 replies; 9+ messages in thread
From: Joseph Salisbury @ 2014-01-22 18:00 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: LKML

On 01/18/2014 09:37 AM, Bjorn Helgaas wrote:
> On Fri, Jan 17, 2014 at 6:35 PM, Joseph Salisbury
> <joseph.salisbury@canonical.com> wrote:
>> On 01/17/2014 05:19 PM, Bjorn Helgaas wrote:
>>> On Fri, Jan 17, 2014 at 02:26:23PM -0500, Joseph Salisbury wrote:
>>>> On 01/17/2014 12:02 PM, Bjorn Helgaas wrote:
>>>>> On Thu, Jan 16, 2014 at 01:14:01PM -0500, Joseph Salisbury wrote:
>>>>>> On 01/16/2014 01:12 PM, Bjorn Helgaas wrote:
>>>>>>> On Thu, Jan 16, 2014 at 10:53 AM, Joseph Salisbury
>>>>>>> <joseph.salisbury@canonical.com> wrote:
>>>>>>>> Hi Bjorn,
>>>>>>>>
>>>>>>>> A kernel bug was opened against Ubuntu [0].  After a kernel bisect, it
>>>>>>>> was found the following commit introduced this bug:
>>>>>>> Sorry about that, and thanks for the report.  Did you mean to include
>>>>>>> URL for the bug?
>>>>>> Yes, sorry about that:
>>>>>> http://pad.lv/1251816
>>>>> Hi Joseph,
>>>>>
>>>>> Can you attach the 3.8.0-32-generic config (the one matching the successful
>>>>> boot at https://launchpadlibrarian.net/156685076/BootDmesg.txt) to the bug?
>>>> I attached the config file to the bug:
>>>> https://launchpadlibrarian.net/162754666/config.common.ubuntu
>>>>
>>>> I also attached a tar file with the complete config directory for that
>>>> kernel version.
>>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/+attachment/3951156/+files/raring-config.tar
>>> Thanks again.  I attached the following reverts to launchpad.  I screwed up
>>> when doing those EISA changes.  I'd like to squeeze these into my v3.14
>>> merge request (probably early next week), so please test and let me know
>>> if this fixes the problem.  I'm really sorry for the inconvenience.
>>>
>>> Bjorn
>>>
>>>
>>> Revert "EISA: Log device resources in dmesg"
>>>
>>> From: Bjorn Helgaas <bhelgaas@google.com>
>>>
>>> This reverts commit a2080d0c561c546d73cb8b296d4b7ca414e6860b.
>>>
>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>> ---
>>>  drivers/eisa/eisa-bus.c |    1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
>>> index 8842cde69177..1b86fe0c2e80 100644
>>> --- a/drivers/eisa/eisa-bus.c
>>> +++ b/drivers/eisa/eisa-bus.c
>>> @@ -288,7 +288,6 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
>>>                       edev->res[i].flags = IORESOURCE_IO | IORESOURCE_BUSY;
>>>               }
>>>
>>> -             dev_printk(KERN_DEBUG, &edev->dev, "%pR\n", &edev->res[i]);
>>>               if (request_resource(root->res, &edev->res[i]))
>>>                       goto failed;
>>>       }
>>> Revert "EISA: Initialize device before its resources"
>>>
>>> From: Bjorn Helgaas <bhelgaas@google.com>
>>>
>>> This reverts commit 26abfeed4341872364386c6a52b9acef8c81a81a.
>>>
>>> In the eisa_probe() force_probe path, if we were unable to request slot
>>> resources (e.g., [io 0x800-0x8ff]), we skipped the slot with "Cannot
>>> allocate resource for EISA slot %d" before reading the EISA signature in
>>> eisa_init_device().
>>>
>>> Commit 26abfeed4341 moved eisa_init_device() earlier, so we tried to read
>>> the EISA signature before requesting the slot resources, and this caused
>>> hangs during boot.
>>>
>>> Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816
>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>> ---
>>>  drivers/eisa/eisa-bus.c |   26 +++++++++++++++-----------
>>>  1 file changed, 15 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
>>> index 1b86fe0c2e80..612afeaec3cb 100644
>>> --- a/drivers/eisa/eisa-bus.c
>>> +++ b/drivers/eisa/eisa-bus.c
>>> @@ -277,11 +277,13 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
>>>               }
>>>
>>>               if (slot) {
>>> +                     edev->res[i].name  = NULL;
>>>                       edev->res[i].start = SLOT_ADDRESS(root, slot)
>>>                                            + (i * 0x400);
>>>                       edev->res[i].end   = edev->res[i].start + 0xff;
>>>                       edev->res[i].flags = IORESOURCE_IO;
>>>               } else {
>>> +                     edev->res[i].name  = NULL;
>>>                       edev->res[i].start = SLOT_ADDRESS(root, slot)
>>>                                            + EISA_VENDOR_ID_OFFSET;
>>>                       edev->res[i].end   = edev->res[i].start + 3;
>>> @@ -327,19 +329,20 @@ static int __init eisa_probe(struct eisa_root_device *root)
>>>               return -ENOMEM;
>>>       }
>>>
>>> -     if (eisa_init_device(root, edev, 0)) {
>>> +     if (eisa_request_resources(root, edev, 0)) {
>>> +             dev_warn(root->dev,
>>> +                      "EISA: Cannot allocate resource for mainboard\n");
>>>               kfree(edev);
>>>               if (!root->force_probe)
>>> -                     return -ENODEV;
>>> +                     return -EBUSY;
>>>               goto force_probe;
>>>       }
>>>
>>> -     if (eisa_request_resources(root, edev, 0)) {
>>> -             dev_warn(root->dev,
>>> -                      "EISA: Cannot allocate resource for mainboard\n");
>>> +     if (eisa_init_device(root, edev, 0)) {
>>> +             eisa_release_resources(edev);
>>>               kfree(edev);
>>>               if (!root->force_probe)
>>> -                     return -EBUSY;
>>> +                     return -ENODEV;
>>>               goto force_probe;
>>>       }
>>>
>>> @@ -362,11 +365,6 @@ static int __init eisa_probe(struct eisa_root_device *root)
>>>                       continue;
>>>               }
>>>
>>> -             if (eisa_init_device(root, edev, i)) {
>>> -                     kfree(edev);
>>> -                     continue;
>>> -             }
>>> -
>>>               if (eisa_request_resources(root, edev, i)) {
>>>                       dev_warn(root->dev,
>>>                                "Cannot allocate resource for EISA slot %d\n",
>>> @@ -375,6 +373,12 @@ static int __init eisa_probe(struct eisa_root_device *root)
>>>                       continue;
>>>               }
>>>
>>> +             if (eisa_init_device(root, edev, i)) {
>>> +                     eisa_release_resources(edev);
>>> +                     kfree(edev);
>>> +                     continue;
>>> +             }
>>> +
>>>               if (edev->state == (EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED))
>>>                       enabled_str = " (forced enabled)";
>>>               else if (edev->state == EISA_CONFIG_FORCED)
>> Thanks again for looking at this, Bjorn!
>>
>> Yes, reverting commit 26abfeed4 does in fact resolve the bug.  I already
>> built a test kernel, noted in comment #34:
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/comments/34
>>
>> The original bug reporter tested the kernel to confirm it fixes the bug:
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/comments/36
> OK.  I don't know exactly what patch you applied to revert this.  I
> assume that you reverted both a2080d0c561 and 26abfeed43.

Right, my original test kernel only had commit 26abfeed43 reverted.  I
built one more kernel with both a2080d0c561 and 26abfeed43 reverted, and
that does resolve the bug:
 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1251816/comments/44

Thanks again for the help, Bjorn!

>
>> Would it be also possible for you to request the revert in the stable
>> trees, in addition to 3.14?
> Oh, of course, thanks for the reminder.  I added a stable tag and re-pushed.
>
> Bjorn


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-01-22 18:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-16 17:53 [v3.11][v3.12][v3.13][Regression] EISA: Initialize device before its resources Joseph Salisbury
2014-01-16 18:12 ` Bjorn Helgaas
2014-01-16 18:14   ` Joseph Salisbury
2014-01-17 17:02     ` Bjorn Helgaas
2014-01-17 19:26       ` Joseph Salisbury
2014-01-17 22:19         ` Bjorn Helgaas
2014-01-18  1:35           ` Joseph Salisbury
2014-01-18 14:37             ` Bjorn Helgaas
2014-01-22 18:00               ` Joseph Salisbury

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).