From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759438AbcHEC1m (ORCPT ); Thu, 4 Aug 2016 22:27:42 -0400 Received: from sender153-mail.zoho.com ([74.201.84.153]:21682 "EHLO sender153-mail.zoho.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966385AbcHEC1i (ORCPT ); Thu, 4 Aug 2016 22:27:38 -0400 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=zapps768; d=zoho.com; h=subject:to:references:cc:from:message-id:date:user-agent:mime-version:in-reply-to:content-type; b=cTtH9P5xA7eJYtrFslD8ppY3vclemrdxKwuAqYtBIqZGbJvBw9eK2pAwoEpfDq5cSR/9XEfYOmUU TpvvVCUqf2Csm8N+Ir8sy/E0H9QpyYlEqMTsGzL0Ek/E6mNEe+7j Subject: Re: [PATCH] mm/vmalloc: fix align value calculation error To: Andrew Morton References: <57A2F6A3.9080908@zoho.com> <57A2FE7B.5070505@zoho.com> <20160804142421.576426492d629f0839298f9a@linux-foundation.org> Cc: tj@kernel.org, hannes@cmpxchg.org, mhocko@kernel.org, minchan@kernel.org, zijun_hu@htc.com, rientjes@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org From: zijun_hu Message-ID: <57A3F98D.1060500@zoho.com> Date: Fri, 5 Aug 2016 10:27:25 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160804142421.576426492d629f0839298f9a@linux-foundation.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/05/2016 05:24 AM, Andrew Morton wrote: >> >> it causes double align requirement for __get_vm_area_node() if parameter >> size is power of 2 and VM_IOREMAP is set in parameter flags >> >> it is fixed by handling the specail case manually due to lack of >> get_count_order() for long parameter >> >> ... >> >> --- a/mm/vmalloc.c >> +++ b/mm/vmalloc.c >> @@ -1357,11 +1357,16 @@ static struct vm_struct *__get_vm_area_node(unsigned long size, >> { >> struct vmap_area *va; >> struct vm_struct *area; >> + int ioremap_size_order; >> >> BUG_ON(in_interrupt()); >> - if (flags & VM_IOREMAP) >> - align = 1ul << clamp_t(int, fls_long(size), >> - PAGE_SHIFT, IOREMAP_MAX_ORDER); >> + if (flags & VM_IOREMAP) { >> + ioremap_size_order = fls_long(size); >> + if (is_power_of_2(size) && size != 1) >> + ioremap_size_order--; >> + align = 1ul << clamp_t(int, ioremap_size_order, PAGE_SHIFT, >> + IOREMAP_MAX_ORDER); >> + } >> >> size = PAGE_ALIGN(size); >> if (unlikely(!size)) > > I'm having trouble with this, and a more complete description would > have helped! > > As far as I can tell, the current code will decide the following: > > size=0x10000: alignment=0x10000 > size=0x0f000: alignment=0x8000 > no, the current code doesn't achieve the above results as shown below size=0x10000 -> fls_long(0x10000)=17 -> alignment=0x20000 size=0x0f000 -> fls_long(0x0f000)=16 -> alignment=0x10000 it is wrong for power of 2 value such as size=0x10000 > And your patch will change it so that > > size=0x10000: alignment=0x8000 > size=0x0f000: alignment=0x8000 > > Correct? > no, my patch will results in the following calculations size=0x10000: alignment=0x10000 size=0x0f000: alignment=0x10000 > If so, I'm struggling to see the sense in this. Shouldn't we be > changing things so that > > size=0x10000: alignment=0x10000 > size=0x0f000: alignment=0x10000 > > ? okay, it is the aim of my patch as explained above > >