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@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
