linux-kernel - Re: [PATCH] base/memory: pass the base_section in add_memory

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: <20170613114842.GI10819@dhcp22.suse.cz>
Date:   Tue, 13 Jun 2017 13:48:42 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Wei Yang <richard.weiyang@...il.com>
Cc:     gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] base/memory: pass the base_section in add_memory_block

[Sorry for a late response]

On Wed 07-06-17 16:52:12, Wei Yang wrote:
> The second parameter of init_memory_block() is used to calculate the
> start_section_nr of this block, which means any section in the same block
> would get the same start_section_nr.

Could you be more specific what is the problem here?

> This patch passes the base_section to init_memory_block(), so that to
> reduce a local variable and a check in every loop.

But then you are not handling a memblock which starts with a !present
section. The code is quite hairy but I do not see why your change is any
more correct. This needs much better justification than what the above
gives us. Maybe the whole thing about incomplete memblock is just
overengineered piece of code, who knows this area is full of stuff that
makes only little sense but again the changelog should be pretty verbose
about all the consequences and focus on the high level rather than
particular issues here and there.

Thanks

> Signed-off-by: Wei Yang <richard.weiyang@...il.com>
> ---
>  drivers/base/memory.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index cc4f1d0cbffe..1e903aba2aa1 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -664,21 +664,20 @@ static int init_memory_block(struct memory_block **memory,
>  static int add_memory_block(int base_section_nr)
>  {
>  	struct memory_block *mem;
> -	int i, ret, section_count = 0, section_nr;
> +	int i, ret, section_count = 0;
>  
>  	for (i = base_section_nr;
>  	     (i < base_section_nr + sections_per_block) && i < NR_MEM_SECTIONS;
>  	     i++) {
>  		if (!present_section_nr(i))
>  			continue;
> -		if (section_count == 0)
> -			section_nr = i;
>  		section_count++;
>  	}
>  
>  	if (section_count == 0)
>  		return 0;
> -	ret = init_memory_block(&mem, __nr_to_section(section_nr), MEM_ONLINE);
> +	ret = init_memory_block(&mem, __nr_to_section(base_section_nr),
> +				MEM_ONLINE);
>  	if (ret)
>  		return ret;
>  	mem->section_count = section_count;
> -- 
> 2.11.0

-- 
Michal Hocko
SUSE Labs