lkml.org 
[lkml]   [2016]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: memory_hotplug: zone_can_shift() returns boolean value
On Mon, Dec 12, 2016 at 03:29:04PM -0500, Yasuaki Ishimatsu wrote:
>--- a/drivers/base/memory.c
>+++ b/drivers/base/memory.c
>@@ -410,15 +410,13 @@ static ssize_t show_valid_zones(struct device *dev,
> sprintf(buf, "%s", zone->name);
>
> /* MMOP_ONLINE_KERNEL */
>- zone_shift = zone_can_shift(start_pfn, nr_pages, ZONE_NORMAL);
>- if (zone_shift) {
>+ if (zone_can_shift(start_pfn, nr_pages, ZONE_NORMAL, &zone_shift)) {
> strcat(buf, " ");
> strcat(buf, (zone + zone_shift)->name);
> }
>
> /* MMOP_ONLINE_MOVABLE */
>- zone_shift = zone_can_shift(start_pfn, nr_pages, ZONE_MOVABLE);
>- if (zone_shift) {
>+ if (zone_can_shift(start_pfn, nr_pages, ZONE_MOVABLE, &zone_shift)) {
> strcat(buf, " ");
> strcat(buf, (zone + zone_shift)->name);
> }

You still need to check zone_shift != 0, otherwise you may get duplicate
output:

$ cat /sys/devices/system/node/node1/memory256/valid_zones
Movable Normal Movable
$ cat /sys/devices/system/node/node1/memory257/valid_zones
Movable Movable

>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -1033,8 +1033,8 @@ static void node_states_set_node(int node, struct memory_notify *arg)
> node_set_state(node, N_MEMORY);
> }
>
>-int zone_can_shift(unsigned long pfn, unsigned long nr_pages,
>- enum zone_type target)
>+bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
>+ enum zone_type target, int *zone_shift)
> {
> struct zone *zone = page_zone(pfn_to_page(pfn));
> enum zone_type idx = zone_idx(zone);

I think you should initialize zone_shift here. It should be 0 if the
function returns false.

*zone_shift = 0;

>@@ -1043,26 +1043,27 @@ int zone_can_shift(unsigned long pfn, unsigned long nr_pages,
> if (idx < target) {
> /* pages must be at end of current zone */
> if (pfn + nr_pages != zone_end_pfn(zone))
>- return 0;
>+ return false;
>
> /* no zones in use between current zone and target */
> for (i = idx + 1; i < target; i++)
> if (zone_is_initialized(zone - idx + i))
>- return 0;
>+ return false;
> }
>
> if (target < idx) {
> /* pages must be at beginning of current zone */
> if (pfn != zone->zone_start_pfn)
>- return 0;
>+ return false;
>
> /* no zones in use between current zone and target */
> for (i = target + 1; i < idx; i++)
> if (zone_is_initialized(zone - idx + i))
>- return 0;
>+ return false;
> }
>
>- return target - idx;
>+ *zone_shift = target - idx;
>+ return true;
> }
>
> /* Must be protected by mem_hotplug_begin() */
>@@ -1089,10 +1090,13 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
> !can_online_high_movable(zone))
> return -EINVAL;
>
>- if (online_type == MMOP_ONLINE_KERNEL)
>- zone_shift = zone_can_shift(pfn, nr_pages, ZONE_NORMAL);
>- else if (online_type == MMOP_ONLINE_MOVABLE)
>- zone_shift = zone_can_shift(pfn, nr_pages, ZONE_MOVABLE);
>+ if (online_type == MMOP_ONLINE_KERNEL) {
>+ if (!zone_can_shift(pfn, nr_pages, ZONE_NORMAL, &zone_shift))
>+ return -EINVAL;
>+ } else if (online_type == MMOP_ONLINE_MOVABLE) {
>+ if (!zone_can_shift(pfn, nr_pages, ZONE_MOVABLE, &zone_shift))
>+ return -EINVAL;
>+ }
>
> zone = move_pfn_range(zone_shift, pfn, pfn + nr_pages);
> if (!zone)
>--
>1.8.3.1
>

--
Reza Arbab

\
 
 \ /
  Last update: 2016-12-13 16:57    [W:0.215 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site