Re: [PATCH 2/2] v1 powerpc/powernv: Enable removal of memory for in memory tracing

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 05/03/2017 11:38 AM, Rashmica Gupta wrote:
> 
> 
> On 03/05/17 13:52, Rashmica Gupta wrote:
>> On 28/04/17 19:52, Anshuman Khandual wrote:
>> ....
>>>> +static int check_memblock_online(struct memory_block *mem, void *arg)
>>>> +{
>>>> +    if (mem->state != MEM_ONLINE)
>>>> +        return -1;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int change_memblock_state(struct memory_block *mem, void *arg)
>>>> +{
>>>> +    unsigned long state = (unsigned long)arg;
>>>> +
>>>> +    mem->state = state;
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static bool memtrace_offline_pages(u32 nid, u64 start_pfn, u64
>>>> nr_pages)
>>>> +{
>>>> +    u64 end_pfn = start_pfn + nr_pages - 1;
>>>> +
>>>> +    if (walk_memory_range(start_pfn, end_pfn, NULL,
>>>> +        check_memblock_online))
>>>> +        return false;
>>>> +
>>>> +    walk_memory_range(start_pfn, end_pfn, (void *)MEM_GOING_OFFLINE,
>>>> +              change_memblock_state);
>>>> +
>>> walk_memory_range() might be expensive, cant we just change the state
>>> to MEM_GOING_OFFLINE while checking the state for MEM_ONLINE during
>>> the first loop and bail out if any of the memblock is not in MEM_ONLINE
>>> in the first place.
>>
>> Good idea.
>>
> 
> This is assuming that it's more likely that the state of memory will be
> MEM_ONLINE rather than anything else (if the state isn't MEM_ONLINE we
> will still have to do a second call of walk_memory_range() to revert the
> state of any memory blocks that we changed). Seems like a reasonable
> assumption to me, thoughts?

Revert the state of memory blocks that we changed till the point we
discover that something is not MEM_ONLINE and when we decide to abort.
In that case we have to remember all the changes we have done till
that point for us to revert back. Lets keep it as it is.