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=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS autolearn=unavailable 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 41665C43381 for ; Thu, 28 Feb 2019 11:28:14 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1096320854 for ; Thu, 28 Feb 2019 11:28:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="tK37PKB+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1096320854 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=DvpiT2OKHfgGKgO40DNJMO3OOT6YGnB6pI/67arxn/c=; b=tK37PKB+WzJx9v noZZZZ48G3oRDuiRBLuJ0aSAXhIoKwDJdfV6BSYxDEKTPfZM/z4YWwWSd7+OZs3mENb3Ia0bswMnu 2x7C4fzRiyKvBRqUw1xcxL9Jdn6e8t+qn0kolVC/TKl/RBfWle2nybZm1G4tytolx756pQROpLlFd c/bR3LSt4OK262ttEMkCNfsiU7ZAMbP5DTrDhpZm32Cu5RmcPoU3+9yQeVoPulMVTPZlviyQjK+AA 8qzkpBEfKtsj9uS3HEkC/EjqhpFa93Iu+ib3QkMCeTjElrgI5/xG4ptsTmvlqEP0hZ5sC5d0YEesZ A5lHC1L59HHv8ejh7rbg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gzJrI-0004NT-CT; Thu, 28 Feb 2019 11:28:12 +0000 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70] helo=foss.arm.com) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gzJrE-0004Mm-OY for linux-arm-kernel@lists.infradead.org; Thu, 28 Feb 2019 11:28:10 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8A9D9EBD; Thu, 28 Feb 2019 03:28:06 -0800 (PST) Received: from [10.1.196.69] (e112269-lin.cambridge.arm.com [10.1.196.69]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6AABA3F738; Thu, 28 Feb 2019 03:28:03 -0800 (PST) Subject: Re: [PATCH v3 27/34] mm: pagewalk: Add 'depth' parameter to pte_hole To: Dave Hansen , linux-mm@kvack.org References: <20190227170608.27963-1-steven.price@arm.com> <20190227170608.27963-28-steven.price@arm.com> From: Steven Price Message-ID: <02b9ec67-75c5-4a36-9110-cc4ba6ee4f94@arm.com> Date: Thu, 28 Feb 2019 11:28:01 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190228_032808_810424_50CC8AC2 X-CRM114-Status: GOOD ( 21.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Dave Hansen , Arnd Bergmann , Ard Biesheuvel , Peter Zijlstra , Catalin Marinas , x86@kernel.org, Will Deacon , linux-kernel@vger.kernel.org, =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Ingo Molnar , Borislav Petkov , Andy Lutomirski , "H. Peter Anvin" , James Morse , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, "Liang, Kan" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 27/02/2019 17:38, Dave Hansen wrote: > On 2/27/19 9:06 AM, Steven Price wrote: >> #ifdef CONFIG_SHMEM >> static int smaps_pte_hole(unsigned long addr, unsigned long end, >> - struct mm_walk *walk) >> + __always_unused int depth, struct mm_walk *walk) >> { > > I think this 'depth' argument is a mistake. It's synthetic and it's > surely going to be a source of bugs. > > The page table dumpers seem to be using this to dump out the "name" of a > hole which seems a bit bogus in the first place. I'd much rather teach > the dumpers about the length of the hole, "the hole is 0x12340000 bytes > long", rather than "there's a hole at this level". I originally started by trying to calculate the 'depth' from (end - addr), e.g. for arm64: level = 4 - (ilog2(end - addr) - PAGE_SHIFT) / (PAGE_SHIFT - 3) However there are two issues that I encountered: * walk_page_range() takes a range of addresses to walk. This means that holes at the beginning/end of the range are clamped to the address range. This particularly shows up at the end of the range as I use ~0ULL as the end which leads to (~0ULL - addr) which is 1 byte short of the desired size. Obviously that particular corner-case is easy to work round, but it seemed fragile. * The above definition for arm64 isn't correct in all cases. You need to account for things like CONFIG_PGTABLE_LEVELS. Other architectures also have various quirks in their page tables. I guess I could try something like: static int get_level(unsigned long addr, unsigned long end) { /* Add 1 to account for ~0ULL */ unsigned long size = (end - addr) + 1; if (size < PMD_SIZE) return 4; else if (size < PUD_SIZE) return 3; else if (size < P4D_SIZE) return 2; else if (size < PGD_SIZE) return 1; return 0; } There are two immediate problems with that: * The "+1" to deal with ~0ULL is fragile * PGD_SIZE isn't what you might expect, it's not defined for most architectures and arm64/x86 use it as the size of the PGD table. Although that's easy enough to fix up. Do you think a function like above would be preferable? The other option would of course be to just drop the information from the debugfs file about at which level the holes are. But it can be useful information to see whether there are empty levels in the page table structure. Although this is an area where x86 and arm64 differ currently (x86 explicitly shows the gaps, arm64 doesn't), so if x86 doesn't mind losing that functionality that would certainly simplify things! Thanks, Steve _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel