On Fri, Feb 03, 2023 at 04:19:32PM +0000, Matthew Wilcox wrote: > On Fri, Feb 03, 2023 at 04:00:08PM +0000, Hyeonggon Yoo wrote: > > On Mon, Jan 30, 2023 at 05:11:48AM +0000, Matthew Wilcox wrote: > > > On Mon, Jan 30, 2023 at 01:34:59PM +0900, Hyeonggon Yoo wrote: > > > > > Seems like quite some changes to page_type to accomodate SLAB, which is > > > > > hopefully going away soon(TM). Could we perhaps avoid that? > > > > > > > > If it could be done with less changes, I'll try to avoid that. > > > > > > Let me outline the idea I had for removing PG_slab: > > > > > > Observe that PG_reserved and PG_slab are mutually exclusive. Also, > > > if PG_reserved is set, no other flags are set. If PG_slab is set, only > > > PG_locked is used. Many of the flags are only for use by anon/page > > > cache pages (eg referenced, uptodate, dirty, lru, active, workingset, > > > waiters, error, owner_priv_1, writeback, mappedtodisk, reclaim, > > > swapbacked, unevictable, mlocked). > > > > > > Redefine PG_reserved as PG_kernel. Now we can use the other _15_ > > > flags to indicate pagetype, as long as PG_kernel is set. > > > > So PG_kernel is a new special flag, I thought it indicates > > "not usermappable pages", but considering PG_vmalloc it's not. > > Right, it means "The kernel allocated this page for its own purposes; > what that purpose is might be available by looking at PG_type". ie > it's not-anon, not-page-cache. > > > > So, eg > > > PageSlab() can now be (page->flags & PG_type) == PG_slab where > > > > But if PG_xxx and PG_slab shares same bit, PG_xxx would be confused? > > Correct. Ideally those tests wouldn't be used on arbitrary pages, > only pages which are already confirmed to be anon or file. I suspect > we haven't been super-careful about that in the past, and so there > would be some degree of "Oh, we need to fix this up". But flags like > PG_mappedtodisk, PG_mlocked, PG_unevictable, PG_workingset should be > all gated behind "We know this is anon/file". Okay. let's just try then! > > > PG_dirty 0x000008 > > > PG_owner_priv_1 0x000010 > > > PG_arch_1 0x000020 > > > PG_private 0x000040 > > > PG_waiters 0x000080 > > > PG_kernel 0x000100 > > > PG_referenced 0x000200 > > > PG_uptodate 0x000400 > > > PG_lru 0x000800 > > > PG_active 0x001000 > > > PG_workingset 0x002000 > > > PG_error 0x004000 > > > PG_private_2 0x008000 > > > PG_mappedtodisk 0x010000 > > > PG_reclaim 0x020000 > > > PG_swapbacked 0x040000 > > > PG_unevictable 0x080000 > > > PG_mlocked 0x100000 > > > > > > ... or something. There are a number of constraints and it may take > > > a few iterations to get this right. Oh, and if this is the layout > > > we use, then: > > > > > > PG_type 0x1fff00 > > > PG_reserved (PG_kernel | 0x200) > > > PG_slab (PG_kernel | 0x400) > > > PG_buddy (PG_kernel | 0x600) > > > PG_offline (PG_kernel | 0x800) > > > PG_table (PG_kernel | 0xa00) > > > PG_guard (PG_kernel | 0xc00) > > > PG_vmalloc (PG_kernel | 0xe00) > > > > what is PG_vmalloc for, is it just an example for > > explaining possible layout? > > I really want to mark pages as being allocated from vmalloc. It's > one of the things we could do to make debugging better. Got it. Anyway, the proposed approach is not compatible with this series so I'll try implementing new series based on your outline. Thanks!
