Re: [PATCH V2 1/2] mm/memblock.c: trivial code refine in memblock_is_region_memory()
From: Michal Hocko <mhocko@kernel.org>
Date: 2016-12-21 12:48:22
Also in:
lkml
On Wed 21-12-16 12:43:20, Wei Yang wrote:
On Wed, Dec 21, 2016 at 08:48:09AM +0100, Michal Hocko wrote:quoted
On Tue 20-12-16 16:35:40, Wei Yang wrote:quoted
On Mon, Dec 19, 2016 at 04:15:14PM +0100, Michal Hocko wrote:quoted
On Sun 18-12-16 14:47:49, Wei Yang wrote:quoted
The base address is already guaranteed to be in the region by memblock_search().Hi, Michal Nice to receive your comment.quoted
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...Hmm... the memblock_search() is called by memblock_is_region_memory(). Maybe I paste the whole function here would clarify the change. int __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t size) { int idx = memblock_search(&memblock.memory, base); phys_addr_t end = base + memblock_cap_size(base, &size); if (idx == -1) return 0; return memblock.memory.regions[idx].base <= base && (memblock.memory.regions[idx].base + memblock.memory.regions[idx].size) >= end; }Ohh, my bad. I thought that memblock_search is calling memblock_is_region_memory. I didn't notice this is other way around. Then I agree that the check for the base is not needed and can be removed.Thanks~ I would feel honored if you would like to add Acked-by :-)
My Nack to the original patch still holds. If you want to remove the check then remove it rather than comment it out. -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>