| Message ID | 576C042B.20507@codeaurora.org |
|---|---|
| State | Not Applicable |
| Headers | show |
[+cc Alex, FYI] This thread is about a regression that we'll want to fix before v4.7 releases. It looks like it's related to these changes which were merged via the ACPI tree: 9e5ed6d1fb87 ACPI,PCI,IRQ: remove SCI penalize function 1fcb6a813c4f ACPI,PCI,IRQ: remove redundant code in acpi_irq_penalty_init() 5c5087a55390 ACPI,PCI,IRQ: reduce static IRQ array size to 16 103544d86976 ACPI,PCI,IRQ: reduce resource requirements If that turns out to be the case, it probably makes sense to merge the fix via the ACPI tree as well. But in the unlikely event the fix turns out to be in PCI, Alex will probably have to merge it because I'll be on vacation. So I'm adding Alex to the CC: list in case that happens. On Thu, Jun 23, 2016 at 11:45:47AM -0400, Sinan Kaya wrote: > On 6/23/2016 10:55 AM, Sinan Kaya wrote: > > On 6/23/2016 10:12 AM, Wim Osterholt wrote: > >> On Wed, Jun 22, 2016 at 11:54:39PM -0400, okaya@codeaurora.org wrote: > >>> On 2016-06-21 18:13, Wim Osterholt wrote: > >>>>> > >>>>> pr_info("%s:%d irq = %d penalty = %d\n", __func__, __LINE__, irq, > >>>>> penalty); > >>>>> > >>>> > >>>> This produced some 60 lines extra.... > >>> > >>> Thanks, let's go back to 4.6 and add a very similar printf to every > >>> single place where the array is modified and also right before the > >>> enabled message. > >>> > >> > >> I don't get this right. > >> Assuming that you're still talking about the same file, I find a few > >> instances of 'enabled', most of them in if-statements and one where it might > >> be set, so it looks. However, that's already in a printk statement. > >> I don't know about arrays and even less where these are set. Even worse, I > >> don't know what to put in a 'similar' line if you don't mean 'exactly the > >> same'. > >> So please state file and line numbers and the line to be inserted. > >> > > > > Sure, let me get a patch for you. I was hoping to do it yesterday. > > I ran out of time. I typed the message from my phone. > > > > Here it is > > > > -- > Sinan Kaya > Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index ededa90..228b61f 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -487,15 +487,18 @@ int __init acpi_irq_penalty_init(void) > link->irq.possible_count; > > for (i = 0; i < link->irq.possible_count; i++) { > - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) > + if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) { > acpi_irq_penalty[link->irq. > possible[i]] += > penalty; > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.possible[i], acpi_irq_penalty[link->irq.possible[i]]); > + } > } > > } else if (link->irq.active) { > acpi_irq_penalty[link->irq.active] += > PIRQ_PENALTY_PCI_POSSIBLE; > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.active, acpi_irq_penalty[link->irq.active]); > } > } > > @@ -548,8 +551,11 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) > */ > for (i = (link->irq.possible_count - 1); i >= 0; i--) { > if (acpi_irq_penalty[irq] > > - acpi_irq_penalty[link->irq.possible[i]]) > + acpi_irq_penalty[link->irq.possible[i]]) { > + pr_info("%s:%d acpi_irq_penalty[irq=%d](0x%x) vs. acpi_irq_penalty[%d](0x%x)\n", > + __func__, __LINE__, irq, acpi_irq_penalty[irq], link->irq.possible[i], acpi_irq_penalty[link->irq.possible[i]]); > irq = link->irq.possible[i]; > + } > } > } > if (acpi_irq_penalty[irq] >= PIRQ_PENALTY_ISA_ALWAYS) { > @@ -569,6 +575,7 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) > return -ENODEV; > } else { > acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING; > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.active, acpi_irq_penalty[link->irq.active]); > printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n", > acpi_device_name(link->device), > acpi_device_bid(link->device), link->irq.active); > @@ -804,6 +811,8 @@ static int __init acpi_irq_penalty_update(char *str, int used) > else > acpi_irq_penalty[irq] = PIRQ_PENALTY_PCI_AVAILABLE; > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > + > if (retval != 2) /* no next number */ > break; > } > @@ -824,11 +833,16 @@ void acpi_penalize_isa_irq(int irq, int active) > acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_USED; > else > acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; > + > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > } > } > > bool acpi_isa_irq_available(int irq) > { > + if (irq >= 0 && (irq < ARRAY_SIZE(acpi_irq_penalty))) > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > + > return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) || > acpi_irq_penalty[irq] < PIRQ_PENALTY_ISA_ALWAYS); > } > @@ -846,6 +860,8 @@ void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS; > else > acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; > + > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > } > } > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 23 Jun 2016 11:21:58 -0500 Bjorn Helgaas <helgaas@kernel.org> wrote: > [+cc Alex, FYI] > > This thread is about a regression that we'll want to fix before v4.7 > releases. It looks like it's related to these changes which were > merged via the ACPI tree: > > 9e5ed6d1fb87 ACPI,PCI,IRQ: remove SCI penalize function > 1fcb6a813c4f ACPI,PCI,IRQ: remove redundant code in acpi_irq_penalty_init() > 5c5087a55390 ACPI,PCI,IRQ: reduce static IRQ array size to 16 > 103544d86976 ACPI,PCI,IRQ: reduce resource requirements > > If that turns out to be the case, it probably makes sense to merge the fix > via the ACPI tree as well. But in the unlikely event the fix turns out to > be in PCI, Alex will probably have to merge it because I'll be on vacation. > So I'm adding Alex to the CC: list in case that happens. Thanks, I'm watching it now. I'll also encourage anyone posting other fixes that they feel are relevant to v4.7 while Bjorn is away to please proactively poke me. Bjorn is pretty strict about only accepting regression fixes after the merge window and I intend to do the same. Thanks, Alex > On Thu, Jun 23, 2016 at 11:45:47AM -0400, Sinan Kaya wrote: > > On 6/23/2016 10:55 AM, Sinan Kaya wrote: > > > On 6/23/2016 10:12 AM, Wim Osterholt wrote: > > >> On Wed, Jun 22, 2016 at 11:54:39PM -0400, okaya@codeaurora.org wrote: > > >>> On 2016-06-21 18:13, Wim Osterholt wrote: > > >>>>> > > >>>>> pr_info("%s:%d irq = %d penalty = %d\n", __func__, __LINE__, irq, > > >>>>> penalty); > > >>>>> > > >>>> > > >>>> This produced some 60 lines extra.... > > >>> > > >>> Thanks, let's go back to 4.6 and add a very similar printf to every > > >>> single place where the array is modified and also right before the > > >>> enabled message. > > >>> > > >> > > >> I don't get this right. > > >> Assuming that you're still talking about the same file, I find a few > > >> instances of 'enabled', most of them in if-statements and one where it might > > >> be set, so it looks. However, that's already in a printk statement. > > >> I don't know about arrays and even less where these are set. Even worse, I > > >> don't know what to put in a 'similar' line if you don't mean 'exactly the > > >> same'. > > >> So please state file and line numbers and the line to be inserted. > > >> > > > > > > Sure, let me get a patch for you. I was hoping to do it yesterday. > > > I ran out of time. I typed the message from my phone. > > > > > > > Here it is > > > > > > > > -- > > Sinan Kaya > > Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project > > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > > index ededa90..228b61f 100644 > > --- a/drivers/acpi/pci_link.c > > +++ b/drivers/acpi/pci_link.c > > @@ -487,15 +487,18 @@ int __init acpi_irq_penalty_init(void) > > link->irq.possible_count; > > > > for (i = 0; i < link->irq.possible_count; i++) { > > - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) > > + if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) { > > acpi_irq_penalty[link->irq. > > possible[i]] += > > penalty; > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.possible[i], acpi_irq_penalty[link->irq.possible[i]]); > > + } > > } > > > > } else if (link->irq.active) { > > acpi_irq_penalty[link->irq.active] += > > PIRQ_PENALTY_PCI_POSSIBLE; > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.active, acpi_irq_penalty[link->irq.active]); > > } > > } > > > > @@ -548,8 +551,11 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) > > */ > > for (i = (link->irq.possible_count - 1); i >= 0; i--) { > > if (acpi_irq_penalty[irq] > > > - acpi_irq_penalty[link->irq.possible[i]]) > > + acpi_irq_penalty[link->irq.possible[i]]) { > > + pr_info("%s:%d acpi_irq_penalty[irq=%d](0x%x) vs. acpi_irq_penalty[%d](0x%x)\n", > > + __func__, __LINE__, irq, acpi_irq_penalty[irq], link->irq.possible[i], acpi_irq_penalty[link->irq.possible[i]]); > > irq = link->irq.possible[i]; > > + } > > } > > } > > if (acpi_irq_penalty[irq] >= PIRQ_PENALTY_ISA_ALWAYS) { > > @@ -569,6 +575,7 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) > > return -ENODEV; > > } else { > > acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING; > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.active, acpi_irq_penalty[link->irq.active]); > > printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n", > > acpi_device_name(link->device), > > acpi_device_bid(link->device), link->irq.active); > > @@ -804,6 +811,8 @@ static int __init acpi_irq_penalty_update(char *str, int used) > > else > > acpi_irq_penalty[irq] = PIRQ_PENALTY_PCI_AVAILABLE; > > > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > > + > > if (retval != 2) /* no next number */ > > break; > > } > > @@ -824,11 +833,16 @@ void acpi_penalize_isa_irq(int irq, int active) > > acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_USED; > > else > > acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; > > + > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > > } > > } > > > > bool acpi_isa_irq_available(int irq) > > { > > + if (irq >= 0 && (irq < ARRAY_SIZE(acpi_irq_penalty))) > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > > + > > return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) || > > acpi_irq_penalty[irq] < PIRQ_PENALTY_ISA_ALWAYS); > > } > > @@ -846,6 +860,8 @@ void acpi_penalize_sci_irq(int irq, int trigger, int polarity) > > acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS; > > else > > acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; > > + > > + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); > > } > > } > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 23, 2016 at 11:45:47AM -0400, Sinan Kaya wrote: > > > > Sure, let me get a patch for you. > > Here it is http://webserver.djo.tudelft.nl/dmesg460+printpatch2 > I am trying to find a system with similar characteristics for debug All from the same laptop, Dell Inspiron 4100. The same problem arises at a Dell Inspiron 510m. I've not seen it on a workstation Dell XW4300. Groeten, Wim. ----- wim@djo.tudelft.nl ----- -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index ededa90..228b61f 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -487,15 +487,18 @@ int __init acpi_irq_penalty_init(void) link->irq.possible_count; for (i = 0; i < link->irq.possible_count; i++) { - if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) + if (link->irq.possible[i] < ACPI_MAX_ISA_IRQ) { acpi_irq_penalty[link->irq. possible[i]] += penalty; + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.possible[i], acpi_irq_penalty[link->irq.possible[i]]); + } } } else if (link->irq.active) { acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_POSSIBLE; + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.active, acpi_irq_penalty[link->irq.active]); } } @@ -548,8 +551,11 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) */ for (i = (link->irq.possible_count - 1); i >= 0; i--) { if (acpi_irq_penalty[irq] > - acpi_irq_penalty[link->irq.possible[i]]) + acpi_irq_penalty[link->irq.possible[i]]) { + pr_info("%s:%d acpi_irq_penalty[irq=%d](0x%x) vs. acpi_irq_penalty[%d](0x%x)\n", + __func__, __LINE__, irq, acpi_irq_penalty[irq], link->irq.possible[i], acpi_irq_penalty[link->irq.possible[i]]); irq = link->irq.possible[i]; + } } } if (acpi_irq_penalty[irq] >= PIRQ_PENALTY_ISA_ALWAYS) { @@ -569,6 +575,7 @@ static int acpi_pci_link_allocate(struct acpi_pci_link *link) return -ENODEV; } else { acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING; + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, link->irq.active, acpi_irq_penalty[link->irq.active]); printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n", acpi_device_name(link->device), acpi_device_bid(link->device), link->irq.active); @@ -804,6 +811,8 @@ static int __init acpi_irq_penalty_update(char *str, int used) else acpi_irq_penalty[irq] = PIRQ_PENALTY_PCI_AVAILABLE; + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); + if (retval != 2) /* no next number */ break; } @@ -824,11 +833,16 @@ void acpi_penalize_isa_irq(int irq, int active) acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_USED; else acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; + + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); } } bool acpi_isa_irq_available(int irq) { + if (irq >= 0 && (irq < ARRAY_SIZE(acpi_irq_penalty))) + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); + return irq >= 0 && (irq >= ARRAY_SIZE(acpi_irq_penalty) || acpi_irq_penalty[irq] < PIRQ_PENALTY_ISA_ALWAYS); } @@ -846,6 +860,8 @@ void acpi_penalize_sci_irq(int irq, int trigger, int polarity) acpi_irq_penalty[irq] += PIRQ_PENALTY_ISA_ALWAYS; else acpi_irq_penalty[irq] += PIRQ_PENALTY_PCI_USING; + + pr_info("%s:%d acpi_irq_penalty[%d] = 0x%x\n", __func__, __LINE__, irq, acpi_irq_penalty[irq]); } }