Re: [PATCH v1 04/11] mm/memremap: add ZONE_DEVICE support for compound pages

Matthew Wilcox
2021-05-05T19:00:01Z

On Wed, May 05, 2021 at 11:44:29AM -0700, Dan Williams wrote:
> > @@ -6285,6 +6285,8 @@ void __ref memmap_init_zone_device(struct zone *zone,
> > unsigned long pfn, end_pfn = start_pfn + nr_pages;
> > struct pglist_data *pgdat = zone->zone_pgdat;
> > struct vmem_altmap *altmap = pgmap_altmap(pgmap);
> > + unsigned int pfn_align = pgmap_pfn_align(pgmap);
> > + unsigned int order_align = order_base_2(pfn_align);
> > unsigned long zone_idx = zone_idx(zone);
> > unsigned long start = jiffies;
> > int nid = pgdat->node_id;
> > @@ -6302,10 +6304,30 @@ void __ref memmap_init_zone_device(struct zone *zone,
> > nr_pages = end_pfn - start_pfn;
> > }
> >
> > - for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> > + for (pfn = start_pfn; pfn < end_pfn; pfn += pfn_align) {
>
> pfn_align is in bytes and pfn is in pages... is there a "pfn_align >>=
> PAGE_SHIFT" I missed somewhere?

If something is measured in bytes, I like to use size_t (if it's
in memory) and loff_t (if it's on storage). The compiler doesn't do
anything useful to warn you, but it's a nice indication to humans about
what's going on. And it removes the temptation to do 'pfn_align >>=
PAGE_SHIFT' and suddenly take pfn_align from being measured in bytes to
being measured in pages.