Messages in this thread | | | Date | Thu, 1 Sep 2016 16:45:53 -0500 | From | Reza Arbab <> | Subject | Re: [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
| |