Messages in this thread | | | Subject | Re: [UPDATE][PATCH 2/3] resource: Add release_mem_region_adjustable() | From | Toshi Kani <> | Date | Mon, 08 Apr 2013 08:26:36 -0600 |
| |
On Mon, 2013-04-08 at 11:01 +0900, Yasuaki Ishimatsu wrote: > Hi Toshi, > > 2013/04/04 8:23, Toshi Kani wrote: > > Added release_mem_region_adjustable(), which releases a requested > > region from a currently busy memory resource. This interface > > adjusts the matched memory resource accordingly if the requested > > region does not match exactly but still fits into. > > > > This new interface is intended for memory hot-delete. During > > bootup, memory resources are inserted from the boot descriptor > > table, such as EFI Memory Table and e820. Each memory resource > > entry usually covers the whole contigous memory range. Memory > > hot-delete request, on the other hand, may target to a particular > > range of memory resource, and its size can be much smaller than > > the whole contiguous memory. Since the existing release interfaces > > like __release_region() require a requested region to be exactly > > matched to a resource entry, they do not allow a partial resource > > to be released. > > > > There is no change to the existing interfaces since their restriction > > is valid for I/O resources. > > > > Signed-off-by: Toshi Kani <toshi.kani@hp.com> > > Reviewed-by : Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Great! Thanks Yasuaki!
> One nitpick below. >
(snip)
> > +/** > > + * release_mem_region_adjustable - release a previously reserved memory region > > + * @parent: parent resource descriptor > > + * @start: resource start address > > + * @size: resource region size > > + * > > + * The requested region is released from a currently busy memory resource. > > + * It adjusts the matched busy memory resource accordingly if the requested > > + * region does not match exactly but still fits into. Existing children of > > + * the busy memory resource must be immutable in this request. > > + * > > + * Note, when the busy memory resource gets split into two entries, the code > > + * assumes that all children remain in the lower address entry for simplicity. > > + * Enhance this logic when necessary. > > + */ > > +int release_mem_region_adjustable(struct resource *parent, > > + resource_size_t start, resource_size_t size) > > +{ > > + struct resource **p; > > + struct resource *res, *new; > > + resource_size_t end; > > + int ret = -EINVAL; > > + > > > + end = start + size - 1; > > + if ((start < parent->start) || (end > parent->end)) > > + return -EINVAL; > > "ret" is initialized to -EINVAL. So how about use it?
Sounds good. I will make the change.
Thanks again, -Toshi
| |