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.6 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 E461DC433DF for ; Fri, 16 Oct 2020 09:11:36 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 490D221582 for ; Fri, 16 Oct 2020 09:11:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="aNB5vZr3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 490D221582 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 588D9940009; Fri, 16 Oct 2020 05:11:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 539C7900002; Fri, 16 Oct 2020 05:11:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 42838940009; Fri, 16 Oct 2020 05:11:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0229.hostedemail.com [216.40.44.229]) by kanga.kvack.org (Postfix) with ESMTP id 27A80900002 for ; Fri, 16 Oct 2020 05:11:35 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id D96C58249980 for ; Fri, 16 Oct 2020 09:11:34 +0000 (UTC) X-FDA: 77377220508.24.waste99_10138852721b Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin24.hostedemail.com (Postfix) with ESMTP id BD7931A4A5 for ; Fri, 16 Oct 2020 09:11:34 +0000 (UTC) X-HE-Tag: waste99_10138852721b X-Filterd-Recvd-Size: 7131 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by imf29.hostedemail.com (Postfix) with ESMTP for ; Fri, 16 Oct 2020 09:11:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1602839493; 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=N0iuzsKV+en+co/Ft7zltRtGpfmCivUKMQYMYF4dZIE=; b=aNB5vZr3C43v+Ylpr04BcuN+kwJHBJIhDoZl+qrC97AONeRrL6WMCWrowutQiNkJ4u3Dcp DU29kNCTJuInLuByN1/GPSQe+luONZKZ3v3WS4P+Psy0tH4MXsJP8uWBaK4URBbJkJDUh4 RFBQOzBZAUj1XgpgGMRn0hc2iGmB4wA= 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-154-K1OXnZx8NnSMeViY6FHIoQ-1; Fri, 16 Oct 2020 05:11:31 -0400 X-MC-Unique: K1OXnZx8NnSMeViY6FHIoQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5222F1020906; Fri, 16 Oct 2020 09:11:30 +0000 (UTC) Received: from [10.36.113.23] (ovpn-113-23.ams2.redhat.com [10.36.113.23]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7EFE41992D; Fri, 16 Oct 2020 09:11:25 +0000 (UTC) Subject: Re: [PATCH v1 05/29] virtio-mem: generalize check for added memory To: Wei Yang Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, virtualization@lists.linux-foundation.org, Andrew Morton , "Michael S . Tsirkin" , Jason Wang , Pankaj Gupta References: <20201012125323.17509-1-david@redhat.com> <20201012125323.17509-6-david@redhat.com> <20201015082808.GE86495@L-31X9LVDL-1304.local> <994394f3-c16d-911c-c9fc-d2280f32e7b1@redhat.com> <20201016021651.GI86495@L-31X9LVDL-1304.local> From: David Hildenbrand Organization: Red Hat GmbH Message-ID: <5caec772-295c-436a-2b19-ca261ea1ad0c@redhat.com> Date: Fri, 16 Oct 2020 11:11:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: <20201016021651.GI86495@L-31X9LVDL-1304.local> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 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: >> That's an interesting corner case. Assume you have a 128MB memory block >> but only 64MB are plugged. > > Since we just plug a part of memory block, this state is OFFLINE_PARTIAL > first. But then we would add these memory and online it. This means the state > of this memory block is ONLINE_PARTIAL. > > When this state is changed to OFFLINE_PARTIAL again? Please note that memory onlining is *completely* controllable by user space. User space can offline/online memory blocks as it wants. Not saying this might actually be the right thing to do - but we cannot trust that user space does the right thing. So at any point in time, you have to assume that a) added memory might not get onlined b) previously onlined memory might get offlined c) previously offline memory might get onlined > >> >> As long as we have our online_pages callback in place, we can hinder the >> unplugged 64MB from getting exposed to the buddy >> (virtio_mem_online_page_cb()). However, once we unloaded the driver, > > Yes, > > virtio_mem_set_fake_offline() would __SetPageOffline() to those pages. > >> this is no longer the case. If someone would online that memory block, >> we would expose unplugged memory to the buddy - very bad. >> > > Per my understanding, at this point of time, the memory block is at online > state. Even part of it is set to *fake* offline. > > So how could user trigger another online from sysfs interface? Assume we added a partially plugged memory block, which is now offline. Further assume user space did not online the memory block (e.g., no udev rules). User space could happily online the block after unloading the driver. Again, we have to assume user space could do crazy things. > >> So we have to remove these partially plugged, offline memory blocks when >> losing control over them. >> >> I tried to document that via: >> >> "After we unregistered our callbacks, user space can online partially >> plugged offline blocks. Make sure to remove them." >> >>> >>> Also, during virtio_mem_remove(), we just handle OFFLINE_PARTIAL memory block. >>> How about memory block in other states? It is not necessary to remove >>> ONLINE[_PARTIAL] memroy blocks? >> >> Blocks that are fully plugged (ONLINE or OFFLINE) can get >> onlined/offlined without us having to care. Works fine - we only have to >> care about partially plugged blocks. >> >> While we *could* unplug OFFLINE blocks, there is no way we can >> deterministically offline+remove ONLINE blocks. So that memory has to >> stay, even after we unloaded the driver (similar to the dax/kmem driver). > > For OFFLINE memory blocks, would that leave the situation: > > Guest doesn't need those pages, while host still maps them? Yes, but the guest could online the memory and make use of it. (again, whoever decides to unload the driver better be knowing what he does) To do it even more cleanly, we would a) Have to remove completely plugged offline blocks (not done) b) Have to remove partially plugged offline blocks (done) c) Actually send unplug requests to the hypervisor Right now, only b) is done, because it might actually cause harm (as discussed). However, the problem is, that c) might actually fail. Long short: we could add a) if it turns out to be a real issue. But than, unloading the driver isn't really suggested, the current implementation just "keeps it working without crashes" - and I guess that's good enough for now. > >> >> ONLINE_PARTIAL is already taken care of: it cannot get offlined anymore, >> as we still hold references to these struct pages >> (virtio_mem_set_fake_offline()), and as we no longer have the memory >> notifier in place, we can no longer agree to offline this memory (when >> going_offline). >> > > Ok, I seems to understand the logic now. > > But how we prevent ONLINE_PARTIAL memory block get offlined? There are three > calls in virtio_mem_set_fake_offline(), while all of them adjust page's flag. > How they hold reference to struct page? Sorry, I should have given you the right pointer. (similar to my other reply) We hold a reference either via 1. alloc_contig_range() 2. memmap init code, when not calling generic_online_page(). So these fake-offline pages can never be actually offlined, because we no longer have the memory notifier registered to fix that up. -- Thanks, David / dhildenb