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

[Balbir Singh <bsingharora@gmail.com>]

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@linux.vnet.ibm.com>
> ---
>  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@kvack.org>
> +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@kvack.org>
> 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

>  	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

Balbir