Re: [PATCH] mm: mempolicy: fix the absence of the last bit of nodemask

Re: [PATCH] mm: mempolicy: fix the absence of the last bit of nodemask

[Vlastimil Babka]

On 10/14/19 11:12 AM, Michal Hocko wrote:
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index 4ae967b..a23509f 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -1328,9 +1328,11 @@ static int get_nodes(nodemask_t *nodes, const unsigned long __user *nmask,
>>  	unsigned long nlongs;
>>  	unsigned long endmask;
>>  
>> -	--maxnode;
>>  	nodes_clear(*nodes);
>> -	if (maxnode == 0 || !nmask)
>> +	/*
>> +	 * If the user specified only one node, no need to set nodemask
>> +	 */
>> +	if (maxnode - 1 == 0 || !nmask)
>>  		return 0;
>>  	if (maxnode > PAGE_SIZE*BITS_PER_BYTE)
>>  		return -EINVAL;
> 
> I am afraid this is a wrong fix. It is really hard to grasp the code but my
> understanding is that the caller is supposed to provide maxnode larger
> than than the nodemask. So if you want 2 nodes then maxnode should be 3.
> Have a look at the libnuma (which is a reference implementation)
> 
> static void setpol(int policy, struct bitmask *bmp)
> {
> 	if (set_mempolicy(policy, bmp->maskp, bmp->size + 1) < 0)
> 		numa_error("set_mempolicy");
> }
> 
> The semantic is quite awkward but it is that way for years.

Yes, unfortunately. Too late to change. We could just update the
manpages at this point.

get_mempolicy(2) says:
 maxnode specifies the number of node IDs that can be stored into
nodemask—that is, the maximum node ID plus one.

- Since node ID starts with 0, it should be actually "plus two".

set_mempolicy(2) says:
 nodemask  points to a bit mask of node IDs that contains up to maxnode
bits.

- should be also clarified.