From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <44D7E584.10109@yahoo.com.au> Date: Tue, 08 Aug 2006 11:14:44 +1000 From: Nick Piggin MIME-Version: 1.0 Subject: Re: [patch][rfc] possible lock_page fix for Andrea's nopage vs invalidate race? References: <44CF3CB7.7030009@yahoo.com.au> <44D74B98.3030305@yahoo.com.au> In-Reply-To: Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: Hugh Dickins Cc: Andrea Arcangeli , Andrew Morton , David Howells , Linux Memory Management List-ID: Hugh Dickins wrote: > On Tue, 8 Aug 2006, Nick Piggin wrote: > >>Hugh Dickins wrote: >> >>>Hmmm, page_mkwrite when called from do_wp_page would not expect to >>>be holding page lock: we don't want it called with in one case and >>>without in the other. Maybe do_no_page needs to unlock_page before >>>calling page_mkwrite, lock_page after, and check page->mapping when >>>VM_NOPAGE_LOCKED?? >> >>That's pretty foul. I'll take a bit of a look. Is it really a problem >>to call in either state, if it is well documented? (we could even >>send a flag down if needed). I thought filesystem code loved this >>kind of spaghetti locking? > > > Agreed foul. David's helpful mail reassures not an immediate problem, > but I'm pretty sure other future uses of page_mkwrite would need to > know if the page is held locked or not. Yes, could be done by a flag, > though that's not pretty (gives the ->page_mkwrite implementation much > the same schizophrenia as I was disliking here in do_no_page). But it would be better to do it in the theoretical page_mkwrite that cares, maybe? Imagine one that did want to have the page locked. Well I'll leave this issue alone for the next iteration. > > >>I don't think ->populate has ever particularly troubled itself with >>these kinds of theoretical races. I was really hoping to fix linear >>pagecache first before getting bogged down with nonlinear. > > > install_page has had mapping & i_size check for quite a while, but > perhaps by theoretical races you mean Andrea's invalidate case. > The nonlinear case is much less a concern than MAP_POPULATE > (though I don't know if anyone really uses that). Sure but it doesn't do any truncate_count checking. So nothing in my patch will make it worse than it already is. > > >>After thinking about it a bit more, I think I've found my filemap_nopage >>wanting. Suppose i_size is shrunk and the page truncated before the >>first find_lock_page. OK, no we'll allocate a new page, add it to the >>pagecache, and do a ->readpage(). > > > I've got a bit lost between merges against different trees, > I'll let you sort that one out. I wonder if we should have the i_size check (under the page lock) in do_no_page or down in the ->nopage implementations? -- SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com -- 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