Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Zeno Davatz]

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]

Hi

I am getting this hang with kernel 6.1. See attachment.

Is this related to your patch?

Kernel 6.1 does not boot for me. Kernel 6.0 was fine.

Thank you for your reply.

Best
Zeno

[-- Attachment #2: image0.jpeg --]
[-- Type: image/jpeg, Size: 1538042 bytes --]

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Baolu Lu]

On 2022/12/25 20:11, Zeno Davatz wrote:
> Hi
> 
> I am getting this hang with kernel 6.1. See attachment.
> 
> Is this related to your patch?
> 
> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.

It seems unlikely.

Have you located the patch through the bisect tool? Or does it boot well
if you revert this patch?

--
Best regards,
baolu

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Zeno Davatz]

Dear Baolu

On Sun, Dec 25, 2022 at 2:16 PM Baolu Lu <baolu.lu@linux.intel.com> wrote:
>
> On 2022/12/25 20:11, Zeno Davatz wrote:
> > Hi
> >
> > I am getting this hang with kernel 6.1. See attachment.
> >
> > Is this related to your patch?
> >
> > Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
>
> It seems unlikely.
>
> Have you located the patch through the bisect tool? Or does it boot well
> if you revert this patch?

I did not bisect the patch. All I currently know is that version 5.5
and version 6.0 of the Kernel booted fine for me.

I test every Kernel release and normally the boot process goes flawless.

The patch I am referring to is also mentioned by Linus in the Kernel
log, AFAICT tell the only PCI patch.

https://lkml.org/lkml/2022/12/11/206

Best
Zeno

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Baolu Lu]

On 2022/12/25 21:37, Zeno Davatz wrote:
> On Sun, Dec 25, 2022 at 2:16 PM Baolu Lu<baolu.lu@linux.intel.com>  wrote:
>> On 2022/12/25 20:11, Zeno Davatz wrote:
>>> Hi
>>>
>>> I am getting this hang with kernel 6.1. See attachment.
>>>
>>> Is this related to your patch?
>>>
>>> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
>> It seems unlikely.
>>
>> Have you located the patch through the bisect tool? Or does it boot well
>> if you revert this patch?
> I did not bisect the patch. All I currently know is that version 5.5
> and version 6.0 of the Kernel booted fine for me.
> 
> I test every Kernel release and normally the boot process goes flawless.
> 
> The patch I am referring to is also mentioned by Linus in the Kernel
> log, AFAICT tell the only PCI patch.
> 
> https://lkml.org/lkml/2022/12/11/206

I didn't find any reference to this patch in above thread. Can you
please elaborate a bit more?

By the way, from the video you shared, the hang happens in the PCI
enumeration process. This patch touched the code that only executes
after that.

Can you please simply revert this patch and check the boot again?

--
Best regards,
baolu

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Zeno Davatz]

Dear Baolu

On Sun, Dec 25, 2022 at 2:52 PM Baolu Lu <baolu.lu@linux.intel.com> wrote:
>
> On 2022/12/25 21:37, Zeno Davatz wrote:
> > On Sun, Dec 25, 2022 at 2:16 PM Baolu Lu<baolu.lu@linux.intel.com>  wrote:
> >> On 2022/12/25 20:11, Zeno Davatz wrote:
> >>> Hi
> >>>
> >>> I am getting this hang with kernel 6.1. See attachment.
> >>>
> >>> Is this related to your patch?
> >>>
> >>> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
> >> It seems unlikely.
> >>
> >> Have you located the patch through the bisect tool? Or does it boot well
> >> if you revert this patch?
> > I did not bisect the patch. All I currently know is that version 5.5
> > and version 6.0 of the Kernel booted fine for me.
> >
> > I test every Kernel release and normally the boot process goes flawless.
> >
> > The patch I am referring to is also mentioned by Linus in the Kernel
> > log, AFAICT tell the only PCI patch.
> >
> > https://lkml.org/lkml/2022/12/11/206
>
> I didn't find any reference to this patch in above thread. Can you
> please elaborate a bit more?
>
> By the way, from the video you shared, the hang happens in the PCI
> enumeration process. This patch touched the code that only executes
> after that.
>
> Can you please simply revert this patch and check the boot again?

Yes, I can try that. What is the git checkout hash, where I have to checkout at?

This hang is very strange. Did something change in the Kernel
enumeration process?

Best
Zeno

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Zeno Davatz]

Dear Baolu

On Sun, Dec 25, 2022 at 2:52 PM Baolu Lu <baolu.lu@linux.intel.com> wrote:
>
> On 2022/12/25 21:37, Zeno Davatz wrote:
> > On Sun, Dec 25, 2022 at 2:16 PM Baolu Lu<baolu.lu@linux.intel.com>  wrote:
> >> On 2022/12/25 20:11, Zeno Davatz wrote:
> >>> Hi
> >>>
> >>> I am getting this hang with kernel 6.1. See attachment.
> >>>
> >>> Is this related to your patch?
> >>>
> >>> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
> >> It seems unlikely.
> >>
> >> Have you located the patch through the bisect tool? Or does it boot well
> >> if you revert this patch?
> > I did not bisect the patch. All I currently know is that version 5.5
> > and version 6.0 of the Kernel booted fine for me.
> >
> > I test every Kernel release and normally the boot process goes flawless.
> >
> > The patch I am referring to is also mentioned by Linus in the Kernel
> > log, AFAICT tell the only PCI patch.
> >
> > https://lkml.org/lkml/2022/12/11/206
>
> I didn't find any reference to this patch in above thread. Can you
> please elaborate a bit more?
>
> By the way, from the video you shared, the hang happens in the PCI
> enumeration process. This patch touched the code that only executes
> after that.
>
> Can you please simply revert this patch and check the boot again?

I am also seeing some "refcount" mentions here:
https://lore.kernel.org/lkml/CAHk-=wgyXu4D44b8wQU9dpTYUft6WhZ0wr3nV1fziy6k0GwCCw@mail.gmail.com/

Best
Zeno

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Zeno Davatz]

Dear Baolu

On Sun, Dec 25, 2022 at 2:52 PM Baolu Lu <baolu.lu@linux.intel.com> wrote:
>
> On 2022/12/25 21:37, Zeno Davatz wrote:
> > On Sun, Dec 25, 2022 at 2:16 PM Baolu Lu<baolu.lu@linux.intel.com>  wrote:
> >> On 2022/12/25 20:11, Zeno Davatz wrote:
> >>> Hi
> >>>
> >>> I am getting this hang with kernel 6.1. See attachment.
> >>>
> >>> Is this related to your patch?
> >>>
> >>> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
> >> It seems unlikely.
> >>
> >> Have you located the patch through the bisect tool? Or does it boot well
> >> if you revert this patch?
> > I did not bisect the patch. All I currently know is that version 5.5
> > and version 6.0 of the Kernel booted fine for me.
> >
> > I test every Kernel release and normally the boot process goes flawless.
> >
> > The patch I am referring to is also mentioned by Linus in the Kernel
> > log, AFAICT tell the only PCI patch.
> >
> > https://lkml.org/lkml/2022/12/11/206
>
> I didn't find any reference to this patch in above thread. Can you
> please elaborate a bit more?
>
> By the way, from the video you shared, the hang happens in the PCI
> enumeration process. This patch touched the code that only executes
> after that.
>
> Can you please simply revert this patch and check the boot again?

The problem happens somewhere between Kernel Version 6.0 and 6.1-rc1.

So my Boot is fine with Kernel 6.0 but stops working with Kernel
version 6.1-rc1.

Best
Zeno

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Zeno Davatz]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

Hi

> Am 25.12.2022 um 14:16 schrieb Baolu Lu <baolu.lu@linux.intel.com>:
> 
> On 2022/12/25 20:11, Zeno Davatz wrote:
>> Hi
>> I am getting this hang with kernel 6.1. See attachment.
>> Is this related to your patch?
>> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
> 
> It seems unlikely.
> 
> Have you located the patch through the bisect tool? Or does it boot well
> if you revert this patch?

This is the video of the boot process that hangs:


[-- Attachment #2: Video.mov --]
[-- Type: video/quicktime, Size: 2911691 bytes --]

[-- Attachment #3: Type: text/plain, Size: 11 bytes --]



Best
Zeno

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci() #forregzbot

[Thorsten Leemhuis]

[Note: this mail contains only information for Linux kernel regression
tracking. Mails like these contain '#forregzbot' in the subject to make
then easy to spot and filter out. The author also tried to remove most
or all individuals from the list of recipients to spare them the hassle.]

On 25.12.22 13:11, Zeno Davatz wrote:
> Hi
> 
> I am getting this hang with kernel 6.1. See attachment.
> 
> Is this related to your patch?
> 
> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
> 
> Thank you for your reply.

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced v6.0..v6.1-rc1
#regzbot title pci(?)/iommu(?): hang with kernel 6.1
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci() #forregzbot

[Baolu Lu]

On 2022/12/26 21:29, Thorsten Leemhuis wrote:
> [Note: this mail contains only information for Linux kernel regression
> tracking. Mails like these contain '#forregzbot' in the subject to make
> then easy to spot and filter out. The author also tried to remove most
> or all individuals from the list of recipients to spare them the hassle.]
> 
> On 25.12.22 13:11, Zeno Davatz wrote:
>> Hi
>>
>> I am getting this hang with kernel 6.1. See attachment.
>>
>> Is this related to your patch?
>>
>> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
>>
>> Thank you for your reply.
> 
> Thanks for the report. To be sure below issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
> tracking bot:
> 
> #regzbot ^introduced v6.0..v6.1-rc1
> #regzbot title pci(?)/iommu(?): hang with kernel 6.1
> #regzbot ignore-activity
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I deal with a lot of
> reports and sometimes miss something important when writing mails like
> this. If that's the case here, don't hesitate to tell me in a public
> reply, it's in everyone's interest to set the public record straight.
> 

The reporter said that "The problem happens somewhere between Kernel
Version 6.0 and 6.1-rc1." This patch only shows up since v6.1-rc8. This
patch cannot generate any kernel regression on v6.1-rc1.

--
Best regards,
baolu

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Thorsten Leemhuis]

On 27.12.22 08:20, Baolu Lu wrote:
> On 2022/12/26 21:29, Thorsten Leemhuis wrote:
>> [Note: this mail contains only information for Linux kernel regression
>> tracking. Mails like these contain '#forregzbot' in the subject to make
>> then easy to spot and filter out. The author also tried to remove most
>> or all individuals from the list of recipients to spare them the hassle.]
>>
>> On 25.12.22 13:11, Zeno Davatz wrote:
>>> Hi
>>>
>>> I am getting this hang with kernel 6.1. See attachment.
>>>
>>> Is this related to your patch?
>>>
>>> Kernel 6.1 does not boot for me. Kernel 6.0 was fine.
>>>
>>> Thank you for your reply.
>>
>> Thanks for the report. To be sure below issue doesn't fall through the
>> cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
>> tracking bot:
>>
>> #regzbot ^introduced v6.0..v6.1-rc1
>> #regzbot title pci(?)/iommu(?): hang with kernel 6.1
>> #regzbot ignore-activity
>>
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>
>> P.S.: As the Linux kernel's regression tracker I deal with a lot of
>> reports and sometimes miss something important when writing mails like
>> this. If that's the case here, don't hesitate to tell me in a public
>> reply, it's in everyone's interest to set the public record straight.
>>
> 
> The reporter said that "The problem happens somewhere between Kernel
> Version 6.0 and 6.1-rc1." 

Don't worry, I know, that's why I used "#regzbot ^introduced
v6.0..v6.1-rc1" above.

> This patch only shows up since v6.1-rc8. This
> patch cannot generate any kernel regression on v6.1-rc1.

I'm not blaming the patch mentioned in the subject, but this is the
report about an regression that shouldn't be forgotten about, hence I
track it, even if the discussion uses a misleading subject (hopefully
someone will change that).

Or am I missing something here?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci() #forregzbot

[Bjorn Helgaas]

On Mon, Dec 26, 2022 at 02:29:39PM +0100, Thorsten Leemhuis wrote:
> [Note: this mail contains only information for Linux kernel regression
> tracking. Mails like these contain '#forregzbot' in the subject to make
> then easy to spot and filter out. The author also tried to remove most
> or all individuals from the list of recipients to spare them the hassle.]

> #regzbot ^introduced v6.0..v6.1-rc1
> #regzbot title pci(?)/iommu(?): hang with kernel 6.1
> #regzbot ignore-activity

#regzbot dup-of: https://bugzilla.kernel.org/show_bug.cgi?id=216859
#regzbot introduced: 145eed48de27

Re: [PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci() #forregzbot

[Bjorn Helgaas]

On Wed, Feb 01, 2023 at 05:24:21PM -0600, Bjorn Helgaas wrote:
> On Mon, Dec 26, 2022 at 02:29:39PM +0100, Thorsten Leemhuis wrote:
> > [Note: this mail contains only information for Linux kernel regression
> > tracking. Mails like these contain '#forregzbot' in the subject to make
> > then easy to spot and filter out. The author also tried to remove most
> > or all individuals from the list of recipients to spare them the hassle.]
> 
> > #regzbot ^introduced v6.0..v6.1-rc1
> > #regzbot title pci(?)/iommu(?): hang with kernel 6.1
> > #regzbot ignore-activity
> 
> #regzbot dup-of: https://bugzilla.kernel.org/show_bug.cgi?id=216859
> #regzbot introduced: 145eed48de27

#regzbot fix: 04119ab1a49f ("nvidiafb: detect the hardware support before removing console.")

[PATCH 0/2] iommu/vt-d: Fix PCI device reference count leak

[Xiongfeng Wang]

for_each_pci_dev() is implemented by pci_get_device(). The comment of
pci_get_device() says that it will increase the reference count for the
returned pci_dev and also decrease the reference count for the input
pci_dev @from if it is not NULL.

If we break for_each_pci_dev() loop with pdev not NULL, we need to call
pci_dev_put() to decrease the reference count.

Xiongfeng Wang (2):
  iommu/vt-d: Fix PCI device reference count leak in has_external pci()
  iommu/vt-d: Fix PCI device reference count leak in
    dmar_dev_scope_init()

 drivers/iommu/intel/dmar.c  | 1 +
 drivers/iommu/intel/iommu.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.20.1

[PATCH 1/2] iommu/vt-d: Fix PCI device reference count leak in has_external pci()

[Xiongfeng Wang]

for_each_pci_dev() is implemented by pci_get_device(). The comment of
pci_get_device() says that it will increase the reference count for the
returned pci_dev and also decrease the reference count for the input
pci_dev @from if it is not NULL.

If we break for_each_pci_dev() loop with pdev not NULL, we need to call
pci_dev_put() to decrease the reference count. Add the missing
pci_dev_put() before 'return true' to avoid reference count leak.

Fixes: 89a6079df791 ("iommu/vt-d: Force IOMMU on for platform opt in hint")
Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
---
 drivers/iommu/intel/iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 996a8b5ee5ee..1343b13fb1c2 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3854,8 +3854,10 @@ static inline bool has_external_pci(void)
 	struct pci_dev *pdev = NULL;
 
 	for_each_pci_dev(pdev)
-		if (pdev->external_facing)
+		if (pdev->external_facing) {
+			pci_dev_put(pdev);
 			return true;
+		}
 
 	return false;
 }
-- 
2.20.1