useless node/has_cpu sysfs attribute

useless node/has_cpu sysfs attribute

[Andreas Herrmann]

Hi,

commit 99dcc3e5a94ed491fbef402831d8c0bbb267f995 (this_cpu: Page
allocator conversion) removed the call

    node_set_state(node, N_CPU);    /* this node has a cpu */

(in fact process_zones() where this call happened was removed).

Thus the sysfs attribute /sys/devices/system/node/has_cpu doesn't
show anything anymore.

Before this commit the attribute usually listed all nodes ("having a CPU")
in the system, e.g.

  worms linux # cat /sys/devices/system/node/has_cpu
  0-7

This attribute never reflected any changes triggered by CPU hotplug.
I.e. you could set all CPUs of a node offline but the node still was listed
in this attribute. (because no hotplug notifiers exist that would
call node_set_state(node, N_CPU)/node_clear_state(node,N_CPU)).

Do we need this attribute?
Who relies on information from this attribute?

I think if we do not need the attribute we should remove it. But if
there is use for it we need to fix it and make it CPU hotplug aware.

Opinions?


Thanks,

Andreas

PS: The attribute was introduced with commit
bde631a51876f23e9bbdce43f02b7232502c151e (mm: add node states sysfs
class attributeS)

Re: useless node/has_cpu sysfs attribute

[Christoph Lameter]

On Thu, 15 Apr 2010, Andreas Herrmann wrote:

> commit 99dcc3e5a94ed491fbef402831d8c0bbb267f995 (this_cpu: Page
> allocator conversion) removed the call
>
>     node_set_state(node, N_CPU);    /* this node has a cpu */

This was moved to vmstat_cpuup_callback. See mm/vmstat.c. Maybe it needs
to be  added to setup_vmstat() as well.

> Thus the sysfs attribute /sys/devices/system/node/has_cpu doesn't
> show anything anymore.

Thats strange. Why is the cpuup notifier not executing?

> This attribute never reflected any changes triggered by CPU hotplug.

Duh some breakage in the cpuup notifier handling?

This needs to be fixed.

Re: useless node/has_cpu sysfs attribute

[Andreas Herrmann]

On Thu, Apr 15, 2010 at 07:27:42PM -0500, Christoph Lameter wrote:
> On Thu, 15 Apr 2010, Andreas Herrmann wrote:
> 
> > commit 99dcc3e5a94ed491fbef402831d8c0bbb267f995 (this_cpu: Page
> > allocator conversion) removed the call
> >
> >     node_set_state(node, N_CPU);    /* this node has a cpu */

> This was moved to vmstat_cpuup_callback. See mm/vmstat.c. Maybe it needs
> to be  added to setup_vmstat() as well.

Ok.
I missed that one.

> > Thus the sysfs attribute /sys/devices/system/node/has_cpu doesn't
> > show anything anymore.
> 
> Thats strange. Why is the cpuup notifier not executing?

I'll try to figure this out.

> > This attribute never reflected any changes triggered by CPU hotplug.
> 
> Duh some breakage in the cpuup notifier handling?
> 
> This needs to be fixed.

Why don't we try to call node_clear_state(node, N_CPU)
anywhere? (Maybe there is such a call and I missed it.)

Do we want to list each node with this attribute that has a CPU
regardless whether the CPU is actually offline or online?

That is the reason why I wanted to know who is relying on this
attribute and what are the use cases of it.


Thanks,
Andreas

Re: useless node/has_cpu sysfs attribute

[Christoph Lameter]

On Fri, 16 Apr 2010, Andreas Herrmann wrote:

> Why don't we try to call node_clear_state(node, N_CPU)
> anywhere? (Maybe there is such a call and I missed it.)

There is none since it would require a scan of all processors to see if a
certain node is still used. Seems that this was not implemented.

> Do we want to list each node with this attribute that has a CPU
> regardless whether the CPU is actually offline or online?

Nope.

> That is the reason why I wanted to know who is relying on this
> attribute and what are the use cases of it.

Obviously not many right now since the breakage went unnoticed. There is
certainly going to be more usage with the high end processors requiring
NUMA. Would be glad if you could clear up the situation, fix breakage and
implement missing functionality.