Re: [RFC 8/8] mm: Add N_COHERENT_DEVICE node type into node_states[]

[Anshuman Khandual]

On 10/25/2016 12:52 PM, Balbir Singh wrote:
> 
> 
> On 24/10/16 15:31, Anshuman Khandual wrote:
>> Add a new member N_COHERENT_DEVICE into node_states[] nodemask array to
>> enlist all those nodes which contain only coherent device memory. Also
>> creates a new sysfs interface /sys/devices/system/node/is_coherent_device
>> to list down all those nodes which has coherent device memory.
>>
>> Signed-off-by: Anshuman Khandual <khandual@xxxxxxxxxxxxxxxxxx>
>> ---
>>  Documentation/ABI/stable/sysfs-devices-node |  7 +++++++
>>  drivers/base/node.c                         |  6 ++++++
>>  include/linux/nodemask.h                    |  3 +++
>>  mm/memory_hotplug.c                         | 10 ++++++++++
>>  4 files changed, 26 insertions(+)
>>
>> diff --git a/Documentation/ABI/stable/sysfs-devices-node b/Documentation/ABI/stable/sysfs-devices-node
>> index 5b2d0f0..5538791 100644
>> --- a/Documentation/ABI/stable/sysfs-devices-node
>> +++ b/Documentation/ABI/stable/sysfs-devices-node
>> @@ -29,6 +29,13 @@ Description:
>>  		Nodes that have regular or high memory.
>>  		Depends on CONFIG_HIGHMEM.
>>  
>> +What:		/sys/devices/system/node/is_coherent_device
>> +Date:		October 2016
>> +Contact:	Linux Memory Management list <linux-mm@xxxxxxxxx>
>> +Description:
>> +		Lists the nodemask of nodes that have coherent memory.
>> +		Depends on CONFIG_COHERENT_DEVICE.
>> +
>>  What:		/sys/devices/system/node/nodeX
>>  Date:		October 2002
>>  Contact:	Linux Memory Management list <linux-mm@xxxxxxxxx>
>> diff --git a/drivers/base/node.c b/drivers/base/node.c
>> index 5548f96..5b5dd89 100644
>> --- a/drivers/base/node.c
>> +++ b/drivers/base/node.c
>> @@ -661,6 +661,9 @@ static struct node_attr node_state_attr[] = {
>>  	[N_MEMORY] = _NODE_ATTR(has_memory, N_MEMORY),
>>  #endif
>>  	[N_CPU] = _NODE_ATTR(has_cpu, N_CPU),
>> +#ifdef CONFIG_COHERENT_DEVICE
>> +	[N_COHERENT_DEVICE] = _NODE_ATTR(is_coherent_device, N_COHERENT_DEVICE),
>> +#endif
>>  };
>>  
>>  static struct attribute *node_state_attrs[] = {
>> @@ -674,6 +677,9 @@ static struct attribute *node_state_attrs[] = {
>>  	&node_state_attr[N_MEMORY].attr.attr,
>>  #endif
>>  	&node_state_attr[N_CPU].attr.attr,
>> +#ifdef CONFIG_COHERENT_DEVICE
>> +	&node_state_attr[N_COHERENT_DEVICE].attr.attr,
>> +#endif
>>  	NULL
>>  };
>>  
>> diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
>> index f746e44..605cb0d 100644
>> --- a/include/linux/nodemask.h
>> +++ b/include/linux/nodemask.h
>> @@ -393,6 +393,9 @@ enum node_states {
>>  	N_MEMORY = N_HIGH_MEMORY,
>>  #endif
>>  	N_CPU,		/* The node has one or more cpus */
>> +#ifdef CONFIG_COHERENT_DEVICE
>> +	N_COHERENT_DEVICE,	/* The node has coherent device memory */
>> +#endif
>>  	NR_NODE_STATES
>>  };
>>  
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 9629273..8f03962 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1044,6 +1044,11 @@ static void node_states_set_node(int node, struct memory_notify *arg)
>>  	if (arg->status_change_nid_high >= 0)
>>  		node_set_state(node, N_HIGH_MEMORY);
>>  
>> +#ifdef CONFIG_COHERENT_DEVICE
>> +	if (isolated_cdm_node(node))
>> +		node_set_state(node, N_COHERENT_DEVICE);
>> +#endif
>> +
> 
> #ifdef not required, see below
> 

Right, will change.

>>  	node_set_state(node, N_MEMORY);
>>  }
>>  
>> @@ -1858,6 +1863,11 @@ static void node_states_clear_node(int node, struct memory_notify *arg)
>>  	if ((N_MEMORY != N_HIGH_MEMORY) &&
>>  	    (arg->status_change_nid >= 0))
>>  		node_clear_state(node, N_MEMORY);
>> +
>> +#ifdef CONFIG_COHERENT_DEVICE
>> +	if (isolated_cdm_node(node))
>> +		node_clear_state(node, N_COHERENT_DEVICE);
>> +#endif
>>  }
>>  
> 
> I think the #ifdefs are not needed if isolated_cdm_node
> is defined for both with and without CONFIG_COHERENT_DEVICE.
> 
> I think this patch needs to move up in the series so that
> node state can be examined by other core algorithms

Okay, will move up.