# Re: \[PATCH v1 06/11\] mm/sparse-vmemmap: refactor vmemmap\_populate\_basepages()

- From: Joao Martins \<hidden\>
- Date: 2021-05-06 10:28:11 
- List: linux-mm
- Page: https://ratatoskr.run/linux-mm/2021/05/6507986
- Thread: https://ratatoskr.run/linux-mm/2021/03/6507171/t.md

```text


On 5/5/21 11:43 PM, Dan Williams wrote:
> I suspect it's a good sign I'm only finding cosmetic and changelog
> changes in the review... 

Hopefully it continues that way, but the meat of the series is located
in patches 4, 6, 7 and 11. *Specially* 6 and 7.

I very strongly suspect I am gonna get non-cosmetic comments there.

> I have some more:
> 
> A year for now if I'm tracking down a problem and looking through mm
> commits I would appreciate a subject line like the following:
> "refactor core of vmemmap_populate_basepages() to helper" that gives
> an idea of the impact and side effects of the change.
> 
Fixed.

> On Thu, Mar 25, 2021 at 4:10 PM Joao Martins [ref] wrote:
>>
> 
> I would add a lead in phrase like: "In preparation for describing a
> memmap with compound pages, move the actual..."
> 
>> Move the actual pte population logic into a separate function
>> vmemmap_populate_address() and have vmemmap_populate_basepages()
>> walk through all base pages it needs to populate.
> 
> Aside from the above, looks good.
>
Cool!

```