From: Toshi Kani <toshi.kani@hp.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Ram Pai <linuxram@us.ibm.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"guz.fnst@cn.fujitsu.com" <guz.fnst@cn.fujitsu.com>,
"Makphaibulchoke, Thavatchai" <thavatchai.makpahibulchoke@hp.com>,
"isimatu.yasuaki@jp.fujitsu.com" <isimatu.yasuaki@jp.fujitsu.com>,
"wency@cn.fujitsu.com" <wency@cn.fujitsu.com>,
"tangchen@cn.fujitsu.com" <tangchen@cn.fujitsu.com>,
"jiang.liu@huawei.com" <jiang.liu@huawei.com>
Subject: Re: [UPDATE][PATCH v2 2/3] resource: Add release_mem_region_adjustable()
Date: Tue, 09 Apr 2013 13:42:08 -0600 [thread overview]
Message-ID: <1365536528.32127.56.camel@misato.fc.hp.com> (raw)
In-Reply-To: <20130409125141.bfe6e26142e5bb2ff229ed29@linux-foundation.org>
On Tue, 2013-04-09 at 12:51 -0700, Andrew Morton wrote:
> On Tue, 09 Apr 2013 13:02:30 -0600 Toshi Kani <toshi.kani@hp.com> wrote:
>
> > > > + /* look for the next resource if it does not fit into */
> > > > + if (res->start > start || res->end < end) {
> > > > + p = &res->sibling;
> > > > + continue;
> > > > + }
> > >
> > > What if the resource overlaps. In other words, the res->start > start
> > > but res->end > end ?
> > >
> > > Also do you handle the case where the range <start,end> spans
> > > across multiple adjacent resources?
> >
> > Good questions! The two cases above are handled as error cases
> > (-EINVAL) by design. A requested region must either match exactly or
> > fit into a single resource entry. There are basically two design
> > choices in release -- restrictive or non-restrictive. Restrictive only
> > releases under certain conditions, and non-restrictive releases under
> > any conditions. Since the existing release interfaces,
> > __release_region() and __release_resource(), are restrictive, I intend
> > to follow the same policy and made this new interface restrictive as
> > well. This new interface handles the common scenarios of memory
> > hot-plug operations well. I think your example cases are non-typical
> > scenarios for memory hot-plug, and I am not sure if they happen under
> > normal cases at this point. Hence, they are handled as error cases for
> > now. We can always enhance this interface when we find them necessary
> > to support as this interface is dedicated for memory hot-plug. In other
> > words, we should make such enhancement after we understand their
> > scenarios well. Does it make sense?
>
> Can you please update the comment to describe the above? Because if
> one reviewer was wondering then later readers will also wonder.
Yes, I will update the comment and send an updated patch again.
Thanks,
-Toshi
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Toshi Kani <toshi.kani@hp.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Ram Pai <linuxram@us.ibm.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"guz.fnst@cn.fujitsu.com" <guz.fnst@cn.fujitsu.com>,
"Makphaibulchoke, Thavatchai" <thavatchai.makpahibulchoke@hp.com>,
"isimatu.yasuaki@jp.fujitsu.com" <isimatu.yasuaki@jp.fujitsu.com>,
"wency@cn.fujitsu.com" <wency@cn.fujitsu.com>,
"tangchen@cn.fujitsu.com" <tangchen@cn.fujitsu.com>,
"jiang.liu@huawei.com" <jiang.liu@huawei.com>
Subject: Re: [UPDATE][PATCH v2 2/3] resource: Add release_mem_region_adjustable()
Date: Tue, 09 Apr 2013 13:42:08 -0600 [thread overview]
Message-ID: <1365536528.32127.56.camel@misato.fc.hp.com> (raw)
In-Reply-To: <20130409125141.bfe6e26142e5bb2ff229ed29@linux-foundation.org>
On Tue, 2013-04-09 at 12:51 -0700, Andrew Morton wrote:
> On Tue, 09 Apr 2013 13:02:30 -0600 Toshi Kani <toshi.kani@hp.com> wrote:
>
> > > > + /* look for the next resource if it does not fit into */
> > > > + if (res->start > start || res->end < end) {
> > > > + p = &res->sibling;
> > > > + continue;
> > > > + }
> > >
> > > What if the resource overlaps. In other words, the res->start > start
> > > but res->end > end ?
> > >
> > > Also do you handle the case where the range <start,end> spans
> > > across multiple adjacent resources?
> >
> > Good questions! The two cases above are handled as error cases
> > (-EINVAL) by design. A requested region must either match exactly or
> > fit into a single resource entry. There are basically two design
> > choices in release -- restrictive or non-restrictive. Restrictive only
> > releases under certain conditions, and non-restrictive releases under
> > any conditions. Since the existing release interfaces,
> > __release_region() and __release_resource(), are restrictive, I intend
> > to follow the same policy and made this new interface restrictive as
> > well. This new interface handles the common scenarios of memory
> > hot-plug operations well. I think your example cases are non-typical
> > scenarios for memory hot-plug, and I am not sure if they happen under
> > normal cases at this point. Hence, they are handled as error cases for
> > now. We can always enhance this interface when we find them necessary
> > to support as this interface is dedicated for memory hot-plug. In other
> > words, we should make such enhancement after we understand their
> > scenarios well. Does it make sense?
>
> Can you please update the comment to describe the above? Because if
> one reviewer was wondering then later readers will also wonder.
Yes, I will update the comment and send an updated patch again.
Thanks,
-Toshi
next prev parent reply other threads:[~2013-04-09 19:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-08 21:47 [UPDATE][PATCH v2 2/3] resource: Add release_mem_region_adjustable() Toshi Kani
2013-04-08 21:47 ` Toshi Kani
2013-04-09 5:48 ` Ram Pai
2013-04-09 5:48 ` Ram Pai
2013-04-09 19:02 ` Toshi Kani
2013-04-09 19:02 ` Toshi Kani
2013-04-09 19:51 ` Andrew Morton
2013-04-09 19:51 ` Andrew Morton
2013-04-09 19:42 ` Toshi Kani [this message]
2013-04-09 19:42 ` Toshi Kani
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=1365536528.32127.56.camel@misato.fc.hp.com \
--to=toshi.kani@hp.com \
--cc=akpm@linux-foundation.org \
--cc=guz.fnst@cn.fujitsu.com \
--cc=isimatu.yasuaki@jp.fujitsu.com \
--cc=jiang.liu@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxram@us.ibm.com \
--cc=tangchen@cn.fujitsu.com \
--cc=thavatchai.makpahibulchoke@hp.com \
--cc=wency@cn.fujitsu.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.