lkml.org 
[lkml]   [2016]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] memory-hotplug: fix store_mem_state() return value
On Thu, Sep 01, 2016 at 01:37:17PM -0700, Andrew Morton wrote:
>What the heck are the return value semantics of bus_type.online?
>Sometimes 0, sometimes 1 and apparently sometimes -Efoo values. What
>are these things trying to tell the caller and why is "1" ever useful
>and why doesn't anyone document anything. grr.

You might be getting tangled in the two codepaths the way I was.

If you do 'echo 1 > online':
dev_attr_store
online_store
device_online
memory_subsys_online
memory_block_change_state

If you do 'echo online > state':
dev_attr_store
store_mem_state
device_online
memory_subsys_online
memory_block_change_state

>static int memory_subsys_online(struct device *dev)
>{
> struct memory_block *mem = to_memory_block(dev);
> int ret;
>
> if (mem->state == MEM_ONLINE)
> return 0;
>
>Doesn't that "return 0" contradict the changelog?

The online-to-online check being used is higher in the call chain:

int device_online(struct device *dev)
{
if (device_supports_offline(dev)) {
if (dev->offline) {
...
} else {
ret = 1;
}
}

>Also, is store_mem_state() the correct place to fix this? Instead,
>should memory_block_change_state() detect an attempt to online
>already-online memory and itself return -EINVAL, and permit that to be
>propagated back?

Doing that would affect both codepaths, and as David made clear, would
break backwards compatibility because their established behaviors are
different.

'echo 1 > online' returns 0 if the device is already online
'echo online > state' returns -EINVAL if the device is already online

--
Reza Arbab

\
 
 \ /
  Last update: 2016-09-17 09:58    [W:1.478 / U:0.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site