Hi Wen, 2012/07/04 14:08, Wen Congyang wrote: > At 07/04/2012 12:45 PM, Yasuaki Ishimatsu Wrote: >> Hi Wen, >> >> 2012/07/03 15:35, Wen Congyang wrote: >>> At 07/03/2012 01:56 PM, Yasuaki Ishimatsu Wrote: >>>> When (hot)adding memory into system, /sys/firmware/memmap/X/{end, start, type} >>>> sysfs files are created. But there is no code to remove these files. The patch >>>> implements the function to remove them. >>>> >>>> Note : The code does not free firmware_map_entry since there is no way to free >>>> memory which is allocated by bootmem. >>>> >>>> CC: David Rientjes <rientjes@xxxxxxxxxx> >>>> CC: Jiang Liu <liuj97@xxxxxxxxx> >>>> CC: Len Brown <len.brown@xxxxxxxxx> >>>> CC: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> >>>> CC: Paul Mackerras <paulus@xxxxxxxxx> >>>> CC: Christoph Lameter <cl@xxxxxxxxx> >>>> Cc: Minchan Kim <minchan.kim@xxxxxxxxx> >>>> CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >>>> CC: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx> >>>> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@xxxxxxxxxxxxxx> >>>> >>>> --- >>>> drivers/firmware/memmap.c | 70 +++++++++++++++++++++++++++++++++++++++++++ >>>> include/linux/firmware-map.h | 6 +++ >>>> mm/memory_hotplug.c | 6 +++ >>>> 3 files changed, 81 insertions(+), 1 deletion(-) >>>> >>>> Index: linux-3.5-rc4/mm/memory_hotplug.c >>>> =================================================================== >>>> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-07-03 14:22:00.190240794 +0900 >>>> +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-07-03 14:22:03.549198802 +0900 >>>> @@ -661,7 +661,11 @@ EXPORT_SYMBOL_GPL(add_memory); >>>> >>>> int remove_memory(int nid, u64 start, u64 size) >>>> { >>>> - return -EBUSY; >>>> + lock_memory_hotplug(); >>>> + /* remove memmap entry */ >>>> + firmware_map_remove(start, start + size - 1, "System RAM"); >>>> + unlock_memory_hotplug(); >>>> + return 0; >>>> >>>> } >>>> EXPORT_SYMBOL_GPL(remove_memory); >>>> Index: linux-3.5-rc4/include/linux/firmware-map.h >>>> =================================================================== >>>> --- linux-3.5-rc4.orig/include/linux/firmware-map.h 2012-07-03 14:21:45.766421116 +0900 >>>> +++ linux-3.5-rc4/include/linux/firmware-map.h 2012-07-03 14:22:03.550198789 +0900 >>>> @@ -25,6 +25,7 @@ >>>> >>>> int firmware_map_add_early(u64 start, u64 end, const char *type); >>>> int firmware_map_add_hotplug(u64 start, u64 end, const char *type); >>>> +int firmware_map_remove(u64 start, u64 end, const char *type); >>>> >>>> #else /* CONFIG_FIRMWARE_MEMMAP */ >>>> >>>> @@ -38,6 +39,11 @@ static inline int firmware_map_add_hotpl >>>> return 0; >>>> } >>>> >>>> +static inline int firmware_map_remove(u64 start, u64 end, const char *type) >>>> +{ >>>> + return 0; >>>> +} >>>> + >>>> #endif /* CONFIG_FIRMWARE_MEMMAP */ >>>> >>>> #endif /* _LINUX_FIRMWARE_MAP_H */ >>>> Index: linux-3.5-rc4/drivers/firmware/memmap.c >>>> =================================================================== >>>> --- linux-3.5-rc4.orig/drivers/firmware/memmap.c 2012-07-03 14:21:45.761421180 +0900 >>>> +++ linux-3.5-rc4/drivers/firmware/memmap.c 2012-07-03 14:22:03.569198549 +0900 >>>> @@ -79,7 +79,16 @@ static const struct sysfs_ops memmap_att >>>> .show = memmap_attr_show, >>>> }; >>>> >>>> +static void release_firmware_map_entry(struct kobject *kobj) >>>> +{ >>>> + /* >>>> + * FIXME : There is no idea. >>>> + * How to free the entry which allocated bootmem? >>>> + */ >>> >>> I find a function free_bootmem(), but I am not sure whether it can work here. >> >> It cannot work here. >> >>> Another problem: how to check whether the entry uses bootmem? >> >> When firmware_map_entry is allocated by kzalloc(), the page has PG_slab. > > This is not true. In my test, I find the page does not have PG_slab sometimes. I think that it depends on the allocated size. firmware_map_entry size is smaller than PAGE_SIZE. So the page has PG_Slab. Thanks, Yasuaki Ishimatsu > > Thanks > Wen Congyang. > >> So we can check whether the entry was allocated by bootmem or not. >> If the eantry was allocated by kzalloc(), we can free the entry by kfree(). >> But if the entry was allocated by bootmem, we have no way to free the entry. >> >> Thanks, >> Yasuaki Ishimatsu >> >>> >>> Thanks >>> Wen Congyang >>> >>>> +} >>>> + >>>> static struct kobj_type memmap_ktype = { >>>> + .release = release_firmware_map_entry, >>>> .sysfs_ops = &memmap_attr_ops, >>>> .default_attrs = def_attrs, >>>> }; >>>> @@ -123,6 +132,16 @@ static int firmware_map_add_entry(u64 st >>>> return 0; >>>> } >>>> >>>> +/** >>>> + * firmware_map_remove_entry() - Does the real work to remove a firmware >>>> + * memmap entry. >>>> + * @entry: removed entry. >>>> + **/ >>>> +static inline void firmware_map_remove_entry(struct firmware_map_entry *entry) >>>> +{ >>>> + list_del(&entry->list); >>>> +} >>>> + >>>> /* >>>> * Add memmap entry on sysfs >>>> */ >>>> @@ -144,6 +163,31 @@ static int add_sysfs_fw_map_entry(struct >>>> return 0; >>>> } >>>> >>>> +/* >>>> + * Remove memmap entry on sysfs >>>> + */ >>>> +static inline void remove_sysfs_fw_map_entry(struct firmware_map_entry *entry) >>>> +{ >>>> + kobject_put(&entry->kobj); >>>> +} >>>> + >>>> +/* >>>> + * Search memmap entry >>>> + */ >>>> + >>>> +struct firmware_map_entry * __meminit >>>> +find_firmware_map_entry(u64 start, u64 end, const char *type) >>>> +{ >>>> + struct firmware_map_entry *entry; >>>> + >>>> + list_for_each_entry(entry, &map_entries, list) >>>> + if ((entry->start == start) && (entry->end == end) && >>>> + (!strcmp(entry->type, type))) >>>> + return entry; >>>> + >>>> + return NULL; >>>> +} >>>> + >>>> /** >>>> * firmware_map_add_hotplug() - Adds a firmware mapping entry when we do >>>> * memory hotplug. >>>> @@ -196,6 +240,32 @@ int __init firmware_map_add_early(u64 st >>>> return firmware_map_add_entry(start, end, type, entry); >>>> } >>>> >>>> +/** >>>> + * firmware_map_remove() - remove a firmware mapping entry >>>> + * @start: Start of the memory range. >>>> + * @end: End of the memory range (inclusive). >>>> + * @type: Type of the memory range. >>>> + * >>>> + * removes a firmware mapping entry. >>>> + * >>>> + * Returns 0 on success, or -EINVAL if no entry. >>>> + **/ >>>> +int __meminit firmware_map_remove(u64 start, u64 end, const char *type) >>>> +{ >>>> + struct firmware_map_entry *entry; >>>> + >>>> + entry = find_firmware_map_entry(start, end, type); >>>> + if (!entry) >>>> + return -EINVAL; >>>> + >>>> + /* remove the memmap entry */ >>>> + remove_sysfs_fw_map_entry(entry); >>>> + >>>> + firmware_map_remove_entry(entry); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> /* >>>> * Sysfs functions ------------------------------------------------------------- >>>> */ >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> Please read the FAQ at http://www.tux.org/lkml/ >>>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ >>> >> >> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html