From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lb0-f174.google.com ([209.85.217.174]:46353 "EHLO mail-lb0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753021Ab2HaAlK (ORCPT ); Thu, 30 Aug 2012 20:41:10 -0400 Received: by lbbgj3 with SMTP id gj3so897978lbb.19 for ; Thu, 30 Aug 2012 17:41:08 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1340736849-14875-1-git-send-email-yinghai@kernel.org> <1340736849-14875-3-git-send-email-yinghai@kernel.org> From: Bjorn Helgaas Date: Thu, 30 Aug 2012 17:40:48 -0700 Message-ID: Subject: Re: [PATCH -v12 02/15] resources: Add probe_resource() To: Yinghai Lu Cc: Linus Torvalds , Benjamin Herrenschmidt , Tony Luck , David Miller , x86 , Dominik Brodowski , Andrew Morton , Greg Kroah-Hartman , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-pci-owner@vger.kernel.org List-ID: On Wed, Aug 29, 2012 at 10:36 AM, Yinghai Lu wrote: > On Wed, Aug 29, 2012 at 8:57 AM, Yinghai Lu wrote: >> also have another version for probe_resource, please check attached version -v8. >> > > sorry, v8 forget removing two lines. > > please -v9 instead. > > -v8: Linus said: allocation/return is not right, and -1 step tricks make it > not work as generic resource probe. > So try to remove the needed_size tricks, and also use __adjust_resource > for probing instead. > -v9: remove two lines that is supposed to be removed after converting to use > __adjust_resource These tweaks might be slight improvements, but they completely miss the point of my objection. I just don't think the probe_resource() interface is a reasonable addition to kernel/resource.c. I think it's too hard to describe what it does, and it seems like it's too specific to what PCI needs in this particular case. We should be able to look at the prototype and get a pretty good idea of what the function does, but I can't do that with this: +int probe_resource(struct resource *b_res, + struct resource *busn_res, + resource_size_t needed_size, struct resource **p, + int skip_nr, int limit, int stop_flags) We already have adjust_resource(), which grows or shrinks a resource while maintaining the invariants that the adjusted resource (1) doesn't overlap any of its siblings and (2) still contains all its children. adjust_resource() seems like a fairly generic, generally useful interface. What you're trying to do with probe_resource() is quite similar, except that probe_resource() adds the idea of walking up the tree. I think you should consider something like an "expand_resource()" that just balloons a resource at both ends until it abuts its siblings, i.e., it grows the resource as much as possible. Then you know the largest possible size, and you can use adjust_resource() to shrink it again if you don't need that much. You can walk up the tree in the caller when you need to. Bjorn