Struct page cleanup [LWN.net]

Struct page cleanup

Posted May 1, 2018 0:03 UTC (Tue) by willy (subscriber, #9762)
Parent article: Willy's memory-management to-do list

I made a few cleanups to struct page last cycle, so it used to be worse ;-)
https://elixir.bootlin.com/linux/v4.15.18/source/include/...

I didn't know where I was going at the time; it wasn't until I cleaned up some bits that I could see how to clean up other bits.

Anyway, the patches to rearrange struct page into the later version are now posted:
https://marc.info/?l=linux-mm&m=152511980529195&w=2

to post comments

Struct page cleanup

Posted May 1, 2018 7:58 UTC (Tue) by Sesse (subscriber, #53779) [Link] (5 responses)

Am I stupid for not understanding your diagram? :-) I don't understand what the horizontal and vertical axes are. It sort of looks like horizontal counts bits (because the first few things are the page flags, right?), except it possibly can't. Are the things along the horizontal axes perhaps different uses (ie., a union)? Why are they numbered, then?

Struct page cleanup

Posted May 1, 2018 11:14 UTC (Tue) by willy (subscriber, #9762) [Link] (4 responses)

It helps to be in the room when I'm explaining the diagram :-)

For the first three tabs, each row represents four bytes. The first row (or two rows for the 64-bit struct page) are 'flags', and each cell represents one bit with a two-letter abbreviation of the name of the flag.
Then there's a row of headers which enumerate some of the users of struct page (page cache, anon, slab, slub, page tables, etc)
At this point the meaning of each cell changes; one cell represents one byte.
The dark lines represent boundaries within the struct page, eg we currently have:

struct page {
union {
struct address_space *mapping;
/* deferred_list.head */
};
union {
unsigned long index;
/* deferred_list.tail */
};
...
};

so there's a dark line between mapping and index to show that we can't just put a list_head into the union.

Fields which have the same meaning between all page types are shown without vertical lines (eg refcount) while fields which happen to be used for the same meaning between different page types are shown with vertical lines.

Hope that's helpful. Yes, it's a confusing diagram. I spent hours trying to come up with a good graphical representation of what's going on in struct page, and this was the best I could do.

Struct page cleanup

Posted May 1, 2018 11:16 UTC (Tue) by Sesse (subscriber, #53779) [Link] (3 responses)

Maybe you should add an extra blank line after the flags, to point out that the numbering no longer matters there?

Also, I wonder, why are people all “WE ARE OUT OF PAGE FLAGS” all the time, when there are clearly tons of them left for 64-bit and a bunch even for 32-bit?

Struct page cleanup

Posted May 1, 2018 11:49 UTC (Tue) by willy (subscriber, #9762) [Link] (2 responses)

I've added the blank line as requested. I think that improves things for people following along at home, but I'm not sure it would have fit on the projector's screen :-)

My slot wasn't focused on the flags, so I didn't fill in all the rest of the details; I juts handwaved at it because everybody else in the room knew what other things were in those bits. I've filled in the fields that are commonly stored in those bits; the problem is that they're all dependent on various CONFIG options and I haven't taken the trouble to figure out likely values in order to size the fields to realistic number of bits. If you can make sense of page-flags-layout.h and the rest of the generating machinery, please let me know. The first problem you'll face is that whoever did the ASCII art in those files was a fan of big-endian bit numbering (probably from IBM).

Struct page cleanup

Posted May 1, 2018 11:51 UTC (Tue) by Sesse (subscriber, #53779) [Link] (1 responses)

Thanks! That made it clearer for me, at least. :-)

Struct page cleanup

Posted May 1, 2018 12:23 UTC (Tue) by willy (subscriber, #9762) [Link]

Excellent! Thanks for the constructive criticism!