Re: [PATCH 01/11] pagewalk: update page table walker core

[Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>]

On Mon, Jun 02, 2014 at 04:49:18PM -0700, Dave Hansen wrote:
> On 02/10/2014 01:44 PM, Naoya Horiguchi wrote:
> > When we try to use multiple callbacks in different levels, skip control is
> > also important. For example we have thp enabled in normal configuration, and
> > we are interested in doing some work for a thp. But sometimes we want to
> > split it and handle as normal pages, and in another time user would handle
> > both at pmd level and pte level.
> > What we need is that when we've done pmd_entry() we want to decide whether
> > to go down to pte level handling based on the pmd_entry()'s result. So this
> > patch introduces a skip control flag in mm_walk.
> > We can't use the returned value for this purpose, because we already
> > defined the meaning of whole range of returned values (>0 is to terminate
> > page table walk in caller's specific manner, =0 is to continue to walk,
> > and <0 is to abort the walk in the general manner.)
> 
> This seems a bit complicated for a case which doesn't exist in practice
> in the kernel today.  We don't even *have* a single ->pte_entry handler.

Following users have their own pte_entry() by latter part of this patchset:
- queue_pages_range()
- mem_cgroup_count_precharge()
- show_numa_map()
- pagemap_read()
- clear_refs_write()
- show_smap()
- or1k_dma_alloc()
- or1k_dma_free()
- subpage_mark_vma_nohuge

>  Everybody just sets ->pmd_entry and does the splitting and handling of
> individual pte entries in there.

Walking over every pte entry under some pmd is common task, so if you don't
have any good reason, we should do it in mm/pagewalk.c side, not in each
pmd_entry() callback. (Callbacks should focus on their own task.)

>  The only reason it's needed is because
> of the later patches in the series, which is kinda goofy.

Most of current users use pte_entry() in the latest linux-mm.
Only few callers (mem_cgroup_move_charge() and force_swapin_readahead())
make their pmd_entry() handle pte-level walk in their own way.

BTW, we have some potential callers of page table walker which currently
does page walk completely in their own way. Here's the list:
- mincore()
- copy_page_range()
- remap_pfn_range()        
- zap_page_range()         
- free_pgtables()          
- vmap_page_range_noflush()
- change_protection_range()
Yes, my work for cleanuping page table walker is still on the way.

> I'm biased, but I think the abstraction here is done in the wrong place.
> 
> Naoya, could you take a looked at the new handler I proposed?  Would
> that help make this simpler?

I'll look through this series later and I'd like to add some of your
patches on top of this patchset.

Thanks,
Naoya Horiguchi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>