From patchwork Fri May 25 19:37:16 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 161403 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 C27D3B6EE7 for ; Sat, 26 May 2012 05:37:22 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756553Ab2EYThV (ORCPT ); Fri, 25 May 2012 15:37:21 -0400 Received: from mail-we0-f202.google.com ([74.125.82.202]:37024 "EHLO mail-we0-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751460Ab2EYThT (ORCPT ); Fri, 25 May 2012 15:37:19 -0400 X-Greylist: delayed 90676 seconds by postgrey-1.27 at vger.kernel.org; Fri, 25 May 2012 15:37:19 EDT Received: by were53 with SMTP id e53so68669wer.1 for ; Fri, 25 May 2012 12:37:18 -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=gieo62jvDyMPqX+eB/lT88cFoz1l8vPuwRapb30kisg=; b=Rpepi2pAOl+ngsjbytDUF59TIz/5NjO/AtV0+TAoQ9AKkl5E5mby25yu8ixrPTY3Ps pMhcU04Yd9svgwwETX++Q7m5UC/roPBdbdC0jRZo523eUSz+Wqum22SUnwP/s9d5kIAd VANFqlvyLRZooiD6l0DaBrF6yDhfrhJ2d6y9Zxr3/JKLXP8ec5Kh/eZ8pO3mOF+kDaTi ioEEFcQ9ZE4pxfOJuqytS9QlC3gdCYbNxeoyhXBn0K0s0E072LjDXH3TE1hVFhPq+rnR MYSKTowZTlrePd4yYIcXgm5aHufutT1mSBdiR+GTRcg+FcqujdKW+RJeswr9kBKQeakn hiXw== 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=gieo62jvDyMPqX+eB/lT88cFoz1l8vPuwRapb30kisg=; b=B4DPLMHkP5qBwa7E1DEzB/S7Ki+ordlfOpb21VtzGNP+q39Qubzhu1uA05uwDvtNKg 4oxGdK5vHxKsYCB2dPChGHQ+T4nAVyt1ypC9ysP2q/7wE/A4KUkY3Vv3b4KJgD4MyGqc RG/N89aSpr8JWQEea69lhldNwg5Xu+uX8kULFfh0yLCDFOME11+frmLSwepMxp+BGQHk wBCthYidSvHQTAmy1JTjmuAQVULxuxKfQRRIdTkcI6NpjsoSj5xYeQL4NLhY9f7IlWeC n2QJGis2Z6E5SRGFvVrTHqizj0Tw/4idWSdZZHAeGCb5RevrGk09RYq7IxkefzbSUurR auig== Received: by 10.14.186.3 with SMTP id v3mr67384eem.5.1337974638228; Fri, 25 May 2012 12:37:18 -0700 (PDT) Received: by 10.14.186.3 with SMTP id v3mr67370eem.5.1337974638110; Fri, 25 May 2012 12:37:18 -0700 (PDT) Received: from hpza10.eem.corp.google.com ([74.125.121.33]) by gmr-mx.google.com with ESMTPS id b15si2903451een.0.2012.05.25.12.37.18 (version=TLSv1/SSLv3 cipher=AES128-SHA); Fri, 25 May 2012 12:37:18 -0700 (PDT) Received: from bhelgaas.mtv.corp.google.com (bhelgaas.mtv.corp.google.com [172.18.96.155]) by hpza10.eem.corp.google.com (Postfix) with ESMTP id AD92A20004E; Fri, 25 May 2012 12:37:17 -0700 (PDT) Received: by bhelgaas.mtv.corp.google.com (Postfix, from userid 131485) id F1C30180187; Fri, 25 May 2012 12:37:16 -0700 (PDT) Date: Fri, 25 May 2012 13:37:16 -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: <20120525193716.GA8817@google.com> References: <1337754877-19759-1-git-send-email-yinghai@kernel.org> <1337754877-19759-3-git-send-email-yinghai@kernel.org> <20120525043651.GA1391@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-Gm-Message-State: ALoCoQnUOwuNoZx4VsmTJ4i2Atw41lw3zc7DgLGjS6/+32cdFdNrFdyIXcVvlOhmXeDLQAClxyMAi6ZtPkFs/8qvHD22K4l4Y4nxXV29jUE3r1Hdgsvs/FGeI3IF4FKCedoMobZAE6yTVdiRBaPAJBdr/tjlD+EtzQ== Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Fri, May 25, 2012 at 11:39:26AM -0700, Yinghai Lu wrote: > On Fri, May 25, 2012 at 10:53 AM, Yinghai Lu wrote: > >> 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. > >> > > please check if attached one is more clear. > > make max and bottom is only related to _MEM and not default one. > > - if (!(res->flags & IORESOURCE_MEM_64)) > - max = PCIBIOS_MAX_MEM_32; > + if (res->flags & IORESOURCE_MEM) { > + if (!(res->flags & IORESOURCE_MEM_64)) > + max = PCIBIOS_MAX_MEM_32; > + else if (PCIBIOS_MAX_MEM_32 != -1) > + bottom = (resource_size_t)(1ULL<<32); > + } > > will still not affect to other arches. That's goofy. You're proposing to make only x86_64 and x86-PAE try to put 64-bit BARs above 4GB. Why should this be specific to x86? I acknowledge that there's risk in doing this, but if it's a good idea for x86_64, it should also be a good idea for other 64-bit architectures. And testing for "is this x86_32 without PAE?" with "PCIBIOS_MAX_MEM_32 == -1" is just plain obtuse and hides an important bit of arch-specific behavior. Tangential question about allocate_resource(): Is its "max" argument really necessary? We'll obviously only allocate from inside the root resource, so "max" is just a way to artificially avoid the end of that resource. Is there really a case where that's required? "min" makes sense because in a case like this, it's valid to allocate from anywhere in the root resource, but we want to try to allocate from the >4GB part first, then fall back to allocating from the whole resource. I'm not sure there's a corresponding case for "max." Getting back to this patch, I don't think we should need to adjust "max" at all. For example, this: commit cb1c8e46244cfd84a1a2fe91be860a74c1cf4e25 Author: Bjorn Helgaas Date: Thu May 24 22:15:26 2012 -0600 PCI: try to allocate 64-bit mem resources above 4GB If we have a 64-bit mem resource, try to allocate it above 4GB first. If that fails, 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..075e5b1 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -121,14 +121,16 @@ 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 = MAX_RESOURCE; 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 mem resource, try above 4GB first */ + if (res->flags & IORESOURCE_MEM_64) + start = (resource_size_t) (1ULL << 32); +again: pci_bus_for_each_resource(bus, r, i) { if (!r) continue; @@ -145,12 +147,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; }