Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754449Ab0LGOtx (ORCPT ); Tue, 7 Dec 2010 09:49:53 -0500 Received: from zene.cmpxchg.org ([85.214.230.12]:48412 "EHLO zene.cmpxchg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754355Ab0LGOtl (ORCPT ); Tue, 7 Dec 2010 09:49:41 -0500 Date: Tue, 7 Dec 2010 15:49:24 +0100 From: Johannes Weiner To: Minchan Kim Cc: Andrew Morton , Rik van Riel , KOSAKI Motohiro , linux-mm , LKML , Peter Zijlstra , Wu Fengguang , Nick Piggin , Mel Gorman Subject: Re: [PATCH v4 2/7] deactivate invalidated pages Message-ID: <20101207144923.GB2356@cmpxchg.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1289 Lines: 37 On Mon, Dec 06, 2010 at 02:29:10AM +0900, Minchan Kim wrote: > Changelog since v3: > - Change function comments - suggested by Johannes > - Change function name - suggested by Johannes > - add only dirty/writeback pages to deactive pagevec Why the extra check? > @@ -359,8 +360,16 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping, > if (lock_failed) > continue; > > - ret += invalidate_inode_page(page); > - > + ret = invalidate_inode_page(page); > + /* > + * If the page is dirty or under writeback, we can not > + * invalidate it now. But we assume that attempted > + * invalidation is a hint that the page is no longer > + * of interest and try to speed up its reclaim. > + */ > + if (!ret && (PageDirty(page) || PageWriteback(page))) > + deactivate_page(page); The writeback completion handler does not take the page lock, so you can still miss pages that finish writeback before this test, no? Can you explain why you felt the need to add these checks? Thanks! Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/