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=-15.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,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 62BE0C4361B for ; Thu, 10 Dec 2020 08:02:46 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D038D23D4D for ; Thu, 10 Dec 2020 08:02:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D038D23D4D 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 F35A96B0036; Thu, 10 Dec 2020 03:02:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id EE69B6B005D; Thu, 10 Dec 2020 03:02:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D62946B0068; Thu, 10 Dec 2020 03:02:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0038.hostedemail.com [216.40.44.38]) by kanga.kvack.org (Postfix) with ESMTP id C8D5D6B0036 for ; Thu, 10 Dec 2020 03:02:44 -0500 (EST) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 9B8513625 for ; Thu, 10 Dec 2020 08:02:44 +0000 (UTC) X-FDA: 77576631048.01.horn17_570cd86273f6 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin01.hostedemail.com (Postfix) with ESMTP id 76DB11004D42F for ; Thu, 10 Dec 2020 08:02:44 +0000 (UTC) X-HE-Tag: horn17_570cd86273f6 X-Filterd-Recvd-Size: 7212 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by imf46.hostedemail.com (Postfix) with ESMTP for ; Thu, 10 Dec 2020 08:02:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607587363; 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=TQkSn2F7evUhUtd3xKUtaGHva5aW3Ib9zLjmDxpPHBI=; b=H/bxAkZiIA1/U4dfrTjAnFUxThYvQrXlJaSjwEVeabO4jyu4QJ+NXZGWRu3KxAsEdmpGkW 0oJAB5eGN77pxH1b0QTzuegqWbRYS4MwyeO2y/KP4ix7ietVpWCuRPBvLEa7wf5mTYS1W2 O1YgYjJgGAoft7sv6h7uqgYcWgKHE/c= 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-500-_X5kaKvFOHO1YMeC7QRTTg-1; Thu, 10 Dec 2020 03:02:39 -0500 X-MC-Unique: _X5kaKvFOHO1YMeC7QRTTg-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 82A268042CF; Thu, 10 Dec 2020 08:02:24 +0000 (UTC) Received: from [10.36.113.177] (ovpn-113-177.ams2.redhat.com [10.36.113.177]) by smtp.corp.redhat.com (Postfix) with ESMTP id A76B86F96F; Thu, 10 Dec 2020 08:02:21 +0000 (UTC) Subject: Re: [PATCH 3/3] s390/mm: Define arch_get_mappable_range() To: Anshuman Khandual , Heiko Carstens Cc: linux-mm@kvack.org, akpm@linux-foundation.org, catalin.marinas@arm.com, linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, Vasily Gorbik , Will Deacon , Ard Biesheuvel , Mark Rutland References: <20201210065845.GA20691@osiris> <0a2f6eb1-c38b-9cc2-5c45-16f6c8999ce2@arm.com> From: David Hildenbrand Organization: Red Hat GmbH Message-ID: <2a379949-4ecb-e380-560e-78ef91168c87@redhat.com> Date: Thu, 10 Dec 2020 09:02:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: <0a2f6eb1-c38b-9cc2-5c45-16f6c8999ce2@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Content-Transfer-Encoding: quoted-printable 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 10.12.20 08:40, Anshuman Khandual wrote: >=20 >=20 > On 12/10/20 12:34 PM, David Hildenbrand wrote: >> >>> Am 10.12.2020 um 07:58 schrieb Heiko Carstens : >>> >>> =EF=BB=BFOn Thu, Dec 10, 2020 at 09:48:11AM +0530, Anshuman Khandual = wrote: >>>>>> Alternatively leaving __segment_load() and vmem_add_memory() uncha= nged >>>>>> will create three range checks i.e two memhp_range_allowed() and t= he >>>>>> existing VMEM_MAX_PHYS check in vmem_add_mapping() on all the hotp= lug >>>>>> paths, which is not optimal. >>>>> >>>>> Ah, sorry. I didn't follow this discussion too closely. I just thou= ght >>>>> my point of view would be clear: let's not have two different ways = to >>>>> check for the same thing which must be kept in sync. >>>>> Therefore I was wondering why this next version is still doing >>>>> that. Please find a way to solve this. >>>> >>>> The following change is after the current series and should work wit= h >>>> and without memory hotplug enabled. There will be just a single plac= e >>>> i.e vmem_get_max_addr() to update in case the maximum address change= s >>>> from VMEM_MAX_PHYS to something else later. >>> >>> Still not. That's way too much code churn for what you want to achiev= e. >>> If the s390 specific patch would look like below you can add >>> >>> Acked-by: Heiko Carstens >>> >>> But please make sure that the arch_get_mappable_range() prototype in >>> linux/memory_hotplug.h is always visible and does not depend on >>> CONFIG_MEMORY_HOTPLUG. I'd like to avoid seeing sparse warnings >>> because of this. >>> >>> Thanks. >>> >>> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c >>> index 77767850d0d0..e0e78234ae57 100644 >>> --- a/arch/s390/mm/init.c >>> +++ b/arch/s390/mm/init.c >>> @@ -291,6 +291,7 @@ int arch_add_memory(int nid, u64 start, u64 size, >>> if (WARN_ON_ONCE(params->pgprot.pgprot !=3D PAGE_KERNEL.pgprot)) >>> return -EINVAL; >>> >>> + VM_BUG_ON(!memhp_range_allowed(start, size, 1)); >>> rc =3D vmem_add_mapping(start, size); >>> if (rc) >>> return rc; >>> diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c >>> index b239f2ba93b0..ccd55e2f97f9 100644 >>> --- a/arch/s390/mm/vmem.c >>> +++ b/arch/s390/mm/vmem.c >>> @@ -4,6 +4,7 @@ >>> * Author(s): Heiko Carstens >>> */ >>> >>> +#include >>> #include >>> #include >>> #include >>> @@ -532,11 +533,23 @@ void vmem_remove_mapping(unsigned long start, u= nsigned long size) >>> mutex_unlock(&vmem_mutex); >>> } >>> >>> +struct range arch_get_mappable_range(void) >>> +{ >>> + struct range range; >>> + >>> + range.start =3D 0; >>> + range.end =3D VMEM_MAX_PHYS; >>> + return range; >>> +} >>> + >>> int vmem_add_mapping(unsigned long start, unsigned long size) >>> { >>> + struct range range; >>> int ret; >>> >>> - if (start + size > VMEM_MAX_PHYS || >>> + range =3D arch_get_mappable_range(); >>> + if (start < range.start || >>> + start + size > range.end || >>> start + size < start) >>> return -ERANGE; >>> >>> >> >> Right, what I had in mind as reply to v1. Not sure if we really need n= ew checks in common code. Having a new memhp_get_pluggable_range() would = be sufficient for my use case (virtio-mem). > Didn't quite understand "Not sure if we really need new checks in commo= n code". > Could you please be more specific. New checks as in pagemap_range() ? B= ecause > other places it is either replacing erstwhile check_hotplug_memory_addr= essable() > or just moving existing checks from platform arch_add_memory() to the b= eginning > of various hotplug paths. The main concern I have with current code is that it makes it impossible for some driver to detect which ranges it could actually later hotplug. You cannot warn about a strange setup before you actually run into the issues while trying to add memory. Like returning "-EINVAL" from a function but not exposing which values are actually valid. If we have memhp_get_pluggable_range(), we have such a mechanism and 1. Trying to add out-of-range memory will fail (although deep down in arch code, but at least it fails). 2. There is a way for drivers to find out which values are actually valid before triggering 1. For my use case that's good enough. Do you have others in mind that require new checks in common code (meaning inside add_memory() and friend= s)? --=20 Thanks, David / dhildenb