linux-kernel - Re: [PATCH v3 1/2] mm: vmalloc: Remove a global vmap

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:   Wed, 29 Mar 2023 08:54:20 +0200
From:   Uladzislau Rezki <urezki@...il.com>
To:     Baoquan He <bhe@...hat.com>
Cc:     Uladzislau Rezki <urezki@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        LKML <linux-kernel@...r.kernel.org>,
        Lorenzo Stoakes <lstoakes@...il.com>,
        Christoph Hellwig <hch@...radead.org>,
        Matthew Wilcox <willy@...radead.org>,
        Dave Chinner <david@...morbit.com>,
        Oleksiy Avramchenko <oleksiy.avramchenko@...y.com>
Subject: Re: [PATCH v3 1/2] mm: vmalloc: Remove a global vmap_blocks xarray

On Wed, Mar 29, 2023 at 12:33:05PM +0800, Baoquan He wrote:
> On 03/28/23 at 02:34pm, Uladzislau Rezki wrote:
> ......  
> > > > @@ -2003,8 +2037,8 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
> > > >  	bitmap_set(vb->used_map, 0, (1UL << order));
> > > >  	INIT_LIST_HEAD(&vb->free_list);
> > > >  
> > > > -	vb_idx = addr_to_vb_idx(va->va_start);
> > > > -	err = xa_insert(&vmap_blocks, vb_idx, vb, gfp_mask);
> > > > +	vbq = addr_to_vbq(va->va_start);
> > > > +	err = xa_insert(&vbq->vmap_blocks, va->va_start, vb, gfp_mask);
> > > 
> > > Using va->va_start as index to access xarray may cost extra memory.
> > > Imagine we got a virtual address at VMALLOC_START, its region is
> > > [VMALLOC_START, VMALLOC_START+4095]. In the xarray, its sequence order
> > > is 0. While with va->va_start, it's 0xffffc90000000000UL on x86_64 with
> > > level4 paging mode. That means for the first page size vmalloc area,
> > > storing it into xarray need about 10 levels of xa_node, just for the one
> > > page size. With the old addr_to_vb_idx(), its index is 0. Only one level
> > > height is needed. One xa_node is about 72bytes, it could take more time
> > > and memory to access va->va_start. Not sure if my understanding is correct.
> > > 
> > > static unsigned long addr_to_vb_idx(unsigned long addr)
> > > {
> > >         addr -= VMALLOC_START & ~(VMAP_BLOCK_SIZE-1);
> > >         addr /= VMAP_BLOCK_SIZE;
> > >         return addr;
> > > }
> > > 
> > If the size of array depends on index "length", then, indeed it will require
> > more memory. From the other hand we can keep the old addr_to_vb_idx() function 
> > in order to "cut" a va->va_start index.
> 
> Yeah, the extra 10 levels of xa_node is unnecessary if we keep the old
> addr_to_vb_idx(). And the prolonged path will cost more time to reach the 
> wanted leaf node. E.g on x86_64 with 4 level paging mode, vmalloc area
> is 32TB. With the old calculation, its index range is [0, 8M], 4 level
> heights of xa_node at most is enough to cover.
> 
Good! I have not analyzed how xarray stores its indexes. I will update
the patch to cut indexes so we stay the same as we used to be before.

--
Uladzislau Rezki