Jesse Barnes | 1 Mar 01:27 2012

Re: [PATCH 39/39] x86, PCI: kill busn in acpi pci_root_info

On Wed, 29 Feb 2012 15:51:28 -0800
Greg KH <gregkh <at> linuxfoundation.org> wrote:

> On Wed, Feb 29, 2012 at 03:37:53PM -0800, Yinghai Lu wrote:
> > On Wed, Feb 29, 2012 at 3:32 PM, Bjorn Helgaas

> > > You just *added* this stuff in a prior patch that hasn't been
> > > merged yet.  Why can't you just fix that series rather than doing
> > > the add/remove churn?
> > 
> > as i said before, I'm not quite sure about the life cycle about
> > that object.

I thought Bjorn figured that out for you in the last thread?

> > 
> > still need to wait some months to verify that on system that does
> > support pci root bus hot plug etc.
> > 
> > or we can just this patch for now.
> 
> A statement like that would cause all of these patches to be instantly
> deleted from any queue that I had control over, and I strongly
> recommend that Jesse just ignore them all.
> 
> If you don't know this thing, then you have no right to change it,
> flat out.  Why do we trust these patches from you?  I sure don't.

Oh don't worry, this patch set isn't going upstream anytime soon.
Bjorn has raised several good points that Yinghai has yet to
(Continue reading)

Bjorn Helgaas | 1 Mar 01:42 2012
Picon

Re: [PATCH 02/39] x86, PCI: have own version for pcibios_bus_to_resource

On Wed, Feb 29, 2012 at 4:33 PM, Yinghai Lu <yinghai <at> kernel.org> wrote:
> On Wed, Feb 29, 2012 at 3:20 PM, Bjorn Helgaas <bhelgaas <at> google.com> wrote:
>> On Wed, Feb 29, 2012 at 4:07 PM, Yinghai Lu <yinghai <at> kernel.org> wrote:
>>> x86 does not need to offset the address. So we can skip that costing offset
>>> searching.
>>
>> I tried to start a discussion about this patch (and others), but I
>> don't think you responded:
>> http://marc.info/?l=linux-pci&m=133036414506921&w=2
>
> this patch does reduce some not needed ops.
>
> and it does not affects your effects,

It does affect my efforts in that this patch adds back x86 complexity
that I don't think is necessary.

And this patch makes it so the pci_add_resource_offset() interface
exists and appears to work on x86, but if somebody tries to use it, it
*doesn't* work.  I don't like to write code like that.  I think it's
poor style.

> and it just make x86 not get punished.

What punishment are you worried about?  I really don't think you'll be
able to measure any performance impact.

I agree that the x86 code you add *is* simpler than the generic
version.  But I think the *overall* complexity is higher because now
you have to look at two versions (generic and x86) and convince
(Continue reading)

Hao, Xudong | 1 Mar 02:25 2012
Picon

RE: [PATCH V2] Quirk for IVB graphics FLR errata

> -----Original Message-----
> From: Jesse Barnes [mailto:jbarnes <at> virtuousgeek.org]
> Sent: Thursday, March 01, 2012 2:32 AM
> To: Hao, Xudong
> Cc: linux-pci <at> vger.kernel.org; linux-kernel <at> vger.kernel.org;
> kvm <at> vger.kernel.org; Kay, Allen M; Zhang, Xiantao
> Subject: Re: [PATCH V2] Quirk for IVB graphics FLR errata
> 
> On Wed, 29 Feb 2012 03:11:24 +0000
> "Hao, Xudong" <xudong.hao <at> intel.com> wrote:
> 
> > For IvyBridge Mobile platform, a system hang may occur if a
> > FLR(Function Level Reset) is asserted to internal graphics.
> >
> > This quirk patch is workaround for the IVB FLR errata issue.
> > We are disabling the FLR reset handshake between the PCH and CPU
> > display, then manually powering down the panel power sequencing and
> > resetting the PCH display.
> >
> > Signed-off-by: Xudong Hao <xudong.hao <at> intel.com>
> > Signed-off-by: Kay, Allen M <allen.m.kay <at> intel.com>
> > ---
> >  drivers/pci/quirks.c |   49
> > +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed,
> 49
> > insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > 6476547..5223b80 100644 --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
(Continue reading)

Yinghai Lu | 1 Mar 03:54 2012

Re: [PATCH 02/39] x86, PCI: have own version for pcibios_bus_to_resource

On Wed, Feb 29, 2012 at 4:42 PM, Bjorn Helgaas <bhelgaas <at> google.com> wrote:
> On Wed, Feb 29, 2012 at 4:33 PM, Yinghai Lu <yinghai <at> kernel.org> wrote:
>> On Wed, Feb 29, 2012 at 3:20 PM, Bjorn Helgaas <bhelgaas <at> google.com> wrote:
>>> On Wed, Feb 29, 2012 at 4:07 PM, Yinghai Lu <yinghai <at> kernel.org> wrote:
>>>> x86 does not need to offset the address. So we can skip that costing offset
>>>> searching.
>>>
>>> I tried to start a discussion about this patch (and others), but I
>>> don't think you responded:
>>> http://marc.info/?l=linux-pci&m=133036414506921&w=2
>>
>> this patch does reduce some not needed ops.
>>
>> and it does not affects your effects,
>
> It does affect my efforts in that this patch adds back x86 complexity
> that I don't think is necessary.
>
> And this patch makes it so the pci_add_resource_offset() interface
> exists and appears to work on x86, but if somebody tries to use it, it
> *doesn't* work.  I don't like to write code like that.  I think it's
> poor style.
>
>> and it just make x86 not get punished.
>
> What punishment are you worried about?  I really don't think you'll be
> able to measure any performance impact.
>
> I agree that the x86 code you add *is* simpler than the generic
> version.  But I think the *overall* complexity is higher because now
(Continue reading)

Yinghai Lu | 1 Mar 03:57 2012

Re: [PATCH 39/39] x86, PCI: kill busn in acpi pci_root_info

On Wed, Feb 29, 2012 at 4:27 PM, Jesse Barnes <jbarnes <at> virtuousgeek.org> wrote:
> On Wed, 29 Feb 2012 15:51:28 -0800
> Greg KH <gregkh <at> linuxfoundation.org> wrote:
>
>> On Wed, Feb 29, 2012 at 03:37:53PM -0800, Yinghai Lu wrote:
>> > On Wed, Feb 29, 2012 at 3:32 PM, Bjorn Helgaas
>
>> > > You just *added* this stuff in a prior patch that hasn't been
>> > > merged yet.  Why can't you just fix that series rather than doing
>> > > the add/remove churn?
>> >
>> > as i said before, I'm not quite sure about the life cycle about
>> > that object.
>
> I thought Bjorn figured that out for you in the last thread?
>
>> >
>> > still need to wait some months to verify that on system that does
>> > support pci root bus hot plug etc.
>> >
>> > or we can just this patch for now.
>>
>> A statement like that would cause all of these patches to be instantly
>> deleted from any queue that I had control over, and I strongly
>> recommend that Jesse just ignore them all.
>>
>> If you don't know this thing, then you have no right to change it,
>> flat out.  Why do we trust these patches from you?  I sure don't.
>
> Oh don't worry, this patch set isn't going upstream anytime soon.
(Continue reading)

Yinghai Lu | 1 Mar 04:00 2012

[PATCH 03/36] PCI: rename pci_host_bridge() to find_pci_root_bridge()

Separate find_pci_root_bus out.

-v2: according to Bjorn, remove extra null checking.

Signed-off-by: Yinghai Lu <yinghai <at> kernel.org>
---
 drivers/pci/host-bridge.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 5ca4220..78adcc0 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
 <at>  <at>  -16,15 +16,22  <at>  <at>  void add_to_pci_host_bridges(struct pci_host_bridge *bridge)
 	list_add_tail(&bridge->list, &pci_host_bridges);
 }

-static struct pci_host_bridge *pci_host_bridge(struct pci_dev *dev)
+static struct pci_bus *find_pci_root_bus(struct pci_dev *dev)
 {
 	struct pci_bus *bus;
-	struct pci_host_bridge *bridge;

 	bus = dev->bus;
 	while (bus->parent)
 		bus = bus->parent;

+	return bus;
+}
+
(Continue reading)

Yinghai Lu | 1 Mar 04:00 2012

[PATCH 02/36] x86, PCI: Fix memleak with get_current_resources

in pci_scan_acpi_root, when pci_use_crs is set, get_current_resources is used
to get pci_root_info, and it will allocate name and res array.

later if pci_create_root_bus can not create bus (could be already there...)
it will only free bus res list. but the name and res array is not freed.

let get_current_resource take info pointer instead have local info.

Signed-off-by: Yinghai Lu <yinghai <at> kernel.org>
---
 arch/x86/pci/acpi.c |   49 ++++++++++++++++++++++++++++++-------------------
 1 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 7cc0a44..304ccf0 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
 <at>  <at>  -305,49 +305,55  <at>  <at>  static void add_resources(struct pci_root_info *info)
 	}
 }

+static void free_pci_root_info(struct pci_root_info *info)
+{
+	kfree(info->name);
+	kfree(info->res);
+	memset(info, 0, sizeof(struct pci_root_info));
+}
+
 static void
-get_current_resources(struct acpi_device *device, int busnum,
(Continue reading)

Yinghai Lu | 1 Mar 04:00 2012

[PATCH 07/36] x86, PCI: add host bridge resource release for using _CRS

1. allocate pci_root_info instead of use local stack. we need to pass around info
   for release fn.
2. add release_pci_root_info
3. set x86 own host bridge release fn, so will make sure root bridge
   related resources get freed during root bus removal.

Signed-off-by: Yinghai Lu <yinghai <at> kernel.org>
---
 arch/x86/pci/acpi.c |   63 +++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 763f7bb..c7a230f 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
 <at>  <at>  -305,11 +305,40  <at>  <at>  static void add_resources(struct pci_root_info *info,
 	}
 }

-static void free_pci_root_info(struct pci_root_info *info)
+static void free_pci_root_info_res(struct pci_root_info *info)
 {
 	kfree(info->name);
 	kfree(info->res);
-	memset(info, 0, sizeof(struct pci_root_info));
+	info->res = NULL;
+	info->res_num = 0;
+}
+
+static void __release_pci_root_info(struct pci_root_info *info)
(Continue reading)

Yinghai Lu | 1 Mar 04:00 2012

[PATCH 08/36] x86, PCI: embed name acpi version pci_root_info struct

Now We allocate info, and keep that in during whole life of hostbridge.

So don't allocate info->name seperately, just put name into struct.

Signed-off-by: Yinghai Lu <yinghai <at> kernel.org>
---
 arch/x86/pci/acpi.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index c7a230f..11c3ae7 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
 <at>  <at>  -9,7 +9,7  <at>  <at> 

 struct pci_root_info {
 	struct acpi_device *bridge;
-	char *name;
+	char name[16];
 	unsigned int res_num;
 	struct resource *res;
 	int busnum;
 <at>  <at>  -307,7 +307,6  <at>  <at>  static void add_resources(struct pci_root_info *info,

 static void free_pci_root_info_res(struct pci_root_info *info)
 {
-	kfree(info->name);
 	kfree(info->res);
 	info->res = NULL;
 	info->res_num = 0;
(Continue reading)

Yinghai Lu | 1 Mar 04:00 2012

[PATCH 29/36] PCI: Probe safe range that we can use for unassigned bridge.

Try to allocate from parent bus busn_res. if can not find any big enough, will try
to extend parent bus top. even the extending is through allocating, after allocating
will pad the range to parent buses top.

When extending happens, We will record the parent_res, so could use it as stopper
for really extend/shrink top later.

Signed-off-by: Yinghai Lu <yinghai <at> kernel.org>
---
 drivers/pci/probe.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 110 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 96259f8..c540022 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
 <at>  <at>  -691,6 +691,116  <at>  <at>  static void __devinit pci_bus_shrink_top(struct pci_bus *parent,
 	pci_bus_update_top(parent, -size, parent_res);
 }

+static resource_size_t __devinit find_res_top_free_size(struct resource *res)
+{
+	resource_size_t n_size;
+	struct resource tmp_res;
+
+	/*
+	 *   find out number below res->end, that we can use at first
+	 *	res->start can not be used.
+	 */
+	n_size = resource_size(res) - 1;
(Continue reading)


Gmane