Re: [PATCH v2] memory-hotplug: add sysfs zones_online_to attribute

[Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>]

(2014/08/19 16:35), Zhang Zhen wrote:
> On 2014/8/18 14:11, Yasuaki Ishimatsu wrote:
>> (2014/08/13 13:10), Zhang Zhen wrote:
>>> Currently memory-hotplug has two limits:
>>> 1. If the memory block is in ZONE_NORMAL, you can change it to
>>> ZONE_MOVABLE, but this memory block must be adjacent to ZONE_MOVABLE.
>>> 2. If the memory block is in ZONE_MOVABLE, you can change it to
>>> ZONE_NORMAL, but this memory block must be adjacent to ZONE_NORMAL.
>>>
>>> With this patch, we can easy to know a memory block can be onlined to
>>> which zone, and don't need to know the above two limits.
>>>
>>> Updated the related Documentation.
>>>
>>> Change v1 -> v2:
>>> - optimize the implementation following Dave Hansen's suggestion
>>>
>>> Signed-off-by: Zhang Zhen <zhenzhang.zhang@huawei.com>
>>> ---
>>>    Documentation/ABI/testing/sysfs-devices-memory |  8 ++++
>>>    Documentation/memory-hotplug.txt               |  4 +-
>>>    drivers/base/memory.c                          | 62 ++++++++++++++++++++++++++
>>>    include/linux/memory_hotplug.h                 |  1 +
>>>    mm/memory_hotplug.c                            |  2 +-
>>>    5 files changed, 75 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-devices-memory b/Documentation/ABI/testing/sysfs-devices-memory
>>> index 7405de2..2b2a1d7 100644
>>> --- a/Documentation/ABI/testing/sysfs-devices-memory
>>> +++ b/Documentation/ABI/testing/sysfs-devices-memory
>>> @@ -61,6 +61,14 @@ Users:        hotplug memory remove tools
>>>            http://www.ibm.com/developerworks/wikis/display/LinuxP/powerpc-utils
>>>
>>>
>>> +What:           /sys/devices/system/memory/memoryX/zones_online_to
>>> +Date:           July 2014
>>> +Contact:    Zhang Zhen <zhenzhang.zhang@huawei.com>
>>> +Description:
>>> +        The file /sys/devices/system/memory/memoryX/zones_online_to
>>> +        is read-only and is designed to show which zone this memory block can
>>> +        be onlined to.
>>> +
>>>    What:        /sys/devices/system/memoryX/nodeY
>>>    Date:        October 2009
>>>    Contact:    Linux Memory Management list <linux-mm@kvack.org>
>>> diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt
>>> index 45134dc..5b34e33 100644
>>> --- a/Documentation/memory-hotplug.txt
>>> +++ b/Documentation/memory-hotplug.txt
>>> @@ -155,6 +155,7 @@ Under each memory block, you can see 4 files:
>>>    /sys/devices/system/memory/memoryXXX/phys_device
>>>    /sys/devices/system/memory/memoryXXX/state
>>>    /sys/devices/system/memory/memoryXXX/removable
>>> +/sys/devices/system/memory/memoryXXX/zones_online_to
>>>
>>>    'phys_index'      : read-only and contains memory block id, same as XXX.
>>>    'state'           : read-write
>>> @@ -170,6 +171,8 @@ Under each memory block, you can see 4 files:
>>>                        block is removable and a value of 0 indicates that
>>>                        it is not removable. A memory block is removable only if
>>>                        every section in the block is removable.
>>> +'zones_online_to' : read-only: designed to show which zone this memory block
>>> +            can be onlined to.
>>>
>>>    NOTE:
>>>      These directories/files appear after physical memory hotplug phase.
>>> @@ -408,7 +411,6 @@ node if necessary.
>>>      - allowing memory hot-add to ZONE_MOVABLE. maybe we need some switch like
>>>        sysctl or new control file.
>>>      - showing memory block and physical device relationship.
>>> -  - showing memory block is under ZONE_MOVABLE or not
>>>      - test and make it better memory offlining.
>>>      - support HugeTLB page migration and offlining.
>>>      - memmap removing at memory offline.
>>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>>> index a2e13e2..b5d693f 100644
>>> --- a/drivers/base/memory.c
>>> +++ b/drivers/base/memory.c
>>> @@ -373,10 +373,71 @@ static ssize_t show_phys_device(struct device *dev,
>>>        return sprintf(buf, "%d\n", mem->phys_device);
>>>    }
>>>
>>> +static int __zones_online_to(unsigned long end_pfn,
>>> +                struct page *first_page, unsigned long nr_pages)
>>> +{
>>> +    struct zone *zone_next;
>>> +
>>
>>> +    /*The mem block is the last block of memory.*/
>>> +    if (!pfn_valid(end_pfn + 1))
>>> +        return 1;
>>
>> The check is not enough if memory has hole as follows:
>>
>> PFN       0x00          0xd0          0xe0          0xf0
>>              +-------------+-------------+-------------+
>> zone type   |   Normal    |     hole    |   Normal    |
>>              +-------------+-------------+-------------+
>>
>> In this case, 0xd1 is invalid pfn. But __zones_online_to should return 0
>> since 0xe0-0xf0 is Normal zone.
>>
>> Thanks,
>> Yasuaki Ishimatsu
>>
> You are right, it is not enough.
>

> Here we should make a check as follows.
> if ((end_pfn + 1) > zone_end_pfn(page_zone(first_page)))
> 	return 1;
> I will send a patch to fix it.

It's looks good to me.

Thanks,
Yasuaki Ishimatsu

>
> Thanks !
>>
>>> +    zone_next = page_zone(first_page + nr_pages);
>>> +    if (zone_idx(zone_next) == ZONE_MOVABLE)
>>> +        return 1;
>>> +    return 0;
>>> +}
>>> +
>>> +static ssize_t show_zones_online_to(struct device *dev,
>>> +                struct device_attribute *attr, char *buf)
>>> +{
>>> +    struct memory_block *mem = to_memory_block(dev);
>>> +    unsigned long start_pfn, end_pfn;
>>> +    unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
>>> +    struct page *first_page;
>>> +    struct zone *zone, *zone_prev;
>>> +
>>> +    start_pfn = section_nr_to_pfn(mem->start_section_nr);
>>> +    end_pfn = start_pfn + nr_pages;
>>> +    first_page = pfn_to_page(start_pfn);
>>> +
>>> +    /*The block contains more than one zone can not be offlined.*/
>>> +    if (!test_pages_in_a_zone(start_pfn, end_pfn))
>>> +        return sprintf(buf, "none\n");
>>> +
>>> +    zone = page_zone(first_page);
>>> +
>>> +#ifdef CONFIG_HIGHMEM
>>> +    if (zone_idx(zone) == ZONE_HIGHMEM) {
>>> +        if (__zones_online_to(end_pfn, first_page, nr_pages))
>>> +            return sprintf(buf, "%s %s\n",
>>> +                    zone->name, (zone + 1)->name);
>>> +    }
>>> +#else
>>> +    if (zone_idx(zone) == ZONE_NORMAL) {
>>> +        if (__zones_online_to(end_pfn, first_page, nr_pages))
>>> +            return sprintf(buf, "%s %s\n",
>>> +                    zone->name, (zone + 1)->name);
>>> +    }
>>> +#endif
>>> +
>>> +    if (zone_idx(zone) == ZONE_MOVABLE) {
>>> +        if (!pfn_valid(start_pfn - nr_pages))
>>> +            return sprintf(buf, "%s %s\n",
>>> +                        zone->name, (zone - 1)->name);
>>> +        zone_prev = page_zone(first_page - nr_pages);
>>> +        if (zone_idx(zone_prev) != ZONE_MOVABLE)
>>> +            return sprintf(buf, "%s %s\n",
>>> +                        zone->name, (zone - 1)->name);
>>> +    }
>>> +
>>> +    return sprintf(buf, "%s\n", zone->name);
>>> +}
>>> +
>>>    static DEVICE_ATTR(phys_index, 0444, show_mem_start_phys_index, NULL);
>>>    static DEVICE_ATTR(state, 0644, show_mem_state, store_mem_state);
>>>    static DEVICE_ATTR(phys_device, 0444, show_phys_device, NULL);
>>>    static DEVICE_ATTR(removable, 0444, show_mem_removable, NULL);
>>> +static DEVICE_ATTR(zones_online_to, 0444, show_zones_online_to, NULL);
>>>
>>>    /*
>>>     * Block size attribute stuff
>>> @@ -523,6 +584,7 @@ static struct attribute *memory_memblk_attrs[] = {
>>>        &dev_attr_state.attr,
>>>        &dev_attr_phys_device.attr,
>>>        &dev_attr_removable.attr,
>>> +    &dev_attr_zones_online_to.attr,
>>>        NULL
>>>    };
>>>
>>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>>> index d9524c4..8f1a419 100644
>>> --- a/include/linux/memory_hotplug.h
>>> +++ b/include/linux/memory_hotplug.h
>>> @@ -84,6 +84,7 @@ extern int zone_grow_waitqueues(struct zone *zone, unsigned long nr_pages);
>>>    extern int add_one_highpage(struct page *page, int pfn, int bad_ppro);
>>>    /* VM interface that may be used by firmware interface */
>>>    extern int online_pages(unsigned long, unsigned long, int);
>>> +extern int test_pages_in_a_zone(unsigned long, unsigned long);
>>>    extern void __offline_isolated_pages(unsigned long, unsigned long);
>>>
>>>    typedef void (*online_page_callback_t)(struct page *page);
>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>> index 2ff8c23..29d8693 100644
>>> --- a/mm/memory_hotplug.c
>>> +++ b/mm/memory_hotplug.c
>>> @@ -1307,7 +1307,7 @@ int is_mem_section_removable(unsigned long start_pfn, unsigned long nr_pages)
>>>    /*
>>>     * Confirm all pages in a range [start, end) is belongs to the same zone.
>>>     */
>>> -static int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn)
>>> +int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn)
>>>    {
>>>        unsigned long pfn;
>>>        struct zone *zone = NULL;
>>>
>>
>>
>> --
>> 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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>
>> .
>>
>
>