# Re: \[PATCH v3 04/14\] mm/memremap: add ZONE\_DEVICE support for compound pages

- From: Joao Martins \<hidden\>
- Date: 2021-07-15 13:15:42 
- List: linux-doc
- Page: https://ratatoskr.run/linux-doc/2021/07/6509864
- Thread: https://ratatoskr.run/linux-mm/2021/07/6509813/t.md

```text
On 7/15/21 7:48 AM, Christoph Hellwig wrote:
>> +static inline unsigned long pgmap_geometry(struct dev_pagemap *pgmap)
>> +{
>> +	if (!pgmap || !pgmap->geometry)
>> +		return PAGE_SIZE;
>> +	return pgmap->geometry;
> 
> Nit, but avoiding all the negations would make this a little easier to
> read:
> 
> 	if (pgmap && pgmap->geometry)
> 		return pgmap->geometry;
> 	return PAGE_SIZE
> 
Nicer indeed.

But this might be removed, should we follow Dan's suggestion on geometry representing nr
of pages.


>> +	if (pgmap_geometry(pgmap) > PAGE_SIZE)
>> +		percpu_ref_get_many(pgmap->ref, (pfn_end(pgmap, range_id)
>> +			- pfn_first(pgmap, range_id)) / pgmap_pfn_geometry(pgmap));
>> +	else
>> +		percpu_ref_get_many(pgmap->ref, pfn_end(pgmap, range_id)
>> +				- pfn_first(pgmap, range_id));
> 
> This is a horrible undreadable mess, which is trivially fixed by a
> strategically used local variable:
> 
> 	refs = pfn_end(pgmap, range_id) - pfn_first(pgmap, range_id);
> 	if (pgmap_geometry(pgmap) > PAGE_SIZE)
> 		refs /= pgmap_pfn_geometry(pgmap);
> 	percpu_ref_get_many(pgmap->ref, refs);
> 
> 
Yeap, much readable, thanks for the suggestion.
```