Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Logan Gunthorpe <logang@deltatee.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux PCI <linux-pci@vger.kernel.org>,
	Kit Chow <kchow@gigaio.com>, Yinghai Lu <yinghai@kernel.org>
Subject: Re: [PATCH 1/2] PCI: Prevent 64-bit resources from being counted in 32-bit bridge region
Date: Mon, 4 Mar 2019 13:21:54 -0700
Message-ID: <ac64ba0e-e86a-f7ae-775b-753f933f178f@deltatee.com> (raw)
In-Reply-To: <CAErSpo4U6Kys5OzPwfyFdbnJWfTZ-y0P830okf11BwfNv9Y82A@mail.gmail.com>



On 2019-03-04 1:11 p.m., Bjorn Helgaas wrote:
> On Mon, Mar 4, 2019 at 1:21 PM Logan Gunthorpe <logang@deltatee.com> wrote:
>> On 2019-03-03 5:23 p.m., Bjorn Helgaas wrote:
>>> Sorry for the delay.  This code gives a headache.  I still remember
>>> my headache from the last time we touched it.  Help me understand
>>> what's going on here.
>>
>> Yes, this code gave me a headache debugging it too. And it's not the
>> first time I've tried to figure out what's going on with it because it
>> often just prints noisy messages that look like errors. I think I
>> understand it better now but it's something that's a bit fleeting and
>> easy to forget the details of. There may also be other solutions to this
>> problem.
> 
> Thanks for the explanation below.  I haven't worked through it yet, but I will.
> 
> Obviously it would be far better than an explanation if we could
> simplify the code (and the noisy messages) such that it didn't
> *require* so much explanation.

I agree, but reworking this code scares me and I suspect it was designed
this way for a reason. I'm guessing there are a lot of corner cases and
unusual bios issues this stuff works around. We might end up fixing a
some cases and breaking a bunch of other cases.

It would probably be a lot simpler if (for 'realloc', at least) it
unassigns everything then only does one pass. It wouldn't make the code
itself much simpler but it would might make it easier to reason about
and debug; and would also remove a lot of the noisy messages. I suspect
the multi-pass setup would still be required for cases where the bios
doesn't assign a device or whatever it's doing and it was probably
designed this way to try and keep as many of the bios assignments the
same as possible, though I'm not really sure if that would be necessary.

Logan

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-14 17:00 Logan Gunthorpe
2019-02-14 17:00 ` [PATCH 2/2] PCI: Fix disabling of bridge BARs when assigning bus resources Logan Gunthorpe
2019-03-04  0:23 ` [PATCH 1/2] PCI: Prevent 64-bit resources from being counted in 32-bit bridge region Bjorn Helgaas
2019-03-04 19:21   ` Logan Gunthorpe
2019-03-04 20:11     ` Bjorn Helgaas
2019-03-04 20:21       ` Logan Gunthorpe [this message]
2019-03-04 20:29         ` Bjorn Helgaas
2019-03-04 20:39           ` Logan Gunthorpe
2019-04-01 19:22       ` Logan Gunthorpe
2019-03-04 23:58     ` Logan Gunthorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ac64ba0e-e86a-f7ae-775b-753f933f178f@deltatee.com \
    --to=logang@deltatee.com \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=kchow@gigaio.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git