Re: [rfc][patch 2/2] mm: introduce optional pte_special pte bit

[Nick Piggin <npiggin@suse.de>]

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.