Anshuman Khandual's on June 10, 2019 3:42 pm: > > > On 06/10/2019 10:08 AM, Nicholas Piggin wrote: >> ioremap_page_range is a generic function to create a kernel virtual >> mapping, move it to mm/vmalloc.c and rename it vmap_range. > > Absolutely. It belongs in mm/vmalloc.c as its a kernel virtual range. > But what is the rationale of changing the name to vmap_range ? Well it doesn't just map IO. It's for arbitrary kernel virtual mapping (including ioremap). Last patch uses it to map regular cacheable memory. >> For clarity with this move, also: >> - Rename vunmap_page_range (vmap_range's inverse) to vunmap_range. > > Will be inverse for both vmap_range() and vmap_page[s]_range() ? Yes. > >> - Rename vmap_page_range (which takes a page array) to vmap_pages. > > s/vmap_pages/vmap_pages_range instead here ................^^^^^^ Yes. > This deviates from the subject of this patch that it is related to > ioremap only. I believe what this patch intends is to create > > - vunmap_range() takes [VA range] > > This will be the common kernel virtual range tear down > function for ranges created either with vmap_range() or > vmap_pages_range(). Is that correct ? > - vmap_range() takes [VA range, PA range, prot] > - vmap_pages_range() takes [VA range, struct pages, prot] That's right although we already have all those functions, so I don't create anything, only move and re-name. I'm happy to change the subject if you have a preference. > Can we re-order the arguments (pages <--> prot) for vmap_pages_range() > just to make it sync with vmap_range() ? > > static int vmap_pages_range(unsigned long start, unsigned long end, > pgprot_t prot, struct page **pages) > Sure, makes sense. Thanks, Nick