From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S263188AbUDGGLA (ORCPT ); Wed, 7 Apr 2004 02:11:00 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S264105AbUDGGLA (ORCPT ); Wed, 7 Apr 2004 02:11:00 -0400 Received: from sv1.valinux.co.jp ([210.128.90.2]:2491 "EHLO sv1.valinux.co.jp") by vger.kernel.org with ESMTP id S263188AbUDGGKP (ORCPT ); Wed, 7 Apr 2004 02:10:15 -0400 Date: Wed, 07 Apr 2004 15:10:11 +0900 From: IWAMOTO Toshihiro To: Dave Hansen Cc: IWAMOTO Toshihiro , Linux Kernel Mailing List , lhms Subject: Re: [patch 1/3] memory hotplug prototype In-Reply-To: <1081271577.32423.193.camel@nighthawk> References: <20040406105353.9BDE8705DE@sv1.valinux.co.jp> <20040406105649.77F36705DE@sv1.valinux.co.jp> <1081271577.32423.193.camel@nighthawk> User-Agent: Wanderlust/2.8.1 (Something) SEMI/1.14.3 (Ushinoya) FLIM/1.14.3 (=?ISO-8859-4?Q?Unebigory=F2mae?=) APEL/10.3 Emacs/21.2 (i386-debian-linux-gnu) MULE/5.0 (SAKAKI) MIME-Version: 1.0 (generated by SEMI 1.14.3 - "Ushinoya") Content-Type: text/plain; charset=US-ASCII Message-Id: <20040407061011.2BBFA70692@sv1.valinux.co.jp> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org At Tue, 06 Apr 2004 10:12:58 -0700, Dave Hansen wrote: > > -/* Zone modifiers in GFP_ZONEMASK (see linux/mmzone.h - low two bits) */ > > -#define __GFP_DMA 0x01 > > -#define __GFP_HIGHMEM 0x02 > > +/* Zone modifiers in GFP_ZONEMASK (see linux/mmzone.h - low three bits) */ > > +#define __GFP_DMA 0x01 > > +#define __GFP_HIGHMEM 0x02 > > +#define __GFP_HOTREMOVABLE 0x03 > > Are you still determined to add a zone like this? Hotplug will > eventually have to be done on all of the zones (NORMAL, DMA, etc...), so > it seems a bit shortsighted to add a zone like this. I think it would Hotremoval things are a bit confusing. There are __GFP_HOTREMOVABLE macro for alloc_page and node_zonelists[ZONE_HOTREMOVABLE], but there's no node_zones[ZONE_HOTREMOVABLE]. Hotremovable attribute of a zone is stored in its pgdat. And every zone can be hotremovable. > > -#define try_to_unmap(page) SWAP_FAIL > > +#define try_to_unmap(page, force) SWAP_FAIL > > #endif /* CONFIG_MMU * > Could you explain what you're trying to do with try_to_unmap() and why > all of the calls need to be changed? This is necessary for handling mlocked pages. > > - int error = radix_tree_preload(gfp_mask & ~__GFP_HIGHMEM); > > + int error = radix_tree_preload((gfp_mask & ~GFP_ZONEMASK) | > > + ((gfp_mask & GFP_ZONEMASK) == __GFP_DMA ? __GFP_DMA : 0)); > > What is this doing? Trying to filter off the highmem flag without > affecting the hotremove flag??? Because __GFP_HOTREMOVABLE & ~ __GFP_HIGHMEM evaluates to __GFP_DMA. > > /* > > * Builds allocation fallback zone lists. > > */ > > -static int __init build_zonelists_node(pg_data_t *pgdat, struct zonelist *zonelist, int j, int k) > > +static int build_zonelists_node(pg_data_t *pgdat, struct zonelist *zonelist, int j, int k) > > { > > + > > + if (! pgdat->enabled) > > + return j; > > Normal Linux style is: > > + if (!pgdat->enabled) > > + return j; Ok. > > + if (k != ZONE_HOTREMOVABLE && > > + pgdat->removable) > > + return j; > > What is this check supposed to do? This code was used when build_zonelists_node was called from build_zonelists even if CONFIG_MEMHOTPLUG was defined. So, this diff is no longer necessary. > > - memset(zonelist, 0, sizeof(*zonelist)); > > + /* memset(zonelist, 0, sizeof(*zonelist)); */ > > Why is this memset unnecessary now? This diff is also no longer necessary, but unused tail elements of zonelist is zeroed in build_zonelists anyway. -- IWAMOTO Toshihiro