On Wed, Dec 18, 2013 at 05:04:29PM -0800, Andrew Morton wrote: > On Thu, 19 Dec 2013 09:58:05 +0900 Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> wrote: > > > On Wed, Dec 18, 2013 at 04:28:58PM -0800, Andrew Morton wrote: > > > On Thu, 19 Dec 2013 08:16:35 +0800 Wanpeng Li <liwanp@xxxxxxxxxxxxxxxxxx> wrote: > > > > > > > page_get_anon_vma() called in page_referenced_anon() will lock and > > > > increase the refcount of anon_vma, page won't be locked for anonymous > > > > page. This patch fix it by skip check anonymous page locked. > > > > > > > > [ 588.698828] kernel BUG at mm/rmap.c:1663! > > > > > > Why is all this suddenly happening. Did we change something, or did a > > > new test get added to trinity? > > > > It is my fault. > > I should remove this VM_BUG_ON() since rmap_walk() can be called > > without holding PageLock() in this case. > > > > I think that adding VM_BUG_ON() to each rmap_walk calllers is better > > than this patch, because, now, rmap_walk() is called by many places and > > each places has different contexts. > > I don't think that putting the assertion into the caller makes a lot of > sense, particularly if that code just did a lock_page()! If a *callee* > needs PageLocked() then that callee should assert that the page is > locked. So > > VM_BUG_ON(!PageLocked(page)); > > means "this code requires that the page be locked". And if that code > requires PageLocked(), there must be reasons for this. Let's also > include an explanation of those reasons. Yes, if this condition is invariant for rmap_walk(), we should put this on rmap_walk(). But if not, we should put this on the other place. I will investigate more and send good solution :) Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>