Re: [PATCH 1/7] mm: Document x86 uses a linked list of pgds

From: Ira Weiny
Date: Wed Apr 29 2020 - 14:29:54 EST


On Tue, Apr 28, 2020 at 03:52:51PM -0700, Matthew Wilcox wrote:
> On Tue, Apr 28, 2020 at 02:41:09PM -0700, Ira Weiny wrote:
> > On Tue, Apr 28, 2020 at 12:44:43PM -0700, Matthew Wilcox wrote:
> > > x86 uses page->lru of the pages used for pgds, but that's not immediately
> > > obvious to anyone looking to make changes. Add a struct list_head to
> > > the union so it's clearly in use for pgds.
> >
> > Shouldn't pgd_list_{add,del}() use this list head variable instead of lru to
> > complete the documentation?
> >
> > Probably the list iteration loops arch/x86/* as well?
>
> Yes, but I felt that was out of scope for this patchset. Untangling the
> uses of struct page is a long and messy business; if we have to fix
> everything at once, we'll never get anywhere. There's also the slab
> users of page->lru instead of page->slab_list.

But doesn't changing the use of lru with this new name in the code also help to
identify the users?

>
> What I actually want to get to is:
>
> struct page {
> unsigned long flags;
> union {
> struct file_page file;
> struct anon_page anon;
> struct pt_page pt;
> struct slab_page slab;
> struct tail_page tail;
> struct rcu_head rcu;
> };
> union {
> atomic_t _mapcount;
> ...
> };
> atomic_t refcount;
> ...
> };
>
> and then we can refer to page->pt.list and so on.

Then later on we know exactly where page->pt.list needs to be inserted.

I'm not opposed to the patch as it is. But as someone newer it seems like the
following documents the use of lru as much if not more.

Compile tested only but feel free to merge if you like.
Ira