Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer

From: Linus Torvalds
Date: Fri Sep 10 2010 - 18:07:34 EST


On Fri, Sep 10, 2010 at 2:50 PM, Andi Kleen <andi@xxxxxxxxxxxxxx> wrote:
>> Btw, why isn't the code in __page_set_anon_rmap() also doing this
>> cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
>
> Perhaps I misunderstand the question, but __page_set_anon_rmap
> should handle Anon pages, shouldn't it?

I'm talking about this:

if (!exclusive) {
if (PageAnon(page))
return;
anon_vma = anon_vma->root;
} else {
.. big bad comment ...
if (PageAnon(page))
return;
}

where both sides of the if-statement start off doing the same thing.

It would be much cleaner to just do

... big _comprehensible_ comment ...
if (PageAnon(page))
return;

if (!exclusive)
anon_vma = anon_vma->root;

which avoids that silly else that just does something that we always do.

The reason I reacted is that Naoya-san's patch did that cleaner
version for the hugetlb case. So when I compared it to the non-hugetlb
case I just went "Ewww..."

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/