Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
To: David Laight <David.Laight@ACULAB.COM>,
	'Bjorn Helgaas' <helgaas@kernel.org>
Cc: Sam Bobroff <sbobroff@linux.ibm.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux@yadro.com" <linux@yadro.com>,
	Lukas Wunner <lukas@wunner.de>,
	Oliver O'Halloran <oohall@gmail.com>,
	Rajat Jain <rajatja@google.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v5 03/23] PCI: hotplug: Add a flag for the movable BARs feature
Date: Mon, 30 Sep 2019 19:17:31 +0300
Message-ID: <18a27e45-bcba-cdc5-e07e-e73efffce4d9@yadro.com> (raw)
In-Reply-To: <16a86a9e4b464897acee0aeba34d9346@AcuMS.aculab.com>

Hello David,

On 9/30/19 11:44 AM, David Laight wrote:
> From: Bjorn Helgaas
>> Sent: 27 September 2019 23:02
>> On Fri, Aug 16, 2019 at 07:50:41PM +0300, Sergey Miroshnichenko wrote:
>>> When hot-adding a device, the bridge may have windows not big enough (or
>>> fragmented too much) for newly requested BARs to fit in. And expanding
>>> these bridge windows may be impossible because blocked by "neighboring"
>>> BARs and bridge windows.
>>>
>>> Still, it may be possible to allocate a memory region for new BARs with the
>>> following procedure:
>>>
>>> 1) notify all the drivers which support movable BARs to pause and release
>>>     the BARs; the rest of the drivers are guaranteed that their devices will
>>>     not get BARs moved;
>>>
>>> 2) release all the bridge windows except of root bridges;
>>>
>>> 3) try to recalculate new bridge windows that will fit all the BAR types:
>>>     - fixed;
>>>     - immovable;
>>>     - movable;
>>>     - newly requested by hot-added devices;
>>>
>>> 4) if the previous step fails, disable BARs for one of the hot-added
>>>     devices and retry from step 3;
>>>
>>> 5) notify the drivers, so they remap BARs and resume.
>>
>> You don't do the actual recalculation in *this* patch, but since you
>> mention the procedure here, are we confident that we never make things
>> worse?
>>
>> It's possible that a hot-add will trigger this attempt to move things
>> around, and it's possible that we won't find space for the new device
>> even if we move things around.  But are we certain that every device
>> that worked *before* the hot-add will still work *afterwards*?
>>
>> Much of the assignment was probably done by the BIOS using different
>> algorithms than Linux has, so I think there's some chance that the
>> BIOS did a better job and if we lose that BIOS assignment, we might
>> not be able to recreate it.
> 
> Yep, removing everything and starting again is probably OTT and most of the churn won't help.
> 
> I think you need to work out what can be moved in order to make the required resources available
> to each bus and then make the required changes.
> 
> In the simplest case you are trying to add resource below a bridge so need to 'shuffle'
> everything allocated after that bridge to later addresses (etc).
> 

Thank you for the review and suggestions!

But a bridge window may be fragmented: its total free space is enough
to fit everything, but no sufficient gaps for the new BARs. And this
bridge window may be jammed between two immovable/fixed BARs.

Or there may be lots of empty spaces in lower addresses after un-plugs,
but everything if fixed/immovable on higher addresses.

I've spent some time thinking on an optimization technique which can
be efficient enough (touch as few BARs as possible) with as high
success rate as calculating from scratch - and concluded that it is
not worth it: if only release the "obstructing" BARs and bridge
windows, a hotplug event will affect a half of (n+m) on average, which
is still O(n+m), where n is a number of endpoints, and m is a
number of bridges. But it's still need to resize windows of a root and
other common bridges.

Calculating bridge windows from scratch is relatively straightforward
and fast, so I have just added support for fixed/immovable BARs there
and reused.

> Many devices that support address reassignment might not need to be moved - so there is
> no point remmapping them.
> 

And it's the same algorithm that allocated BARs in first place, so it
will reassign the same BARs for the non-affected part of the topology.

> There is also the case when a device that is present but not currently is use could be taken
> through a remove+insert sequence in order to change its resources.
> Much easier to implement than 'remap while active'.
> This would require a call into the driver (than can sleep) to request whether it is idle.
> (and probably one at the end if the remove wasn't done).
> 

Unbind+rebind the "immovable" drivers of non-opened devices may
increase the probability of successful BAR allocation, but I'm afraid
this will produce some amount of false hotplug-like events in the logs.
Probably also some undesired effects like spikes in power consumption
because of driver initialization.

Best regards,
Serge

> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 

  reply index

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-16 16:50 [PATCH v5 00/23] PCI: Allow BAR movement during hotplug Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 01/23] PCI: Fix race condition in pci_enable/disable_device() Sergey Miroshnichenko
2019-08-22 12:37   ` Marta Rybczynska
2019-09-27 21:59   ` Bjorn Helgaas
2019-09-30  8:53     ` Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 02/23] PCI: Enable bridge's I/O and MEM access for hotplugged devices Sergey Miroshnichenko
2019-09-27 22:01   ` Bjorn Helgaas
2019-08-16 16:50 ` [PATCH v5 03/23] PCI: hotplug: Add a flag for the movable BARs feature Sergey Miroshnichenko
2019-09-27 22:02   ` Bjorn Helgaas
2019-09-30  8:44     ` David Laight
2019-09-30 16:17       ` Sergey Miroshnichenko [this message]
2019-09-30 12:59     ` Sergey Miroshnichenko
2019-10-15 22:14       ` Bjorn Helgaas
2019-10-16 15:50         ` Sergey Miroshnichenko
2019-10-16 17:29           ` Bjorn Helgaas
2019-08-16 16:50 ` [PATCH v5 04/23] PCI: Define PCI-specific version of the release_child_resources() Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 05/23] PCI: hotplug: movable BARs: Fix reassigning the released bridge windows Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 06/23] PCI: hotplug: movable BARs: Recalculate all bridge windows during rescan Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 07/23] PCI: hotplug: movable BARs: Don't allow added devices to steal resources Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 08/23] PCI: Include fixed and immovable BARs into the bus size calculating Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 09/23] PCI: Prohibit assigning BARs and bridge windows to non-direct parents Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 10/23] PCI: hotplug: movable BARs: Try to assign unassigned resources only once Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 11/23] PCI: hotplug: movable BARs: Calculate immovable parts of bridge windows Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 12/23] PCI: hotplug: movable BARs: Compute limits for relocated " Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 13/23] PCI: Make sure bridge windows include their fixed BARs Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 14/23] PCI: Fix assigning the fixed prefetchable resources Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 15/23] PCI: hotplug: movable BARs: Assign fixed and immovable BARs before others Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 16/23] PCI: hotplug: movable BARs: Don't reserve IO/mem bus space Sergey Miroshnichenko
2019-09-04  5:42   ` Oliver O'Halloran
2019-09-04 11:22     ` Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 17/23] powerpc/pci: Fix crash with enabled movable BARs Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 18/23] powerpc/pci: Handle BAR movement Sergey Miroshnichenko
2019-09-04  5:37   ` Oliver O'Halloran
2019-09-06 16:24     ` Sergey Miroshnichenko
2019-09-09 14:02       ` Oliver O'Halloran
2019-08-16 16:50 ` [PATCH v5 19/23] PCI: hotplug: Configure MPS for hot-added bridges during bus rescan Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 20/23] PCI: hotplug: movable BARs: Enable the feature by default Sergey Miroshnichenko
2019-08-16 16:50 ` [PATCH v5 21/23] nvme-pci: Handle movable BARs Sergey Miroshnichenko
2019-08-16 16:51 ` [PATCH v5 22/23] PCI/portdrv: Declare support of " Sergey Miroshnichenko
2019-08-16 16:51 ` [PATCH v5 23/23] PCI: pciehp: movable BARs: Trigger a domain rescan on hp events Sergey Miroshnichenko

Reply instructions:

You may reply publically 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=18a27e45-bcba-cdc5-e07e-e73efffce4d9@yadro.com \
    --to=s.miroshnichenko@yadro.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@yadro.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lukas@wunner.de \
    --cc=oohall@gmail.com \
    --cc=rajatja@google.com \
    --cc=sbobroff@linux.ibm.com \
    /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