From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_RED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 640BCC433DB for ; Tue, 23 Mar 2021 11:11:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EB579619BF for ; Tue, 23 Mar 2021 11:11:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EB579619BF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 818C68D0006; Tue, 23 Mar 2021 07:11:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7C85B6B015A; Tue, 23 Mar 2021 07:11:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 643288D0006; Tue, 23 Mar 2021 07:11:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0233.hostedemail.com [216.40.44.233]) by kanga.kvack.org (Postfix) with ESMTP id 472596B0158 for ; Tue, 23 Mar 2021 07:11:19 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id F24F4180AD830 for ; Tue, 23 Mar 2021 11:11:18 +0000 (UTC) X-FDA: 77950872636.24.8373E89 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by imf12.hostedemail.com (Postfix) with ESMTP id 9C5E83D5 for ; Tue, 23 Mar 2021 11:11:17 +0000 (UTC) IronPort-SDR: w+deB8c2UH52q0bVX8/mulQR9SeIhrwtTkJbKpA//YR1j3BlXfiSAxn14+UNPL6GTIp+BkNBTK za1XCH0MkcUg== X-IronPort-AV: E=McAfee;i="6000,8403,9931"; a="177579299" X-IronPort-AV: E=Sophos;i="5.81,271,1610438400"; d="scan'208";a="177579299" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Mar 2021 04:11:15 -0700 IronPort-SDR: 64mApGEIQa4io6AeSQL7N9FDzK2pxtOcSUqA/mPu2Y8sojK3JidjZnvPI8H3aOWneM66tt8KiQ cYhmhCVNRbiA== X-IronPort-AV: E=Sophos;i="5.81,271,1610438400"; d="scan'208";a="604269151" Received: from smile.fi.intel.com (HELO smile) ([10.237.68.40]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Mar 2021 04:11:08 -0700 Received: from andy by smile with local (Exim 4.94) (envelope-from ) id 1lOewC-00Ezlc-F7; Tue, 23 Mar 2021 13:11:04 +0200 Date: Tue, 23 Mar 2021 13:11:04 +0200 From: Andy Shevchenko To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Greg Kroah-Hartman , Dan Williams , Daniel Vetter , Mauro Carvalho Chehab , Dave Young , Baoquan He , Vivek Goyal , Dave Hansen , Keith Busch , Michal Hocko , Qian Cai , Oscar Salvador , Eric Biederman , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Tom Lendacky , Brijesh Singh , x86@kernel.org, kexec@lists.infradead.org Subject: Re: [PATCH v1 3/3] kernel/resource: remove first_lvl / siblings_only logic Message-ID: References: <20210322160200.19633-1-david@redhat.com> <20210322160200.19633-4-david@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210322160200.19633-4-david@redhat.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-Stat-Signature: iqkeseg6m7pw4bh41nzf45bzremoqk1m X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 9C5E83D5 Received-SPF: none (linux.intel.com>: No applicable sender policy available) receiver=imf12; identity=mailfrom; envelope-from=""; helo=mga02.intel.com; client-ip=134.134.136.20 X-HE-DKIM-Result: none/none X-HE-Tag: 1616497877-697314 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Mar 22, 2021 at 05:02:00PM +0100, David Hildenbrand wrote: > All IORESOURCE_SYSTEM_RAM and IORESOURCE_MEM now properly consider the > whole resource tree, not just the first level. Let's drop the unused > first_lvl / siblings_only logic. > > All functions properly search the whole tree, so remove documentation > that indicates that some functions behave differently. Like this clean up! Reviewed-by: Andy Shevchenko Although a few nit-picks below. > Cc: Andrew Morton > Cc: Greg Kroah-Hartman > Cc: Dan Williams > Cc: Daniel Vetter > Cc: Andy Shevchenko > Cc: Mauro Carvalho Chehab > Cc: Signed-off-by: David Hildenbrand > Cc: Dave Young > Cc: Baoquan He > Cc: Vivek Goyal > Cc: Dave Hansen > Cc: Keith Busch > Cc: Michal Hocko > Cc: Qian Cai > Cc: Oscar Salvador > Cc: Eric Biederman > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: "H. Peter Anvin" > Cc: Tom Lendacky > Cc: Brijesh Singh > Cc: x86@kernel.org > Cc: kexec@lists.infradead.org > Signed-off-by: David Hildenbrand > --- > kernel/resource.c | 45 ++++++++++++--------------------------------- > 1 file changed, 12 insertions(+), 33 deletions(-) > > diff --git a/kernel/resource.c b/kernel/resource.c > index 16e0c7e8ed24..7e00239a023a 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -64,12 +64,8 @@ static DEFINE_RWLOCK(resource_lock); > static struct resource *bootmem_resource_free; > static DEFINE_SPINLOCK(bootmem_resource_lock); > > -static struct resource *next_resource(struct resource *p, bool sibling_only) > +static struct resource *next_resource(struct resource *p) > { > - /* Caller wants to traverse through siblings only */ > - if (sibling_only) > - return p->sibling; > - > if (p->child) > return p->child; > while (!p->sibling && p->parent) > @@ -81,7 +77,7 @@ static void *r_next(struct seq_file *m, void *v, loff_t *pos) > { > struct resource *p = v; > (*pos)++; > - return (void *)next_resource(p, false); > + return (void *)next_resource(p); > } > > #ifdef CONFIG_PROC_FS > @@ -330,14 +326,10 @@ EXPORT_SYMBOL(release_resource); > * of the resource that's within [@start..@end]; if none is found, returns > * -ENODEV. Returns -EINVAL for invalid parameters. > * > - * This function walks the whole tree and not just first level children > - * unless @first_lvl is true. > - * > * @start: start address of the resource searched for > * @end: end address of same resource > * @flags: flags which the resource must have > * @desc: descriptor the resource must have > - * @first_lvl: walk only the first level children, if set > * @res: return ptr, if resource found > * > * The caller must specify @start, @end, @flags, and @desc > @@ -345,9 +337,8 @@ EXPORT_SYMBOL(release_resource); > */ > static int find_next_iomem_res(resource_size_t start, resource_size_t end, > unsigned long flags, unsigned long desc, > - bool first_lvl, struct resource *res) > + struct resource *res) > { > - bool siblings_only = true; > struct resource *p; > > if (!res) > @@ -358,7 +349,7 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end, > > read_lock(&resource_lock); > > - for (p = iomem_resource.child; p; p = next_resource(p, siblings_only)) { > + for (p = iomem_resource.child; p; p = next_resource(p)) { > /* If we passed the resource we are looking for, stop */ > if (p->start > end) { > p = NULL; > @@ -369,13 +360,6 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end, > if (p->end < start) > continue; > > - /* > - * Now that we found a range that matches what we look for, > - * check the flags and the descriptor. If we were not asked to > - * use only the first level, start looking at children as well. > - */ > - siblings_only = first_lvl; > - > if ((p->flags & flags) != flags) > continue; > if ((desc != IORES_DESC_NONE) && (desc != p->desc)) > @@ -402,14 +386,14 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end, > > static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end, > unsigned long flags, unsigned long desc, > - bool first_lvl, void *arg, > + void *arg, > int (*func)(struct resource *, void *)) Can it be one line? > { > struct resource res; > int ret = -EINVAL; > > while (start < end && > - !find_next_iomem_res(start, end, flags, desc, first_lvl, &res)) { > + !find_next_iomem_res(start, end, flags, desc, &res)) { > ret = (*func)(&res, arg); > if (ret) > break; > @@ -431,7 +415,6 @@ static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end, > * @arg: function argument for the callback @func > * @func: callback function that is called for each qualifying resource area > * > - * This walks through whole tree and not just first level children. > * All the memory ranges which overlap start,end and also match flags and > * desc are valid candidates. > * > @@ -441,7 +424,7 @@ static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end, > int walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, > u64 end, void *arg, int (*func)(struct resource *, void *)) > { > - return __walk_iomem_res_desc(start, end, flags, desc, false, arg, func); > + return __walk_iomem_res_desc(start, end, flags, desc, arg, func); > } > EXPORT_SYMBOL_GPL(walk_iomem_res_desc); > > @@ -457,8 +440,8 @@ int walk_system_ram_res(u64 start, u64 end, void *arg, > { > unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > > - return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, false, > - arg, func); > + return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, arg, > + func); I guess you may do it on one line. > } > > /* > @@ -470,17 +453,14 @@ int walk_mem_res(u64 start, u64 end, void *arg, > { > unsigned long flags = IORESOURCE_MEM | IORESOURCE_BUSY; > > - return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, false, > - arg, func); > + return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, arg, > + func); Ditto. > } > > /* > * This function calls the @func callback against all memory ranges of type > * System RAM which are marked as IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY. > * It is to be used only for System RAM. > - * > - * This will find System RAM ranges that are children of top-level resources > - * in addition to top-level System RAM resources. > */ > int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, > void *arg, int (*func)(unsigned long, unsigned long, void *)) > @@ -495,8 +475,7 @@ int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, > end = ((u64)(start_pfn + nr_pages) << PAGE_SHIFT) - 1; > flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > while (start < end && > - !find_next_iomem_res(start, end, flags, IORES_DESC_NONE, > - false, &res)) { > + !find_next_iomem_res(start, end, flags, IORES_DESC_NONE, &res)) { > pfn = PFN_UP(res.start); > end_pfn = PFN_DOWN(res.end + 1); > if (end_pfn > pfn) > -- > 2.29.2 > -- With Best Regards, Andy Shevchenko