From: Reza Arbab <arbab@linux.vnet.ibm.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Vlastimil Babka <vbabka@suse.cz>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
David Rientjes <rientjes@google.com>,
Yaowei Bai <baiyaowei@cmss.chinamobile.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Dan Williams <dan.j.williams@intel.com>,
Xishi Qiu <qiuxishi@huawei.com>,
David Vrabel <david.vrabel@citrix.com>,
Chen Yucong <slaoub@gmail.com>, Andrew Banman <abanman@sgi.com>,
Seth Jennings <sjenning@redhat.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] memory-hotplug: fix store_mem_state() return value
Date: Thu, 1 Sep 2016 16:45:53 -0500 [thread overview]
Message-ID: <20160901214553.h7mbmpyzcuxgnloy@arbab-laptop> (raw)
In-Reply-To: <20160901133717.8d753013cfbb640dd28c2783@linux-foundation.org>
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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-09-01 21:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-01 15:29 Reza Arbab
2016-09-01 20:37 ` Andrew Morton
2016-09-01 21:45 ` Reza Arbab [this message]
2016-09-02 1:34 ` Xishi Qiu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160901214553.h7mbmpyzcuxgnloy@arbab-laptop \
--to=arbab@linux.vnet.ibm.com \
--cc=abanman@sgi.com \
--cc=akpm@linux-foundation.org \
--cc=baiyaowei@cmss.chinamobile.com \
--cc=dan.j.williams@intel.com \
--cc=david.vrabel@citrix.com \
--cc=gregkh@linuxfoundation.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=qiuxishi@huawei.com \
--cc=rientjes@google.com \
--cc=sjenning@redhat.com \
--cc=slaoub@gmail.com \
--cc=vbabka@suse.cz \
--cc=vkuznets@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox