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=-7.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 60C5DC433B4 for ; Wed, 21 Apr 2021 08:52:15 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DCCB76142B for ; Wed, 21 Apr 2021 08:52:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DCCB76142B 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 4F35B6B006C; Wed, 21 Apr 2021 04:52:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4CA316B006E; Wed, 21 Apr 2021 04:52:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 36AA96B0070; Wed, 21 Apr 2021 04:52:14 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0036.hostedemail.com [216.40.44.36]) by kanga.kvack.org (Postfix) with ESMTP id 1BC576B006C for ; Wed, 21 Apr 2021 04:52:14 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id CBBA4824934B for ; Wed, 21 Apr 2021 08:52:13 +0000 (UTC) X-FDA: 78055757346.10.08174CD Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by imf08.hostedemail.com (Postfix) with ESMTP id ECFE880192EA for ; Wed, 21 Apr 2021 08:51:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1618995132; 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=ADh1GjnD2traYWKrmdkbx4sS/3yzMkTFPb5D6p+Cevw=; b=Pk3X4VDzCTEBJj56bUI6ITOERmYIGudikTjR8WO6EGjdZGYq1cj1bs06pOhSIoltvOdKxQ OZ5BpWDbVM9Q6fYRtU43bZpKPEm10BxCpu4ReSKXT3s8icJvKO0JNTqTougKS7KDalccMe VUBzMGJAZmxSbQveCkbIaIr4Vug8nKU= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-309-3QGDF0wNNN-yF0lPuzElLA-1; Wed, 21 Apr 2021 04:52:11 -0400 X-MC-Unique: 3QGDF0wNNN-yF0lPuzElLA-1 Received: by mail-ej1-f71.google.com with SMTP id re9-20020a170906d8c9b029037ca22d6744so5631505ejb.0 for ; Wed, 21 Apr 2021 01:52:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=ADh1GjnD2traYWKrmdkbx4sS/3yzMkTFPb5D6p+Cevw=; b=ilzRYqtTJ+rXPKyEbnOTz+XKTV9Az/8DqmuUAaVYxaQPE9AtqLqRoaIm1ePGdI6cFT Nbw+ebYTWjw+KsO3FlaC9db3IQUY59ye4hsI1Ia3iTSzdTrrMAXvSm+roRw2KXhvQnRy 5HNwNnu7aJ6gdQxOdvotDv3bJmtVixNwCAOeOCinJ5ikvumI5lhr2IqwLrk1z9A3SJoO Xha1kX1dPsPyD+M/+rKkFrgaTrViDLrRX/LkwHTunm4R1VC++WNy3E3MfujzLxtHITxL 714ptAa4I7xKKjXEAd47yvH117W75HA//SYRmtTz28cLFoNnoufpzmjzZybAEEVNX2b9 RfTw== X-Gm-Message-State: AOAM531OX2C1V/mTcNCN3PVFABA0Nl+ILfXggtDxIWr62v2E5Q/mCca9 Jb9hcEB1W6o3dTqeXczpPNuEOUT/MtZmqNvVgPSH8IF/+93Jryb06fxPG5Sevyxotfd7tW2wpEA wWRt8uCvpUEs= X-Received: by 2002:a17:906:b2cf:: with SMTP id cf15mr31447462ejb.233.1618995129864; Wed, 21 Apr 2021 01:52:09 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwPu5TB58UU6Pp+QMCJZiDVV54hEiR7K4MqSQTWUG2seDg6yT4lInHBRhqgDKp3s1svx9xIeA== X-Received: by 2002:a17:906:b2cf:: with SMTP id cf15mr31447436ejb.233.1618995129509; Wed, 21 Apr 2021 01:52:09 -0700 (PDT) Received: from [192.168.3.132] (p5b0c64b8.dip0.t-ipconnect.de. [91.12.100.184]) by smtp.gmail.com with ESMTPSA id gt33sm1614955ejc.89.2021.04.21.01.52.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 21 Apr 2021 01:52:09 -0700 (PDT) Subject: Re: [PATCH v9 4/8] mm,memory_hotplug: Allocate memmap from the added memory range To: Michal Hocko Cc: Oscar Salvador , Andrew Morton , Anshuman Khandual , Pavel Tatashin , Vlastimil Babka , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20210416112411.9826-1-osalvador@suse.de> <20210416112411.9826-5-osalvador@suse.de> <20210421081546.GD22456@linux> From: David Hildenbrand Organization: Red Hat Message-ID: <6044ab2c-e130-c21c-08ba-3132e9107843@redhat.com> Date: Wed, 21 Apr 2021 10:52:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: ECFE880192EA X-Stat-Signature: ynnpkbg6qu3qqm7i49euah4y9m48do55 Received-SPF: none (redhat.com>: No applicable sender policy available) receiver=imf08; identity=mailfrom; envelope-from=""; helo=us-smtp-delivery-124.mimecast.com; client-ip=216.205.24.124 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1618995113-193374 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 21.04.21 10:49, Michal Hocko wrote: > On Wed 21-04-21 10:44:38, David Hildenbrand wrote: >> On 21.04.21 10:39, Michal Hocko wrote: >>> 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) >>> >> >> We'll have to be careful in general when removing memory in different >> granularity than it was added, especially calling arch_remove_memory() in >> smaller granularity than it was added via arch_add_memory(). We might fail >> to tear down the direct map, imagine having mapped a 1GiB page but decide to >> remove individual 128 MiB chunks -- that won't work and the direct map would >> currently remain. > > Agreed but I am not referring to arbitrary hotremove path. All I am > pointing at is to split up to memory blocks and do the same kind of work > on each separately. Partial failures might turn out to be more tricky > and as I've said I do not insist on doing that right now but it is a bit > weird to outright fail the operation even when in fact there are more > blocks to be hot removed in once. Agreed. But we should also focus on what actual users need to see if it's worth the trouble (I know of none that will be using memmap_on_memory). -- Thanks, David / dhildenb