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=-9.4 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=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 8F878C07E9A for ; Tue, 13 Jul 2021 01:20:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1FE466115A for ; Tue, 13 Jul 2021 01:20:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1FE466115A 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 45FCE6B0095; Mon, 12 Jul 2021 21:20:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 436CA6B0096; Mon, 12 Jul 2021 21:20:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2FFF76B0098; Mon, 12 Jul 2021 21:20:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0075.hostedemail.com [216.40.44.75]) by kanga.kvack.org (Postfix) with ESMTP id 0BF916B0095 for ; Mon, 12 Jul 2021 21:20:35 -0400 (EDT) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 050578248047 for ; Tue, 13 Jul 2021 01:20:34 +0000 (UTC) X-FDA: 78355809588.23.5DC6A24 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by imf24.hostedemail.com (Postfix) with ESMTP id A530EB000099 for ; Tue, 13 Jul 2021 01:20:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1626139233; h=from:from:reply-to: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=LFDPWJMPXLTaGBNimWBNvQUJexQfku2Sy2ULaI6Wx8A=; b=L/UZKQWAO5OR4ign9L8TSy0Pg2zknjbVD07PTJVXQ4iHIvf9XHTS3ApHgf+21Oh+xbru4C b/vM98Ksiz9e1X2bT0dy/1BI4/w4LGeqKXlPNPAad/cjKNDgna/H0lY6BWYPnYEJPLuUNa Tv7dpq2Ox8xGLepcMRP3QvaUSlhwSqM= 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-391-VyItZFU6NKWe2o5virut4g-1; Mon, 12 Jul 2021 21:20:31 -0400 X-MC-Unique: VyItZFU6NKWe2o5virut4g-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 24EA25074B; Tue, 13 Jul 2021 01:20:30 +0000 (UTC) Received: from [10.64.54.119] (vpn2-54-119.bne.redhat.com [10.64.54.119]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 376375C232; Tue, 13 Jul 2021 01:20:26 +0000 (UTC) Reply-To: Gavin Shan Subject: Re: [PATCH 00/12] mm/debug_vm_pgtable: Enhancements To: Anshuman Khandual , linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org, catalin.marinas@arm.com, will@kernel.org, akpm@linux-foundation.org, shan.gavin@gmail.com, chuhu@redhat.com References: <20210706061748.161258-1-gshan@redhat.com> <42a26202-10f7-e744-3fc5-c9e5a7445193@arm.com> From: Gavin Shan Message-ID: Date: Tue, 13 Jul 2021 11:20:23 +1000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0 MIME-Version: 1.0 In-Reply-To: <42a26202-10f7-e744-3fc5-c9e5a7445193@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: A530EB000099 X-Stat-Signature: ar5eadwzcjiirwq8bk8eq8qfrt1wsez1 Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="L/UZKQWA"; spf=none (imf24.hostedemail.com: domain of gshan@redhat.com has no SPF policy when checking 216.205.24.124) smtp.mailfrom=gshan@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-HE-Tag: 1626139233-504155 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: Hi Anshuman, On 7/12/21 2:14 PM, Anshuman Khandual wrote: > Though I have not jumped into the details for all individual > patches here but still have some high level questions below. > > On 7/6/21 11:47 AM, Gavin Shan wrote: >> There are couple of issues with current implementations and this series >> tries to resolve the issues: >> >> (a) All needed information are scattered in variables, passed to various >> test functions. The code is organized in pretty much relaxed fashion. > All these variables are first prepared in debug_vm_pgtable(), before > getting passed into respective individual test functions. Also these > test functions receive only the required number of variables not all. > Adding a structure that captures all test parameters at once before > passing them down will be unnecessary. I am still wondering what will > be the real benefit of this large code churn ? > Thanks for your review. There are couple of reasons to have "struct vm_pgtable_debug". (1) With the struct, the old and new implementation can coexist. In this way, the patches in this series can be stacked up easily. (2) I think passing single struct to individual test functions improves the code readability. Besides, it also makes the empty stubs simplified. (3) The code can be extended easily if we need in future. >> >> (b) The page isn't allocated from buddy during page table entry modifying >> tests. The page can be invalid, conflicting to the implementations >> of set_{pud, pmd, pte}_at() on ARM64. The target page is accessed >> so that the iCache can be flushed when execution permission is given >> on ARM64. Besides, the target page can be unmapped and access to >> it causes kernel crash. > > Using 'start_kernel' based method for struct page usage, enabled this > test to run on platforms which might not have enough memory required > for various individual test functions. This method is not a problem for > tests that just need an aligned pfn (which creates a page table entry) > not a real struct page. > > But not allocating and owning the struct page might be problematic for > tests that expect a real struct page and transform its state via set_ > {pud, pmd, pte}_at() functions as reported here. > Yeah, I totally agree. The series follows what you explained: Except the test cases where set_{pud, pmd, pte}_at() is used, the allocated page is used. For other test cases, 'start_kernel' based PFN is used as before. >> >> "struct vm_pgtable_debug" is introduced to address issue (a). For issue >> (b), the used page is allocated from buddy in page table entry modifying >> tests. The corresponding tets will be skipped if we fail to allocate the >> (huge) page. For other test cases, the original page around to kernel >> symbol (@start_kernel) is still used. > > For all basic pfn requiring tests, existing 'start_kernel' based method > should continue but allocate a struct page for other tests which change > the passed struct page. Skipping the tests when allocation fails is the > right thing to do. > Yes, it's exactly what this series does. Hope you can jump into the details when you get a chance :) Thanks, Gavin