On Thu 29-11-18 05:08:15, Wei Yang wrote:
> Function show_mem() is used to print system memory status when user
> requires or fail to allocate memory. Generally, this is a best effort
> information and not willing to affect core mm subsystem.
I would drop the part after and
> The data protected by pgdat_resize_lock is mostly correct except there is:
>
> * page struct defer init
> * memory hotplug
This is more confusing than helpful. I would just drop it.
The changelog doesn't explain what is done and why. The second one is
much more important. I would say this
"
Function show_mem() is used to print system memory status when user
requires or fail to allocate memory. Generally, this is a best effort
information so any races with memory hotplug (or very theoretically an
early initialization) should be toleratable and the worst that could
happen is to print an imprecise node state.
Drop the resize lock because this is the only place which might hold the
lock from the interrupt context and so all other callers might use a
simple spinlock. Even though this doesn't solve any real issue it makes
the code easier to follow and tiny more effective.
"
>
> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
> ---
> lib/show_mem.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/lib/show_mem.c b/lib/show_mem.c
> index 0beaa1d899aa..1d996e5771ab 100644
> --- a/lib/show_mem.c
> +++ b/lib/show_mem.c
> @@ -21,7 +21,6 @@ void show_mem(unsigned int filter, nodemask_t *nodemask)
> unsigned long flags;
btw. you want to drop flags.
> int zoneid;
>
> - pgdat_resize_lock(pgdat, &flags);
> for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) {
> struct zone *zone = &pgdat->node_zones[zoneid];
> if (!populated_zone(zone))
> @@ -33,7 +32,6 @@ void show_mem(unsigned int filter, nodemask_t *nodemask)
> if (is_highmem_idx(zoneid))
> highmem += zone->present_pages;
> }
> - pgdat_resize_unlock(pgdat, &flags);
> }
>
> printk("%lu pages RAM\n", total);
> --
> 2.15.1
>
--
Michal Hocko
SUSE Labs
