On Sat, Jan 12, 2008 at 08:45:06PM -0800, Linus Torvalds wrote:
>
>
> On Sun, 13 Jan 2008, Nick Piggin wrote:
> > On Sat, Jan 12, 2008 at 07:41:16PM -0800, Linus Torvalds wrote:
> > >
> > > What's the point of this?
> >
> > Well, it's written in the changelog.
>
> No it's not.
>
> It says "Rather than play interesting games with vmas..", but:
OK, you have to read to the 3rd paragraph.
> > > That's lots of new (ugly) code, and two totally different paths, that
> > > aren't even cleanly abstracted, so now there's two separate things that
> > > are just arbitrarily selected by an #ifdef.
> >
> > How should it be cleanly abstracted?
>
> I don't think it can, since you have to leave the old code anyway.
I don't think having 2 code paths means it is a bad abstraction at
all. We do things like that everywhere, what makes you say this is
bad?
My implementation is a bit crude maybe. You could have another arch
call so you write it with an if() instead of an ifdef I guess.
> Which just returns me to the original question: what's the actual
> improvement here?
Well the immediate improvement from this actual patch is just that
it gives better and smaller code for vm_normal_page (even if you
discount the debug checks in the existing code).
But for example, s390 perhaps cannot implement VM_MIXEDMAP using
pfn_valid() like we have in vm_normal_page. The alternative is basically
to have a different path for those guys anyway, so why not make it a
"core" code thing rather than s390 specific.
So. Is there a big problem with adding this extra path? It's really quite
simple, literally just
if (likely(!pte_special(pte)))
return pte_page(pte);
return NULL;
So I don't think maintainability is a problem.
-
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
