From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:35992 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726208AbgIOJQJ (ORCPT ); Tue, 15 Sep 2020 05:16:09 -0400 Subject: Re: [PATCH v2 1/7] kernel/resource: make release_mem_region_adjustable() never fail References: <20200908201012.44168-1-david@redhat.com> <20200908201012.44168-2-david@redhat.com> <20200915021012.GC2007@L-31X9LVDL-1304.local> <927904b1-1909-f11f-483e-8012bda8ad0c@redhat.com> <20200915090612.GA6936@L-31X9LVDL-1304.local> From: David Hildenbrand Message-ID: Date: Tue, 15 Sep 2020 11:15:53 +0200 MIME-Version: 1.0 In-Reply-To: <20200915090612.GA6936@L-31X9LVDL-1304.local> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-s390-owner@vger.kernel.org List-ID: To: Wei Yang Cc: linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-mm@kvack.org, linux-hyperv@vger.kernel.org, xen-devel@lists.xenproject.org, linux-acpi@vger.kernel.org, linux-nvdimm@lists.01.org, linux-s390@vger.kernel.org, Andrew Morton , Michal Hocko , Dan Williams , Jason Gunthorpe , Kees Cook , Ard Biesheuvel , Pankaj Gupta , Baoquan He On 15.09.20 11:06, Wei Yang wrote: > On Tue, Sep 15, 2020 at 09:35:30AM +0200, David Hildenbrand wrote: >> >>>> static int __ref try_remove_memory(int nid, u64 start, u64 size) >>>> { >>>> int rc = 0; >>>> @@ -1777,7 +1757,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) >>>> memblock_remove(start, size); >>>> } >>>> >>>> - __release_memory_resource(start, size); >>>> + release_mem_region_adjustable(&iomem_resource, start, size); >>>> >>> >>> Seems the only user of release_mem_region_adjustable() is here, can we move >>> iomem_resource into the function body? Actually, we don't iterate the resource >>> tree from any level. We always start from the root. >> >> You mean, making iomem_resource implicit? I can spot that something >> similar was done for >> >> #define devm_release_mem_region(dev, start, n) \ >> __devm_release_region(dev, &iomem_resource, (start), (n)) >> > > What I prefer is remove iomem_resource from the parameter list. Just use is in > the function body. > > For the example you listed, __release_region() would have varies of *parent*, > which looks reasonable to keep it here. Yeah I got that ("making iomem_resource implicit"), as I said: >> I'll send an addon patch for that, ok? - thanks. -- Thanks, David / dhildenb