On Wed, 16 Jan 2008, Nick Piggin wrote:
>
> My biggest concen is that s390 basically are thinking about doing
> something like this:
>
> if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) {
> if (vma->vm_flags & VM_MIXEDMAP) {
> #ifdef s390
> if (pte_special(pte))
> return NULL;
> #else
> if (!pfn_valid(pfn))
> return NULL;
> #endif
Ok. So now that I actually know what the reason for the insanity was.
> Basically what I want to do is make the pte_special part usable by
> all architectures, and bring it out of the vma tests.
But if it's always there, then there is no reason for *any* of this. The
right thing is to just always do
return pte_special(pte) ? NULL : pte_page(pte);
and I'd be happy with that too. I'm just not happy with the mixing, and
with the #ifdef inside code.
All the other code only makes sense if there isn't a special bit in the
pte, but I was literally told that the only architecture that does *not*
have any free bits for that is S390. So now you use S390 as an excuse to
do that "pte_special()", which is what I think EVERYBODY ELSE wanted to do
in the first place!
Can you see why I'm not so enamoured with the patch? Either it makes
sense, and *everybody* should just use that simple one-liner, or it
doesn't make sense, and *nobody* should do it.
It's the "both cases" that just drives me wild. It doesn't make sense.
Linus
-
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
