Date:   Wed, 10 Nov 2021 12:39:25 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     David Hildenbrand <david@redhat.com>
Cc:     Qi Zheng <zhengqi.arch@bytedance.com>, akpm@linux-foundation.org,
        tglx@linutronix.de, kirill.shutemov@linux.intel.com,
        mika.penttila@nextfour.com, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        songmuchun@bytedance.com, zhouchengming@bytedance.com
Subject: Re: [PATCH v3 00/15] Free user PTE page table pages
Message-ID: <20211110163925.GX1740502@nvidia.com>
References: <20211110105428.32458-1-zhengqi.arch@bytedance.com>
 <20211110125601.GQ1740502@nvidia.com>
 <8d0bc258-58ba-52c5-2e0d-a588489f2572@redhat.com>
 <20211110143859.GS1740502@nvidia.com>
 <6ac9cc0d-7dea-0e19-51b3-625ec6561ac7@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6ac9cc0d-7dea-0e19-51b3-625ec6561ac7@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?0aH2ZOEXcFJYQsSEDE4Ghj+AkAkTgbEtXjjuImihVpqeZcJAQZDtSynoSB/Y?=
 =?us-ascii?Q?42RgT1gfMexm5z9Eq6kwQ2pO058p5T3u3FCJ/afDhbTD75uXIL+1+Svx4YKO?=
 =?us-ascii?Q?BMnWwPfWEpMw6nK7XN+8Mhr1iftUE0BztVZ3EJltI/j2kAFQp6Q4VaBHUsvk?=
 =?us-ascii?Q?G4lFf+6ilrOtHejEcLrYzGkZ0jND1r8Shj4rsVxU0I343BaE5D2Z1HUVRAad?=
 =?us-ascii?Q?z7klMxn9o9vbaJjcqL9PZFvW27wqlTmLOvGlKcI+/Ug/NrrTBU/rtKUYg6D7?=
 =?us-ascii?Q?N6DedNWv+TGAZZUTuV7lavWD9qGfc2FxNQ/zDEL/F8c6i5QCu14mIvA2ypQq?=
 =?us-ascii?Q?fBfC9ALfQDihG9IjGHJ7WvABOfHeqRal9wrZSHn2b+XqAMUumXJajVjVbU4T?=
 =?us-ascii?Q?Mbt3vBZ60BO6Txg1LkhDMqWOY8oJ94ZrRDjBedjHr/yV9ysBMyu+DwNQEeiB?=
 =?us-ascii?Q?Pz9L8dC+gtDY+rNMT8zxuMp66qa7PrigGLEMq2dY2ju31o4c5rLU6qfTz74d?=
 =?us-ascii?Q?ICZb9fZ0VCcv4kbL8h8ZCaH6RlY90l5CbJcStpjHvytXc8rZHap8LLdS9e7N?=
 =?us-ascii?Q?BitPnRdJTOfuQktP8Lw8L4ZtA89E8RwB3qQA8WNlXeyI0+nkDWffraoFKy2B?=
 =?us-ascii?Q?N9aZ+h/nkNbyDLerOLueNZQ97pta1dDFw2izPbZcYzFDkQeAemWJsj21/p4W?=
 =?us-ascii?Q?IZZX1/5ynyFnIGbuEn0/Z9sSuoUk6yjylO3u01DJUbpDInHniKhf7JOUJN+T?=
 =?us-ascii?Q?7tXCG8rrvT2FapajRhaMdIB3/IbhyHj8CCOqgxVv1jo/VuqBe5I3p0ZIrw3x?=
 =?us-ascii?Q?0sAgJTWtIEJ/9uaNexPZ5vQD+o0P+mG6+kFuqR0JtCBkvf6OLu7apw6L1N1O?=
 =?us-ascii?Q?fVvJxIFAixAofifFgk8iZXxeUoDM4+unX0dqTM7zRUwlDtUMsJmyRb1jcZsS?=
 =?us-ascii?Q?QBJoWbHzxN8jVNbqPeDpItLW8+MxMTBKM9rKIkUBAD7h4al9jrMqQfylOi16?=
 =?us-ascii?Q?8bB2nknZrXbbx7HPXyioE8z54vIYSuhdL/knImGIa3eqqjVqgASjWRpTSX1b?=
 =?us-ascii?Q?A0xet1rpcFONXKKDga0uc4Ahnwkuepx1z/NU9lvHAeRQVgKOL0u0hmU27NGU?=
 =?us-ascii?Q?rIN2oj6PFpbvjJuvH8w31KLQgmX8H6In0o3WupRXaS/mkgbPZVnwCFbqXdLg?=
 =?us-ascii?Q?MM+AofqSrPStSOkLy9QEIVTN7UYkrq96JQPZV375adq25yBTmVQtjUaHuVJz?=
 =?us-ascii?Q?QznR33v2YOPAzxZTvo0WsPVNg4ZSAk1mA9LiYPAp2qML4CZuogaKSboEJKOa?=
 =?us-ascii?Q?lxssCPBcjjp3dTuLfqgoV3qHHTcfW4OB3cBGu7zbzNc1WZ99TmT6gb97iUFZ?=
 =?us-ascii?Q?6WWij2J4O/ccv30yA6j9jYpbF6XWkCvovPz6+t0xGMq2p2E2PvihjGz4Q4N9?=
 =?us-ascii?Q?5QLnpBa9gtGY5ObtzM0ORtPKInq1vPKg6O5kCxUMKupcMxr3Gva3oUyJFqmB?=
 =?us-ascii?Q?BXgkMcx7tQ0/wwsGODKw/EKL127+O90fBHssCuP3Uy5PyVZmLjPfnMCvXx6t?=
 =?us-ascii?Q?/+JoGVpvtc3CwZepIIU=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e650456d-80d1-4705-f006-08d9a468a953
X-MS-Exchange-CrossTenant-AuthSource: BL0PR12MB5506.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2021 16:39:27.8331
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HpWltfINOHwC3WjTk0gdHQ4uzqWtT76xTGiGPTfPA9T9T1xfYWdIVo9gaOKmfXWA
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5143
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Nov 10, 2021 at 04:37:14PM +0100, David Hildenbrand wrote:

> >  - Fully locked. Under the PTLs (gup slow is an example)
> >  - Semi-locked. Under the read mmap lock and no PTLs (hmm is an example)
> >  - hw-locked. Barriered with TLB flush (gup fast is an example)
> 
> Three additions as far as I can tell:
> 
> 1. Fully locked currently needs the read mmap lock OR the rmap lock in
>    read. PTLs on their own are not sufficient AFAIKT.

I think the reality is we don't have any fully locked walkers.. Even
gup slow is semi-lockless until it reaches lower levels, then it takes
the PTLs. (I forgot about that detail!)

The mmap lock is being used to protect the higher levels of the page
table. It is part of its complicated dual purpose.

> 2. #1 and #2 can currently only walk within VMA ranges.

AFICT this is an artifact of re-using the mmap lock to protect the
page table and not being able to obtain the mmap lock in all the
places the page table structure is manipulated.

> 3. We can theoretically walk page tables outside VMA ranges with the
> write mmap lock, because page tables get removed with the mmap lock in
> read mode and heavy-weight operations (VMA layout, khugepaged) are
> performed under the write mmap lock.

Yes, again, an artifact of the current locking.
 
> The rmap locks protect from modifications where we want to exclude rmap
> walkers similarly to how we grab the mmap lock in write, where the PTLs
> are not sufficient.

It is a similar dual purpose locking as the mmap sem :(

> > #1 should be completely safe as the PTLs will protect eveything
> > #2 is safe so long as the write side is held during any layout changes
> > #3 interacts with the TLB flush, and is also safe with zap
> > 
> > rmap itself is a #1 page table walker, ie it gets the PTLs under
> > page_vma_mapped_walk().
> 
> When you talk about PTLs, do you mean only PTE-PTLs or also PMD-PTLs?

Ah, here I was thinking about a lock that can protect all the
levels. Today we are abusing the mmap lock to act as the pud_lock, for
instance.

> Because the PMD-PTLs re usually not taken in case we know there is a
> page table (nothing would currently change it without heavy
> locking).

This only works with the lockless walkers, and relies on the read mmap
sem/etc to also mean 'a PTE table cannot become a leaf PMD'

> For example, walk_page_range() requires the mmap lock in read and grabs
> the PTE-PTLs.

Yes, that is a semi-locked reader.

> It would still be a fairly coarse-grained locking, I am not sure if that
> is a step into the right direction. If you want to modify *some* page
> table in your process you have exclude each and every page table walker.
> Or did I mis-interpret what you were saying?

That is one possible design, it favours fast walking and penalizes
mutation. We could also stick a lock in the PMD (instead of a
refcount) and still logically be using a lock instead of a refcount
scheme. Remember modify here is "want to change a table pointer into a
leaf pointer" so it isn't an every day activity..

There is some advantage with this thinking because it harmonizes well
with the other stuff that wants to convert tables into leafs, but has
to deal with complicated locking.

On the other hand, refcounts are a degenerate kind of rwsem and only
help with freeing pages. It also puts more atomics in normal fast
paths since we are refcounting each PTE, not read locking the PMD.

Perhaps the ideal thing would be to stick a rwsem in the PMD. read
means a table cannot be come a leaf. I don't know if there is space
for another atomic in the PMD level, and we'd have to use a hitching
post/hashed waitq scheme too since there surely isn't room for a waitq
too..

I wouldn't be so quick to say one is better than the other, but at
least let's have thought about a locking solution before merging
refcounts :)

Jason