linux-kernel - Re: [PATCH V2 1/2] mm/memblock.c: trivial code refine in memblock_is_region

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 19 Dec 2016 16:15:14 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Wei Yang <richard.weiyang@...il.com>
Cc:     trivial@...nel.org, akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 1/2] mm/memblock.c: trivial code refine in
 memblock_is_region_memory()

On Sun 18-12-16 14:47:49, Wei Yang wrote:
> The base address is already guaranteed to be in the region by
> memblock_search().

First of all the way how the check is removed is the worst possible...
Apart from that it is really not clear to me why checking the base
is not needed. You are mentioning memblock_search but what about other
callers? adjust_range_page_size_mask e.g...

You also didn't mention what is the motivation of this change? What will
work better or why it makes sense in general?

Also this seems to be a general purpose function so it should better
be robust.

> This patch removes the check on base.
> 
> Signed-off-by: Wei Yang <richard.weiyang@...il.com>

Without a proper justification and with the horrible way how it is done
Nacked-by: Michal Hocko <mhocko@...e.com>

> ---
>  mm/memblock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..cd85303 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1615,7 +1615,7 @@ int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size
>  
>  	if (idx == -1)
>  		return 0;
> -	return memblock.memory.regions[idx].base <= base &&
> +	return /* memblock.memory.regions[idx].base <= base && */
>  		(memblock.memory.regions[idx].base +
>  		 memblock.memory.regions[idx].size) >= end;
>  }
> -- 
> 2.5.0
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>

-- 
Michal Hocko
SUSE Labs