Messages in this thread | | | Date | Wed, 07 Apr 2004 15:10:11 +0900 | From | IWAMOTO Toshihiro <> | Subject | Re: [patch 1/3] memory hotplug prototype |
| |
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 - 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/
| |