Received-SPF: pass (google.com: domain of linux-kernel+bounces-94899-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <140860ab-59f2-4633-8a55-fb6cfa854250@intel.com>
Date: Thu, 7 Mar 2024 11:01:11 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 1/2] mm/vmalloc: Moved macros with no functional change
 happened
Content-Language: en-US
To: Baoquan He <bhe@redhat.com>, Uladzislau Rezki <urezki@gmail.com>
Cc: akpm@linux-foundation.org, colin.king@intel.com, hch@infradead.org,
 linux-kernel@vger.kernel.org, linux-mm@kvack.org, lstoakes@gmail.com,
 tianyou.li@intel.com, tim.c.chen@intel.com, wangyang.guo@intel.com,
 zhiguo.zhou@intel.com
References: <20240301155417.1852290-1-rulin.huang@intel.com>
 <20240301155417.1852290-2-rulin.huang@intel.com> <Zei9n-VMxtzG8z4Y@pc636>
 <ZekW/nGXfTqOlvPZ@MiWiFi-R3L-srv>
From: "Huang, Rulin" <rulin.huang@intel.com>
In-Reply-To: <ZekW/nGXfTqOlvPZ@MiWiFi-R3L-srv>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

We have made changes based on your latest suggestions.
1.Removed bugs_on.
2.Removed adjustion of macros.

We submitted patch v8 based on this. Thanks to Baoquan for the
discussion, and could you please help to review and confirm if there are
any problems on the latest version?

On 2024/3/7 9:23, Baoquan He wrote:
> On 03/06/24 at 08:01pm, Uladzislau Rezki wrote:
>> On Fri, Mar 01, 2024 at 10:54:16AM -0500, rulinhuang wrote:
> ......
>>
>> Sorry for the late answer, i also just noticed this email. It was not in
>> my inbox...
>>
>> OK, now you move part of the per-cpu allocator on the top and leave
>> another part down making it split. This is just for the:
>>
>> BUG_ON(va_flags & VMAP_RAM);
>>
>> VMAP_RAM macro. Do we really need this BUG_ON()?
> 
> Sorry, I suggested that when reviewing v5:
> https://lore.kernel.org/all/ZdiltpK5fUvwVWtD@MiWiFi-R3L-srv/T/#u
> 
> About part of per-cpu kva allocator moving and the split making, I would
> argue that we will have vmap_nodes defintion and basic helper functions
> like addr_to_node_id() etc at top, and leave other part like
> size_to_va_pool(), node_pool_add_va() etc down. These are similar.
> 
> While about whether we should add 'BUG_ON(va_flags & VMAP_RAM);', I am
> not sure about it. When I suggested that, I am also hesitant. From the
> current code, alloc_vmap_area() is called in below three functions, only
> __get_vm_area_node() will pass the non-NULL vm. 
>  new_vmap_block()     -|
>  vm_map_ram()         ----> alloc_vmap_area()
>  __get_vm_area_node() -|
> 
> It could be wrongly passed in the future? Only checking if vm is
> non-NULL makes me feel a little unsafe. While I am fine if removing the
> BUG_ON, because there's no worry in the current code. We can wait and
> see in the future.
> 
>        if (vm) {
>                BUG_ON(va_flags & VMAP_RAM);
>                setup_vmalloc_vm(vm, va, flags, caller);
>        }
> 
> Thanks
> Baoquan
>