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=-17.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_RED,USER_AGENT_SANE_1 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 97435C433C1 for ; Tue, 23 Mar 2021 11:19:28 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 36B71619B9 for ; Tue, 23 Mar 2021 11:19:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 36B71619B9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id AF67D6B015A; Tue, 23 Mar 2021 07:19:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AA6368D0007; Tue, 23 Mar 2021 07:19:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 91FEE6B015D; Tue, 23 Mar 2021 07:19:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0146.hostedemail.com [216.40.44.146]) by kanga.kvack.org (Postfix) with ESMTP id 741AB6B015A for ; Tue, 23 Mar 2021 07:19:27 -0400 (EDT) Received: from smtpin38.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 2ADA481D6 for ; Tue, 23 Mar 2021 11:19:27 +0000 (UTC) X-FDA: 77950893174.38.73288E8 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by imf04.hostedemail.com (Postfix) with ESMTP id 265B63C5 for ; Tue, 23 Mar 2021 11:19:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1616498366; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rItqcrfiSSCIyGFXFE6rhbI0SFnZjCLml20Heqjbrig=; b=gSIbtGa68/tdOoVDeTUmztNa0gLJeSO9qyIGNFVbKdu1dsaPeuaAYAgxmVF3zxR2KmLnoc XbkPM68+Kh9hNeTLdT+iWc+LeqM6ESt0GZSe2eQR1Zwyqd/RykAKvg8loO+okoph5/tjwK TLn+nawYWypsp2UhuHfxHDXiBnzJb04= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-574-FWIp0ggzOJu8kJo4TYszdA-1; Tue, 23 Mar 2021 07:19:24 -0400 X-MC-Unique: FWIp0ggzOJu8kJo4TYszdA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 63EF05B361; Tue, 23 Mar 2021 11:19:21 +0000 (UTC) Received: from [10.36.115.54] (ovpn-115-54.ams2.redhat.com [10.36.115.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id DA4BE708E0; Tue, 23 Mar 2021 11:19:11 +0000 (UTC) Subject: Re: [PATCH v1 3/3] kernel/resource: remove first_lvl / siblings_only logic To: Andy Shevchenko 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 References: <20210322160200.19633-1-david@redhat.com> <20210322160200.19633-4-david@redhat.com> From: David Hildenbrand Organization: Red Hat GmbH Message-ID: Date: Tue, 23 Mar 2021 12:19:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 265B63C5 X-Stat-Signature: x3ym3ni1w9x1jc8s3yw3qthg39kqcw7n Received-SPF: none (redhat.com>: No applicable sender policy available) receiver=imf04; identity=mailfrom; envelope-from=""; helo=us-smtp-delivery-124.mimecast.com; client-ip=63.128.21.124 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616498365-664167 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 23.03.21 12:11, Andy Shevchenko wrote: > 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. To all your comments: "The preferred limit on the length of a single line is 80 columns." "Statements longer than 80 columns should be broken into sensible chunks ... unless exceeding 80 columns significantly increases readability" I don't think it significantly increases readability. -- Thanks, David / dhildenb