From patchwork Fri May 25 04:36:51 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 161243 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id AB7C7B6F13 for ; Fri, 25 May 2012 14:36:55 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751038Ab2EYEgy (ORCPT ); Fri, 25 May 2012 00:36:54 -0400 Received: from mail-gg0-f202.google.com ([209.85.161.202]:62301 "EHLO mail-gg0-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750850Ab2EYEgx (ORCPT ); Fri, 25 May 2012 00:36:53 -0400 Received: by ggeh3 with SMTP id h3so76703gge.1 for ; Thu, 24 May 2012 21:36:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=Zbp8kZHeTWaLGWjb56UOiGkM7LHCXNjsWIeLORHBn7M=; b=FmDJskef1Nm3LXHT3zhO0y9e6r3ds3EvaHiRKa3hlKDwO2ZCD89oWc9x48L2yrqg+z 8I4Lxwi9opWz82eeSyD3xHleXaDDpj8rc2pXz7jkU1Nrjsv5Ha4YrYzgK5g81bsCzkTI c243NYWlFKYYELQ5i1XBYhuSnuHaPfG4mwVfava+Ur2ey4xJHJVpWIQVkYBolZnOxZU5 b9MTMXrN/ebLbzQTCneCqiUA5rMRHHEDGej2SAk1wNAORO0DcNDsx2KN3VXd3QWpOfCd F4F7RrkwWThNlrm1h/IYXJSlgytbOHgQkB5OzfomyJuShYYnJiqEl7taiigiMIgctdWI XvoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent :x-gm-message-state; bh=Zbp8kZHeTWaLGWjb56UOiGkM7LHCXNjsWIeLORHBn7M=; b=RGPD7esvs/rHQykLevyW5rsyuaLCve64dtrA5XORoDvsNv3hBnGjS/GRB0WhTfguBz joKrgMBYqxtMB7KDonlXjcVxgG8pYDqZyc45c1HLLJ1ce+Iq6ts6pTemJ0nZZog+xlyO Gk3dt1NnPtL+iUkEzwFDGxXsLQkWRKAofxpC3vIfptYEldAfRZBbqcAblcucZ7rx7q16 hC3lbomhcXfMJYP1L1kMT/kRW4i/5RXjgluw4RaVw+PFtJXVsz7q/aDWdqU79mvKbjQm qGPKFF2ncBjAnaxYA47hYPLMsMQQth3tjFJbFzZ3Ixyj0Bh9Mnj6OzN6KD3328tn7yXD m0IQ== Received: by 10.101.2.30 with SMTP id e30mr1088245ani.27.1337920612701; Thu, 24 May 2012 21:36:52 -0700 (PDT) Received: by 10.101.2.30 with SMTP id e30mr1088231ani.27.1337920612609; Thu, 24 May 2012 21:36:52 -0700 (PDT) Received: from wpzn4.hot.corp.google.com (216-239-44-65.google.com [216.239.44.65]) by gmr-mx.google.com with ESMTPS id f27si1029813anj.1.2012.05.24.21.36.52 (version=TLSv1/SSLv3 cipher=AES128-SHA); Thu, 24 May 2012 21:36:52 -0700 (PDT) Received: from bhelgaas.mtv.corp.google.com (bhelgaas.mtv.corp.google.com [172.18.96.155]) by wpzn4.hot.corp.google.com (Postfix) with ESMTP id 67F4C1E004D; Thu, 24 May 2012 21:36:52 -0700 (PDT) Received: by bhelgaas.mtv.corp.google.com (Postfix, from userid 131485) id 10B7E1801B7; Thu, 24 May 2012 21:36:51 -0700 (PDT) Date: Thu, 24 May 2012 22:36:51 -0600 From: Bjorn Helgaas To: Yinghai Lu Cc: Linus Torvalds , Steven Newbury , "H. Peter Anvin" , Andrew Morton , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 02/11] PCI: Try to allocate mem64 above 4G at first Message-ID: <20120525043651.GA1391@google.com> References: <1337754877-19759-1-git-send-email-yinghai@kernel.org> <1337754877-19759-3-git-send-email-yinghai@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-Gm-Message-State: ALoCoQkhk2gAqNDl4hJIj+zicC+p/G8TrURTpzM7D12DNKP0uJE/MulcV0rGpcVsCmMSc9k5gy2wYX/ixWRBe3IhfRuRXwODGwy8PnqJ7HbGObfOUDdWNGYyl4vwt8o3PVT5f3pGIpVq6pEinkFSZEuP2nAdOS9DMg== Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Wed, May 23, 2012 at 11:40:46AM -0700, Yinghai Lu wrote: > On Wed, May 23, 2012 at 10:30 AM, Yinghai Lu wrote: > > On Wed, May 23, 2012 at 8:57 AM, Linus Torvalds > > wrote: > >> On Tue, May 22, 2012 at 11:34 PM, Yinghai Lu wrote: > >>> and will fall back to below 4g if it can not find any above 4g. > >> > >> Has this been tested on 32-bit machines without PAE? There might be > >> things that just happen to work because their allocations were always > >> done bottom-up. > > > > Good point. that problem should be addressed at first before this patch. > > Just checked code for 32bit machines without PAE. > > when X86_PAE is not set, phys_addr_t aka resource_size_t will be 32bit. > so in drivers/pci/bus.c::pci_bus_alloc_resource_fit() > will have bottom to 0. > resource_size_t bottom = PCIBIOS_MAX_MEM_32 + 1ULL; > also in arch/x86/kernel/setup.c::setup_arch() > iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1; > will have iomem_resource.end to 0xffffffff > > when X86_PAE is set, but CPU does not support PAE. > phys_addr_t aka resource_size_t will be 32bit. I think you meant phys_addr_t and resource_size_t will be *64* bit when X86_PAE is set. Obvious to you, but quite confusing to non-x86 experts like me :) > so in drivers/pci/bus.c::pci_bus_alloc_resource_fit() > will have bottom to 4g. > resource_size_t bottom = PCIBIOS_MAX_MEM_32 + 1ULL; > but > in arch/x86/kernel/setup.c::setup_arch() > iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1; > will have iomem_resource.end to 0xffffffff, because x86_phys_bits is 32 when PAE > is not detected in arch/x86/kernel/cpu/common.c::get_cpu_cap. > that mean first try will fail, so it will go to second try with bottom to 0. > > so both case are safe with this patch. I don't really like the dependency on PCIBIOS_MAX_MEM_32 + 1ULL overflowing to zero -- that means the reader has to know what the value of PCIBIOS_MAX_MEM_32 is, and things would break in non-obvious ways if we changed it. What do you think of a patch like the following? It makes it explicit that we can only allocate space the CPU can address. commit feded2ae21d6160292726ccd5128080d42395be4 Author: Bjorn Helgaas Date: Thu May 24 22:15:26 2012 -0600 PCI: try to allocate 64-bit resources above 4GB If we have a 64-bit resource, try to allocate it above 4GB first. If that fails, either because there's no space or the CPU can't address space above 4GB (iomem_resource.end is the highest address the CPU supports), we'll fall back to allocating space below 4GB. --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 4ce5ef2..2c56693 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -121,14 +121,18 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res, { int i, ret = -ENOMEM; struct resource *r; - resource_size_t max = -1; + resource_size_t start = 0; + resource_size_t end = PCIBIOS_MAX_MEM_32; type_mask |= IORESOURCE_IO | IORESOURCE_MEM; - /* don't allocate too high if the pref mem doesn't support 64bit*/ - if (!(res->flags & IORESOURCE_MEM_64)) - max = PCIBIOS_MAX_MEM_32; + /* If this is a 64-bit resource, prefer space above 4GB */ + if (res->flags & IORESOURCE_MEM_64) { + start = PCIBIOS_MAX_MEM_32 + 1ULL; + end = iomem_resource.end; + } +again: pci_bus_for_each_resource(bus, r, i) { if (!r) continue; @@ -145,12 +149,18 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res, /* Ok, try it out.. */ ret = allocate_resource(r, res, size, - r->start ? : min, - max, align, + max(start, r->start ? : min), + end, align, alignf, alignf_data); if (ret == 0) - break; + return 0; } + + if (start != 0) { + start = 0; + goto again; + } + return ret; }