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=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no 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 CEE9DC433B4 for ; Wed, 21 Apr 2021 08:39:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 52ADB60E0C for ; Wed, 21 Apr 2021 08:39:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 52ADB60E0C Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B6A036B0070; Wed, 21 Apr 2021 04:39:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF2A06B0071; Wed, 21 Apr 2021 04:39:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96CAA6B0072; Wed, 21 Apr 2021 04:39:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0205.hostedemail.com [216.40.44.205]) by kanga.kvack.org (Postfix) with ESMTP id 738BB6B0070 for ; Wed, 21 Apr 2021 04:39:19 -0400 (EDT) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 2CEF6181AEF32 for ; Wed, 21 Apr 2021 08:39:19 +0000 (UTC) X-FDA: 78055724838.26.6C568D3 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf12.hostedemail.com (Postfix) with ESMTP id 9B2B0DA for ; Wed, 21 Apr 2021 08:39:10 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1618994357; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=gw6HYqbnkEiPHn0aOFHVdg1JNHJNPqDuI/KUq2+VNGc=; b=acH8fpQDYTnpf/3diOHmqm81+fUNhW4cxcdnsjR03KiaAqk+RXeW3lYiG2XQtB5xnk1u5O bSYO/tpoIbenZC4+JbPRmJIfaeUXGeti/+QySMKcmUJfbjp8OImj4CcY27Ah+qOAhGKIj3 mKcRghZiQ6sk3phfFRPCYXS0TWtfbEs= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 65021B0B6; Wed, 21 Apr 2021 08:39:17 +0000 (UTC) Date: Wed, 21 Apr 2021 10:39:16 +0200 From: Michal Hocko To: Oscar Salvador Cc: Andrew Morton , David Hildenbrand , Anshuman Khandual , Pavel Tatashin , Vlastimil Babka , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v9 4/8] mm,memory_hotplug: Allocate memmap from the added memory range Message-ID: References: <20210416112411.9826-1-osalvador@suse.de> <20210416112411.9826-5-osalvador@suse.de> <20210421081546.GD22456@linux> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210421081546.GD22456@linux> X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 9B2B0DA X-Stat-Signature: 86y3dicf6qrgy5sqpc553hzrob78cspf Received-SPF: none (suse.com>: No applicable sender policy available) receiver=imf12; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1618994350-341012 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 Wed 21-04-21 10:15:46, Oscar Salvador wrote: > On Tue, Apr 20, 2021 at 12:56:03PM +0200, Michal Hocko wrote: [...] > > necessary. Using two different iteration styles is also hurting the code > > readability. I would go with the following > > for (pfn = start_pfn; pfn < end_pfn; ) { > > unsigned long order = min(MAX_ORDER - 1UL, __ffs(pfn)); > > > > while (start + (1UL << order) > end_pfn) > > order--; > > (*online_page_callback)(pfn_to_page(pfn), pageblock_order); > > pfn += 1 << order; > > } > > > > which is what __free_pages_memory does already. > > this is kinda what I used to have in the early versions, but it was agreed > with David to split it in two loops to make it explicit. > I can go back to that if it is preferred. Not that I would insist but I find it better to use common constructs when it doesn't hurt readability. The order evaluation can be even done in a trivial helper. > > > + if (memmap_on_memory) { > > > + nr_vmemmap_pages = walk_memory_blocks(start, size, NULL, > > > + get_nr_vmemmap_pages_cb); > > > + if (nr_vmemmap_pages) { > > > + if (size != memory_block_size_bytes()) { > > > + pr_warn("Refuse to remove %#llx - %#llx," > > > + "wrong granularity\n", > > > + start, start + size); > > > + return -EINVAL; > > > + } > > > + > > > + /* > > > + * Let remove_pmd_table->free_hugepage_table do the > > > + * right thing if we used vmem_altmap when hot-adding > > > + * the range. > > > + */ > > > + mhp_altmap.alloc = nr_vmemmap_pages; > > > + altmap = &mhp_altmap; > > > + } > > > + } > > > + > > > /* remove memmap entry */ > > > firmware_map_remove(start, start + size, "System RAM"); > > > > I have to say I still dislike this and I would just wrap it inside out > > and do the operation from within walk_memory_blocks but I will not > > insist. > > I have to confess I forgot about the details of that dicussion, as we were > quite focused on decoupling vmemmap pages from {online,offline} interface. > Would you mind elaborating a bit more? As I've said I will not insist and this can be done in the follow up. You are iterating over memory blocks just to refuse to do an operation which can be split to several memory blocks. See http://lkml.kernel.org/r/YFtPxH0CT5QZsnR1@dhcp22.suse.cz and follow walk_memory_blocks(start, size, NULL, remove_memory_block_cb) -- Michal Hocko SUSE Labs