From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Wed, 23 Apr 2008 11:41:07 +0900 From: KAMEZAWA Hiroyuki Subject: Re: Warning on memory offline (and possible in usual migration?) Message-Id: <20080423114107.b8df779c.kamezawa.hiroyu@jp.fujitsu.com> In-Reply-To: <20080423004804.GA14134@wotan.suse.de> References: <20080414145806.c921c927.kamezawa.hiroyu@jp.fujitsu.com> <20080422045205.GH21993@wotan.suse.de> <20080422165608.7ab7026b.kamezawa.hiroyu@jp.fujitsu.com> <20080422094352.GB23770@wotan.suse.de> <20080423004804.GA14134@wotan.suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: Nick Piggin Cc: Christoph Lameter , "linux-mm@kvack.org" , Andrew Morton , GOTO List-ID: On Wed, 23 Apr 2008 02:48:04 +0200 Nick Piggin wrote: > KAMEZAWA Hiroyuki found a warning message in the buffer dirtying code that > is coming from page migration caller. > > WARNING: at fs/buffer.c:720 __set_page_dirty+0x330/0x360() > Call Trace: > [] show_stack+0x80/0xa0 > [] dump_stack+0x30/0x60 > [] warn_on_slowpath+0x90/0xe0 > [] __set_page_dirty+0x330/0x360 > [] __set_page_dirty_buffers+0xd0/0x280 > [] set_page_dirty+0xc0/0x260 > [] migrate_page_copy+0x5d0/0x5e0 > [] buffer_migrate_page+0x2e0/0x3c0 > [] migrate_pages+0x770/0xe00 > > What was happening is that migrate_page_copy wants to transfer the PG_dirty > bit from old page to new page, so what it would do is set_page_dirty(newpage). > However set_page_dirty() is used to set the entire page dirty, wheras in > this case, only part of the page was dirty, and it also was not uptodate. > > Marking the whole page dirty with set_page_dirty would lead to corruption or > unresolvable conditions -- a dirty && !uptodate page and dirty && !uptodate > buffers. > > Possibly we could just ClearPageDirty(oldpage); SetPageDirty(newpage); > however in the interests of keeping the change minimal... > > Signed-off-by: Nick Piggin Tested and seems to work well. thank you! BTW, can I ask a question for understanding this change ? ==this check== WARN_ON_ONCE(!PagePrivate(page) && !PageUptodate(page)); in __set_page_dirty_nobuffers() seems to check "the page should have buffer or be up-to-date when it calls this function." When it comes to __set_page_dirty() (in fs/buffer.c) == this check== WARN_ON_ONCE(warn && !PageUptodate(page)); is used and doesn't see page has buffers or not. What's difference between two functions's condition for WARNING ? -Kame -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org