All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anshuman Khandual <anshuman.khandual@arm.com>
To: Gavin Shan <gshan@redhat.com>, 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
Subject: Re: [PATCH 00/12] mm/debug_vm_pgtable: Enhancements
Date: Mon, 12 Jul 2021 09:44:46 +0530	[thread overview]
Message-ID: <42a26202-10f7-e744-3fc5-c9e5a7445193@arm.com> (raw)
In-Reply-To: <20210706061748.161258-1-gshan@redhat.com>

Hi Gavin,

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 ?

> 
>   (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.

> 
> "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.

- Anshuman

  parent reply	other threads:[~2021-07-12  4:14 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-06  6:17 [PATCH 00/12] mm/debug_vm_pgtable: Enhancements Gavin Shan
2021-07-06  6:17 ` [PATCH 01/12] mm/debug_vm_pgtable: Introduce struct vm_pgtable_debug Gavin Shan
2021-07-14  6:24   ` Anshuman Khandual
2021-07-19  5:39     ` Gavin Shan
2021-07-20  7:02       ` Anshuman Khandual
2021-07-20 23:07         ` Gavin Shan
2021-07-21  4:20           ` Anshuman Khandual
2021-07-06  6:17 ` [PATCH 02/12] mm/debug_vm_pgtable: Use struct vm_pgtable_debug in basic tests Gavin Shan
2021-07-06  6:17 ` [PATCH 03/12] mm/debug_vm_pgtable: Use struct vm_pgtable_debug in leaf and savewrite tests Gavin Shan
2021-07-06  6:17 ` [PATCH 04/12] mm/debug_vm_pgtable: Use struct vm_pgtable_debug in protnone and devmap tests Gavin Shan
2021-07-06  6:17 ` [PATCH 05/12] mm/vm_debug_pgtable: Use struct vm_pgtable_debug in soft_dirty and swap tests Gavin Shan
2021-07-06  6:17 ` [PATCH 06/12] mm/debug_vm_pgtable: Use struct vm_pgtable_debug in migration and thp tests Gavin Shan
2021-07-06  6:17 ` [PATCH 07/12] mm/debug_vm_pgtable: Use struct vm_pgtable_debug in PTE modifying tests Gavin Shan
2021-07-06  6:17 ` [PATCH 08/12] mm/debug_vm_pgtable: Use struct vm_pgtable_debug in PMD " Gavin Shan
2021-07-06  6:17 ` [PATCH 09/12] mm/vm_debug_pgtable: Use struct vm_pgtable_debug in PUD " Gavin Shan
2021-07-06  6:17 ` [PATCH 10/12] mm/debug_vm_pgtable: Use struct vm_pgtable_debug in PGD and P4D " Gavin Shan
2021-07-06  6:17 ` [PATCH 11/12] mm/debug_vm_pgtable: Remove unused code Gavin Shan
2021-07-06  6:17 ` [PATCH 12/12] mm/debug_vm_pgtable: Fix corrupted page flag Gavin Shan
2021-07-12  4:14 ` Anshuman Khandual [this message]
2021-07-13  1:20   ` [PATCH 00/12] mm/debug_vm_pgtable: Enhancements Gavin Shan
2021-07-14  5:26     ` Anshuman Khandual
2021-07-15  5:17       ` Gavin Shan
2021-07-18  6:36         ` Anshuman Khandual

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=42a26202-10f7-e744-3fc5-c9e5a7445193@arm.com \
    --to=anshuman.khandual@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=chuhu@redhat.com \
    --cc=gshan@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shan.gavin@gmail.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.