On Sun, Jul 21, 2019 at 09:02:04AM -0700, Matthew Wilcox wrote:
> On Fri, Jul 19, 2019 at 12:29:53PM -0700, Ralph Campbell wrote:
> > Struct page for ZONE_DEVICE private pages uses the page->mapping and
> > and page->index fields while the source anonymous pages are migrated to
> > device private memory. This is so rmap_walk() can find the page when
> > migrating the ZONE_DEVICE private page back to system memory.
> > ZONE_DEVICE pmem backed fsdax pages also use the page->mapping and
> > page->index fields when files are mapped into a process address space.
> >
> > Restructure struct page and add comments to make this more clear.
>
> NAK. I just got rid of this kind of foolishness from struct page,
> and you're making it harder to understand, not easier. The comments
> could be improved, but don't lay it out like this again.
Was V1 of Ralphs patch ok? It seemed ok to me.
Ira
>
> > @@ -76,13 +76,35 @@ struct page {
> > * avoid collision and false-positive PageTail().
> > */
> > union {
> > - struct { /* Page cache and anonymous pages */
> > - /**
> > - * @lru: Pageout list, eg. active_list protected by
> > - * pgdat->lru_lock. Sometimes used as a generic list
> > - * by the page owner.
> > - */
> > - struct list_head lru;
> > + struct { /* Page cache, anonymous, ZONE_DEVICE pages */
> > + union {
> > + /**
> > + * @lru: Pageout list, e.g., active_list
> > + * protected by pgdat->lru_lock. Sometimes
> > + * used as a generic list by the page owner.
> > + */
> > + struct list_head lru;
> > + /**
> > + * ZONE_DEVICE pages are never on the lru
> > + * list so they reuse the list space.
> > + * ZONE_DEVICE private pages are counted as
> > + * being mapped so the @mapping and @index
> > + * fields are used while the page is migrated
> > + * to device private memory.
> > + * ZONE_DEVICE MEMORY_DEVICE_FS_DAX pages also
> > + * use the @mapping and @index fields when pmem
> > + * backed DAX files are mapped.
> > + */
> > + struct {
> > + /**
> > + * @pgmap: Points to the hosting
> > + * device page map.
> > + */
> > + struct dev_pagemap *pgmap;
> > + /** @zone_device_data: opaque data. */
> > + void *zone_device_data;
> > + };
> > + };
> > /* See page-flags.h for PAGE_MAPPING_FLAGS */
> > struct address_space *mapping;
> > pgoff_t index; /* Our offset within mapping. */
> > @@ -155,12 +177,6 @@ struct page {
> > spinlock_t ptl;
> > #endif
> > };
> > - struct { /* ZONE_DEVICE pages */
> > - /** @pgmap: Points to the hosting device page map. */
> > - struct dev_pagemap *pgmap;
> > - void *zone_device_data;
> > - unsigned long _zd_pad_1; /* uses mapping */
> > - };
> >
> > /** @rcu_head: You can use this to free a page by RCU. */
> > struct rcu_head rcu_head;
> > --
> > 2.20.1
> >
>
