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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>