Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S979303AbdDYAAH (ORCPT ); Mon, 24 Apr 2017 20:00:07 -0400 Received: from mail-qk0-f171.google.com ([209.85.220.171]:32938 "EHLO mail-qk0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S979080AbdDYAAD (ORCPT ); Mon, 24 Apr 2017 20:00:03 -0400 Subject: Re: [PATCH 4/5] Hot-remove implementation for arm64 To: Andrea Reale References: <897973dd5d3fc91c70aba4b44350099a61c3a12c.1491920513.git.ar@linux.vnet.ibm.com> <20170411171210.GD32267@leverpostej> <20170414140158.GA17950@samekh> <20170418182125.GL17866@leverpostej> <535ba380-56e8-db3d-25c5-14d51e48105f@redhat.com> <20170421100500.GB20029@samekh> Cc: Ard Biesheuvel , Mark Rutland , "linux-arm-kernel@lists.infradead.org" , Florian Fainelli , m.bielski@virtualopensystems.com, scott.branden@broadcom.com, Will Deacon , "linux-kernel@vger.kernel.org" , Xishi Qiu From: Laura Abbott Message-ID: <3a5afc84-b920-e164-c05a-2c6a3e05bf84@redhat.com> Date: Mon, 24 Apr 2017 16:59:58 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170421100500.GB20029@samekh> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2707 Lines: 71 On 04/21/2017 03:05 AM, Andrea Reale wrote: > Hi all, > > thanks for taking the time to comment. Replies in-line. > > On Wed, Apr 19, 2017 at 08:53:13AM -0700, Laura Abbott wrote: >> On 04/18/2017 11:48 AM, Ard Biesheuvel wrote: >>> On 18 April 2017 at 19:21, Mark Rutland wrote: >>>> On Fri, Apr 14, 2017 at 03:01:58PM +0100, Andrea Reale wrote: > > [...] > >>>> >>>> From a quick scan, I see that it's necessary to use pgtable_page_ctor() >>>> for pages that will be used for userspace page tables, but it's not >>>> clear to me if it's ever necessary for pages used for kernel page >>>> tables. >>>> >>>> If it is, we appear to have a bug on arm64. >>>> >>>> Laura, Ard, thoughts? >>>> >>> >>> The generic apply_to_page_range() will expect the PTE lock to be >>> initialized for page table pages that are not part of init_mm. For >>> arm64, that is precisely efi_mm as far as I am aware. For EFI, the >>> locking is unnecessary but does no harm (the permissions are set once >>> via apply_to_page_range() at boot), so I added this call when adding >>> support for strict permissions in EFI rt services mappings. >>> >>> So I think it is appropriate for create_pgd_mapping() to be in charge >>> of calling the ctor(). We simply have no destroy_pgd_mapping() >>> counterpart that would be the place for the dtor() call, given that we >>> never take down EFI rt services mappi > >>> Whether it makes sense or not to lock/unlock in apply_to_page_range() >>> is something I did not spend any brain cycles on at the time. >>> >> >> Agreed there shouldn't be a problem right now. I do think the locking is >> appropriate in apply_to_page_range given what other functions also get >> locked. >> >> I really wish this were less asymmetrical though since it get hard >> to reason about. It looks like hotplug_paging will call the ctor, >> so is there an issue with calling hot-remove on memory that was once >> hot-added or is that not a concern? >> >> Thanks, >> Laura > > I think the confusion comes from the fact that, in hotplug_paging, we are > passing pgd_pgtable_alloc as the page allocator for __create_pgd_mapping, > which always calls the ctor. > > If I got things right (but, please, correct me if I am wrong), we don't > need to get the pte_lock that the ctor gets since - in hotplug - we are > adding to init_mm. > > Moreover, I am just realizing that calling the dtor while hot-removing > might create problems when removing memory that *was not* previously > hotplugged, as we are calling a dtor on something that was never > ctor'ed. Is that what you were hinting at, Laura? > > Thanks and best regards, > Andrea > Yes, that was what I was thinking. Thanks, Laura