linux-kernel - Re: [PATCH] mm, memory hotplug: print more failure information for online

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1602241331570.5955@chino.kir.corp.google.com>
Date:	Wed, 24 Feb 2016 13:33:41 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	Chen Yucong <slaoub@...il.com>
cc:	akpm@...ux-foundation.org, vbabka@...e.cz, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm, memory hotplug: print more failure information for
 online_pages

On Wed, 24 Feb 2016, Chen Yucong wrote:

> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index c832ef3..e4b6dec3 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1059,10 +1059,9 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
>  
>  	ret = memory_notify(MEM_GOING_ONLINE, &arg);
>  	ret = notifier_to_errno(ret);
> -	if (ret) {
> -		memory_notify(MEM_CANCEL_ONLINE, &arg);
> -		return ret;
> -	}
> +	if (ret)
> +		goto failed_addition;
> +
>  	/*
>  	 * If this zone is not populated, then it is not in zonelist.
>  	 * This means the page allocator ignores this zone.
> @@ -1080,12 +1079,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
>  		if (need_zonelists_rebuild)
>  			zone_pcp_reset(zone);
>  		mutex_unlock(&zonelists_mutex);
> -		printk(KERN_DEBUG "online_pages [mem %#010llx-%#010llx] failed\n",
> -		       (unsigned long long) pfn << PAGE_SHIFT,
> -		       (((unsigned long long) pfn + nr_pages)
> -			    << PAGE_SHIFT) - 1);
> -		memory_notify(MEM_CANCEL_ONLINE, &arg);
> -		return ret;
> +		goto failed_addition;
>  	}
>  
>  	zone->present_pages += onlined_pages;
> @@ -1118,6 +1112,13 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
>  	if (onlined_pages)
>  		memory_notify(MEM_ONLINE, &arg);
>  	return 0;
> +
> +failed_addition:
> +	pr_info("online_pages [mem %#010llx-%#010llx] failed\n",
> +		(unsigned long long) pfn << PAGE_SHIFT,
> +		(((unsigned long long) pfn + nr_pages) << PAGE_SHIFT) - 1);
> +	memory_notify(MEM_CANCEL_ONLINE, &arg);
> +	return ret;
>  }
>  #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
>  

Please explain how the conversion from KERN_DEBUG to KERN_INFO level is 
better?

If the onlining returns an error value, which it will, why do we need to 
leave an artifact behind in the kernel log that it failed?