linux-kernel - Re: [PATCH 1/1] 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]

Message-ID: <ZCFCnJXWx3cAM4zv@pc636>
Date:   Mon, 27 Mar 2023 09:15:40 +0200
From:   Uladzislau Rezki <urezki@...il.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     "Uladzislau Rezki (Sony)" <urezki@...il.com>, linux-mm@...ck.org,
        LKML <linux-kernel@...r.kernel.org>, Baoquan He <bhe@...hat.com>,
        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 1/1] mm: vmalloc: Remove a global vmap_blocks xarray

On Thu, Mar 23, 2023 at 02:12:53PM -0700, Andrew Morton wrote:
> On Thu, 23 Mar 2023 20:21:11 +0100 "Uladzislau Rezki (Sony)" <urezki@...il.com> wrote:
> 
> > A global vmap_blocks-xarray array can be contented under
> > heavy usage of the vm_map_ram()/vm_unmap_ram() APIs. The
> > lock_stat shows that a "vmap_blocks.xa_lock" lock is a
> > second in a top-list when it comes to contentions:
> > 
> > ...
> > 
> > This patch does not fix vmap_area_lock/free_vmap_area_lock and
> > purge_vmap_area_lock bottle-necks, it is rather a separate rework.
> > 
> > ...
> >
> >  static DEFINE_PER_CPU(struct vmap_block_queue, vmap_block_queue);
> >  
> > ...
> >
> > +static struct vmap_block_queue *
> > +addr_to_vbq(unsigned long addr)
> > +{
> > +	int cpu = (addr / VMAP_BLOCK_SIZE) % num_possible_cpus();
> > +	return &per_cpu(vmap_block_queue, cpu);
> > +}
> 
> Seems strange.  vmap_block_queue is not a per-cpu thing in this usage. 
> Instead it's a hash table, indexed off the (hashed) address, not off
> smp_processor_id().
> 
> Yet in other places, vmap_block_queue *is* used in the conventional
> cpu-local fashion.
> 
> So we can have CPU A using the cpu-local entry in vmap_block_queue
> while CPU B is simultaneously using it, having looked it up via `addr'.
> 
> AFAICT this all works OK, no races.
> 
> But still, what it's doing is mixing an addr-indexed hashtable with the
> CPU-indexed array in surprising ways.  It would be clearer to make the
> vmap_blocks array a separate thing from the per-cpu array, although it
> would presumably use a bit more memory.
> 
> Can we please at least get a big fat comment in an appropriate place
> which explains all this to the reader?
>
Yep, i will send out a v2 with all explanation. Indeed i have to add
detailed explanation.

Thanks!

--
Uladzislau Rezki