From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Wed, 28 Jun 2006 12:58:54 -0700 From: Andrew Morton Subject: Re: [PATCH 1/5] mm: tracking shared dirty pages Message-Id: <20060628125854.b4e390d5.akpm@osdl.org> In-Reply-To: References: <20060627182801.20891.11456.sendpatchset@lappy> <20060627182814.20891.36856.sendpatchset@lappy> <20060627175747.521c6733.akpm@osdl.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: Hugh Dickins Cc: a.p.zijlstra@chello.nl, linux-mm@kvack.org, linux-kernel@vger.kernel.org, dhowells@redhat.com, christoph@lameter.com, mbligh@google.com, npiggin@suse.de, torvalds@osdl.org List-ID: On Wed, 28 Jun 2006 20:49:42 +0100 (BST) Hugh Dickins wrote: > On Tue, 27 Jun 2006, Andrew Morton wrote: > > Peter Zijlstra wrote: > > > > > > Tracking of dirty pages in shared writeable mmap()s. > > > > I mangled this a bit to fit it on top of Christoph's vm counters rewrite > > (mm/page-writeback.c). > > > > I worry about the changes to __set_page_dirty_nobuffers() and > > test_clear_page_dirty(). > > > > They both already require that the page be locked (or that the > > address_space be otherwise pinned). But I'm not sure we get that right at > > present. With these changes, our exposure to that gets worse, and we > > additionally are exposed to the possibility of the page itself being > > reclaimed, and not just the address_space. > > > > So ho hum. I'll stick this: > > > > --- a/mm/page-writeback.c~mm-tracking-shared-dirty-pages-checks > > +++ a/mm/page-writeback.c > > @@ -625,6 +625,7 @@ EXPORT_SYMBOL(write_one_page); > > */ > > int __set_page_dirty_nobuffers(struct page *page) > > { > > + WARN_ON_ONCE(!PageLocked(page)); > > I expect this warning to fire: __set_page_dirty_nobuffers is very > careful about page->mapping, it knows it might change precisely > because we often don't have PageLocked here. Yes, that's misplaced. It was a consequence of me incorrectly fixing a reject storm. > > if (!TestSetPageDirty(page)) { > > struct address_space *mapping = page_mapping(page); > > struct address_space *mapping2; > > @@ -722,6 +723,7 @@ int test_clear_page_dirty(struct page *p > > struct address_space *mapping = page_mapping(page); > > unsigned long flags; > > > > + WARN_ON_ONCE(!PageLocked(page)); > > I don't expect this warning to fire: I checked a month or two ago, > and just checked again, I believe all paths have PageLocked here. > If not, we certainly do want to know about it. I've fixed it all up now and all is quiet. Looks OK. -- 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