All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bhelgaas@google.com>
To: Yinghai Lu <yinghai@kernel.org>
Cc: Wei Yang <weiyang@linux.vnet.ibm.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Ram Pai <linuxram@us.ibm.com>,
	Gavin Shan <shangw@linux.vnet.ibm.com>
Subject: Re: [PATCH 4/4] PCI: fix the io resource alignment calculation in pbus_size_io()
Date: Mon, 5 Aug 2013 16:21:35 -0600	[thread overview]
Message-ID: <20130805222135.GA29875@google.com> (raw)
In-Reply-To: <CAE9FiQVJZWFBvOJPna_CFo=Pt2hGXmx39Q278f+0BOwV_38oRQ@mail.gmail.com>

On Mon, Aug 05, 2013 at 02:09:27PM -0700, Yinghai Lu wrote:
> On Mon, Aug 5, 2013 at 1:59 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> then, we should drop that 4k capping.
> >> I was thinking there could be strange or wild res with bigger than 4k.
> >
> > If there *were* an I/O BAR larger than 4KB, how should it be handled?
> > I don't think capping the alignment to 4KB sounds like the best way.
> > For example, a 16KB I/O BAR would still need to be aligned on 16KB.
> >
> > And I think capping to 4KB as you did above will break the powerpc
> > pcibios_window_alignment() implementation.  For example, if
> > pcibios_window_alignment() returned 16KB, and we later capped it to
> > 4KB, we're going to allocate space for the bridge window with the
> > wrong alignment.
> 
> Agree.

OK.  Can you guys try this out and see whether it fixes the problem?
I don't know what the actual problem *is*, so I can't tell whether
this is a possible fix.

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index d4f1ad9..8333c92 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -749,12 +749,12 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 	struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
 	resource_size_t size = 0, size0 = 0, size1 = 0;
 	resource_size_t children_add_size = 0;
-	resource_size_t min_align, io_align, align;
+	resource_size_t min_align, align;
 
 	if (!b_res)
  		return;
 
-	io_align = min_align = window_alignment(bus, IORESOURCE_IO);
+	min_align = window_alignment(bus, IORESOURCE_IO);
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		int i;
 
@@ -781,9 +781,6 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 		}
 	}
 
-	if (min_align > io_align)
-		min_align = io_align;
-
 	size0 = calculate_iosize(size, min_size, size1,
 			resource_size(b_res), min_align);
 	if (children_add_size > add_size)

  reply	other threads:[~2013-08-05 22:21 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-02  9:31 [PATCH 0/4] optimization/fix/cleanup in pci_assign_unassigned_resources Wei Yang
2013-08-02  9:31 ` [PATCH 1/4] PCI: optimize pci_bus_get_depth() by enumerating on pci bus hierachy Wei Yang
2013-08-02  9:31 ` [PATCH 2/4] PCI: add comment for pbus_size_mem() parameter Wei Yang
2013-08-02  9:31 ` [PATCH 3/4] PCI: trivial cleanup in pbus_size_io() Wei Yang
2013-08-02  9:31 ` [PATCH 4/4] PCI: fix the io resource alignment calculation " Wei Yang
2013-08-02 22:51   ` Bjorn Helgaas
2013-08-05 17:58   ` Bjorn Helgaas
2013-08-05 19:05     ` Yinghai Lu
2013-08-05 19:51       ` Bjorn Helgaas
2013-08-05 20:52         ` Yinghai Lu
2013-08-05 20:59           ` Bjorn Helgaas
2013-08-05 21:09             ` Yinghai Lu
2013-08-05 22:21               ` Bjorn Helgaas [this message]
2013-08-06  6:15                 ` Wei Yang
2013-08-06 13:39                   ` Bjorn Helgaas
2013-08-06 15:34                     ` Wei Yang
2013-08-06 17:58                       ` Bjorn Helgaas
2013-08-07  2:01                         ` Wei Yang
     [not found]                 ` <20130806032227.GA7736@shangw.(null)>
2013-08-06  6:26                   ` Wei Yang
2013-08-06 13:42                     ` Bjorn Helgaas
     [not found]                 ` <52006bfc.6a5d3c0a.2753.ffffa6b7SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-06 13:35                   ` Bjorn Helgaas
2013-08-06  6:19     ` Wei Yang
2013-08-06 13:44       ` Bjorn Helgaas
2013-08-06 15:47         ` Wei Yang
2013-08-06 18:01           ` Bjorn Helgaas
2013-08-06 20:56             ` Bjorn Helgaas
2013-08-07  2:01               ` Wei Yang
  -- strict thread matches above, loose matches on Subject: below --
2013-07-01 15:10 [PATCH 0/4] optimization/fix/cleanup in pci_assign_unassigned_resources Wei Yang
2013-07-01 15:10 ` [PATCH 4/4] PCI: fix the io resource alignment calculation in pbus_size_io() Wei Yang
2013-07-08 21:15   ` Bjorn Helgaas
2013-07-09  3:20     ` Wei Yang
2013-07-09 17:38       ` Bjorn Helgaas
2013-07-10  1:34         ` Wei Yang
2013-07-19  3:10           ` Wei Yang
2013-07-25 21:22           ` Bjorn Helgaas

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=20130805222135.GA29875@google.com \
    --to=bhelgaas@google.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxram@us.ibm.com \
    --cc=shangw@linux.vnet.ibm.com \
    --cc=weiyang@linux.vnet.ibm.com \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.