From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933377AbaFIXlk (ORCPT ); Mon, 9 Jun 2014 19:41:40 -0400 Received: from mail-ig0-f175.google.com ([209.85.213.175]:56084 "EHLO mail-ig0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933147AbaFIXlj (ORCPT ); Mon, 9 Jun 2014 19:41:39 -0400 Date: Mon, 9 Jun 2014 16:41:36 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Vlastimil Babka cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Greg Thelen , Minchan Kim , Mel Gorman , Joonsoo Kim , Michal Nazarewicz , Naoya Horiguchi , Christoph Lameter , Rik van Riel Subject: Re: [PATCH 01/10] mm, compaction: do not recheck suitable_migration_target under lock In-Reply-To: <1402305982-6928-1-git-send-email-vbabka@suse.cz> Message-ID: References: <1402305982-6928-1-git-send-email-vbabka@suse.cz> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 9 Jun 2014, Vlastimil Babka wrote: > isolate_freepages_block() rechecks if the pageblock is suitable to be a target > for migration after it has taken the zone->lock. However, the check has been > optimized to occur only once per pageblock, and compact_checklock_irqsave() > might be dropping and reacquiring lock, which means somebody else might have > changed the pageblock's migratetype meanwhile. > > Furthermore, nothing prevents the migratetype to change right after > isolate_freepages_block() has finished isolating. Given how imperfect this is, > it's simpler to just rely on the check done in isolate_freepages() without > lock, and not pretend that the recheck under lock guarantees anything. It is > just a heuristic after all. > > Signed-off-by: Vlastimil Babka > Cc: Minchan Kim > Cc: Mel Gorman > Cc: Joonsoo Kim > Cc: Michal Nazarewicz > Cc: Naoya Horiguchi > Cc: Christoph Lameter > Cc: Rik van Riel > Cc: David Rientjes Acked-by: David Rientjes We already do a preliminary check for suitable_migration_target() in isolate_freepages() in a racy way to avoid unnecessary work (and page_order() there is unprotected, I think you already mentioned this) so this seems fine to abandon. > --- > I suggest folding mm-compactionc-isolate_freepages_block-small-tuneup.patch into this > Hmm, Andrew was just moving some code around in that patch, I'm not sure that it makes sense to couple these two together and your patch here is addressing an optimization rather than a cleanup (and you've documented it well, no need to obscure it with unrelated changes). From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ig0-f174.google.com (mail-ig0-f174.google.com [209.85.213.174]) by kanga.kvack.org (Postfix) with ESMTP id 145136B00C6 for ; Mon, 9 Jun 2014 19:41:40 -0400 (EDT) Received: by mail-ig0-f174.google.com with SMTP id h3so4480634igd.1 for ; Mon, 09 Jun 2014 16:41:39 -0700 (PDT) Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [2607:f8b0:4001:c03::234]) by mx.google.com with ESMTPS id dq1si9688519icb.23.2014.06.09.16.41.39 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 09 Jun 2014 16:41:39 -0700 (PDT) Received: by mail-ie0-f180.google.com with SMTP id at20so6275653iec.39 for ; Mon, 09 Jun 2014 16:41:39 -0700 (PDT) Date: Mon, 9 Jun 2014 16:41:36 -0700 (PDT) From: David Rientjes Subject: Re: [PATCH 01/10] mm, compaction: do not recheck suitable_migration_target under lock In-Reply-To: <1402305982-6928-1-git-send-email-vbabka@suse.cz> Message-ID: References: <1402305982-6928-1-git-send-email-vbabka@suse.cz> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: Vlastimil Babka Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Greg Thelen , Minchan Kim , Mel Gorman , Joonsoo Kim , Michal Nazarewicz , Naoya Horiguchi , Christoph Lameter , Rik van Riel On Mon, 9 Jun 2014, Vlastimil Babka wrote: > isolate_freepages_block() rechecks if the pageblock is suitable to be a target > for migration after it has taken the zone->lock. However, the check has been > optimized to occur only once per pageblock, and compact_checklock_irqsave() > might be dropping and reacquiring lock, which means somebody else might have > changed the pageblock's migratetype meanwhile. > > Furthermore, nothing prevents the migratetype to change right after > isolate_freepages_block() has finished isolating. Given how imperfect this is, > it's simpler to just rely on the check done in isolate_freepages() without > lock, and not pretend that the recheck under lock guarantees anything. It is > just a heuristic after all. > > Signed-off-by: Vlastimil Babka > Cc: Minchan Kim > Cc: Mel Gorman > Cc: Joonsoo Kim > Cc: Michal Nazarewicz > Cc: Naoya Horiguchi > Cc: Christoph Lameter > Cc: Rik van Riel > Cc: David Rientjes Acked-by: David Rientjes We already do a preliminary check for suitable_migration_target() in isolate_freepages() in a racy way to avoid unnecessary work (and page_order() there is unprotected, I think you already mentioned this) so this seems fine to abandon. > --- > I suggest folding mm-compactionc-isolate_freepages_block-small-tuneup.patch into this > Hmm, Andrew was just moving some code around in that patch, I'm not sure that it makes sense to couple these two together and your patch here is addressing an optimization rather than a cleanup (and you've documented it well, no need to obscure it with unrelated changes). -- 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