Re: [RFC PATCH 03/14] mm/migrate: Add copy_pages_mthread function

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

On 02/23/2017 01:32 PM, Naoya Horiguchi wrote:
> On Thu, Feb 23, 2017 at 01:20:16PM +0530, Anshuman Khandual wrote:
> ...
>>>
>>>> +
>>>> +	cthreads = nr_copythreads;
>>>> +	cthreads = min_t(unsigned int, cthreads, cpumask_weight(cpumask));
>>>
>>> nitpick, but looks a little wordy, can it be simply like below?
>>>
>>>   cthreads = min_t(unsigned int, nr_copythreads, cpumask_weight(cpumask));
>>>
>>>> +	cthreads = (cthreads / 2) * 2;
>>>
>>> I'm not sure the intention here. # of threads should be even number?
>>
>> Yes.
>>
>>> If cpumask_weight() is 1, cthreads is 0, that could cause zero division.
>>> So you had better making sure to prevent it.
>>
>> If cpumask_weight() is 1, then min_t(unsigned int, 8, 1) should be
>> greater that equal to 1. Then cthreads can end up in 0. That is
>> possible. But how there is a chance of zero division ? 
> 
> Hi Anshuman,
> 
> I just thought like above when reading the line your patch introduces:
> 
>        chunk_size = PAGE_SIZE * nr_pages / cthreads
>                                            ~~~~~~~~
>                                            (this can be 0?)

Right cthreads can be 0. I am changing like this.

cthreads = min_t(unsigned int, NR_COPYTHREADS, cpumask_weight(cpumask));
cthreads = (cthreads / 2) * 2;
if (!cthreads)
      cthreads = 1;

In the first two statements cthreads can be 0 if cpumask_weight() turns
to be 1 or 0 in which case we force it to be 1. Then with this

        i = 0;
        for_each_cpu(cpu, cpumask) {
                if (i >= cthreads)
                        break;
                cpu_id_list[i] = cpu;
                ++i;
        }

cpu_id_list[0] will have the single cpu (in case cpumask of the node has
a cpu) or it will have 0 in case its memory only cpu less node. In both
cases the page copy happens in single threaded manner. This also removes
the possibility of divide by zero scenario here.

chunk_size = PAGE_SIZE * nr_pages / cthreads;

--
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>