Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 17 May 2021 16:13:57 +0200
From:   Gerald Schaefer <gerald.schaefer@linux.ibm.com>
To:     Anshuman Khandual <anshuman.khandual@arm.com>
Cc:     linux-mm@kvack.org, akpm@linux-foundation.org,
        Heiko Carstens <hca@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, x86@kernel.org,
        linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/thp: Make ARCH_ENABLE_SPLIT_PMD_PTLOCK dependent on
 PGTABLE_LEVELS > 2
Message-ID: <20210517161358.49683f34@thinkpad>
In-Reply-To: <a4403be6-5b74-2c86-bc4c-42ae4f0764dc@arm.com>
References: <1620621345-29176-1-git-send-email-anshuman.khandual@arm.com>
        <a4403be6-5b74-2c86-bc4c-42ae4f0764dc@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 17 May 2021 09:45:31 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:

> 
> 
> On 5/10/21 10:05 AM, Anshuman Khandual wrote:
> > ARCH_ENABLE_SPLIT_PMD_PTLOCK is irrelevant unless there are two page table
> > levels including PMD (also per Documentation/vm/split_page_table_lock.rst).
> > Make this dependency explicit on remaining platforms i.e x86 and s390 where
> > ARCH_ENABLE_SPLIT_PMD_PTLOCK is subscribed.

For s390, I don't think this makes a lot of sense. We always have 5 levels
defined for PGTABLE_LEVELS, and we would not even compile with any other
value, because of the "#error CONFIG_PGTABLE_LEVELS" in include/linux/pgtable.h.

Our pagetable folding also works a bit different than it does on other archs,
and we would actually have pmd level entries for 2-level pagetables, so it should
all work fine also with PGTABLE_LEVELS == 2 (if it was possible).

In fact, I do not really see why you would need "more than two levels" on any
arch, in order to use split PMD locks. Your description also just says
"irrelevant unless there are two page table levels", and not "more than two
levels", like in Documentation/vm/split_page_table_lock.rst.

Yet, your patch adds checks for "more than", so at least the description
seems a bit misleading. I assume that the "more than" has to do with folded
PMD on a 2-level system, but the way we fold on s390 I do not see why that
should be a problem. Could you please elaborate a bit?

We also have different levels of pagetables for kernel (CONFIG_PGTABLE_LEVELS)
and user processes on s390. The latter can have dynamic levels, currently
starting with 3, but previously we also had 2 levels for compat tasks e.g.
These dynamic levels for user processes are also independent from the
CONFIG_PGTABLE_LEVELS used for the kernel pagetable, while the split PMD lock
of course also affects user process pagetables, so that would be another
reason not to add such a dependency for ARCH_ENABLE_SPLIT_PMD_PTLOCK on s390.