On Tue, Nov 13, 2012 at 02:49:10PM +0100, Ingo Molnar wrote:
>
> * Mel Gorman <mgorman@xxxxxxx> wrote:
>
> > > But given that most architectures will be just fine reusing
> > > the already existing generic PROT_NONE machinery, the far
> > > better approach is to do what we've been doing in generic
> > > kernel code for the last 10 years: offer a default generic
> > > version, and then to offer per arch hooks on a strict
> > > as-needed basis, if they want or need to do something weird
> > > ...
> >
> > If they are *not* fine with it, it's a large retrofit because
> > the PROT_NONE machinery has been hard-coded throughout. [...]
>
> That was a valid criticism for earlier versions of the NUMA
> patches - but should much less be the case in the latest
> iterations of the patches:
>
Which are where? They are possible somewhere in -tip, maybe the
tip/numa/core but I am seeing this;
$ git diff e657e078d3dfa9f96976db7a2b5fd7d7c9f1f1a6..tip/numa/core | grep change_prot_none
+change_prot_none(struct vm_area_struct *vma, unsigned long start, unsigned long end)
+ change_prot_none(vma, offset, end);
+ change_prot_none(vma, start, endvma);
This is being called from task_numa_work() for example so it's case where
the maintainer has to memember that prot_none actually means prot_numa in
this case. Further, the generic implementation of pte_numa is hard-coding
prot_none
+static bool pte_numa(struct vm_area_struct *vma, pte_t pte)
+{
.......
+ if (pte_same(pte, pte_modify(pte, vma->vm_page_prot)))
+ return false;
+
+ return pte_same(pte, pte_modify(pte, vma_prot_none(vma)));
+}
I can take the structuring idea of moving pte_numa around but it still
should have the _PAGE_NUMA naming. So it still looks to me as the PROT_NONE
machine is hard-coded.
> - it has generic pte_numa() / pmd_numa() instead of using
> prot_none() directly
>
I intend to move the pte_numa out myself.
> - the key utility functions are named using the _numa pattern,
> not *_prot_none*() anymore.
>
Where did change_prot_none() come from then?
> Let us know if you can still see such instances - it's probably
> simple oversight.
>
I could be lookjing at the wrong tip branch. Please post the full series
to the list so it can be reviewed that way instead of trying to second
guess.
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>