On Tue, May 23, 2023 at 04:02:12PM +0200, Thomas Gleixner wrote: > _vunmap_aliases() walks the per CPU xarrays to find partially unmapped > blocks and then walks the per cpu free lists to purge fragmented blocks. > > Arguably that's waste of CPU cycles and cache lines as the full xarray walk > already touches every block. > > Avoid this double iteration: > > - Split out the code to purge one block and the code to free the local > purge list into helper functions. > > - Try to purge the fragmented blocks in the xarray walk before looking at > their dirty space. > > Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > --- > mm/vmalloc.c | 66 ++++++++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 43 insertions(+), 23 deletions(-) > > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2086,39 +2086,52 @@ static void free_vmap_block(struct vmap_ > kfree_rcu(vb, rcu_head); > } > > +static bool purge_fragmented_block(struct vmap_block *vb, struct vmap_block_queue *vbq, > + struct list_head *purge_list) Please stick to 80 character lines for the vmalloc code. And while it's personal preference, two tab indents for the continuation make prototypes like this a lot more readable. a lot easier if you > + if (!(vb->free + vb->dirty == VMAP_BBMAP_BITS && vb->dirty != VMAP_BBMAP_BITS)) > + return false; This comes from the old code, but it does looks almost intentionally obsfucated.. if (vb->free + vb->dirty != VMAP_BBMAP_BITS || vb->dirty == VMAP_BBMAP_BITS) actually gets the logic across much better. > + /* prevent further allocs after releasing lock */ > + vb->free = 0; > + /* prevent purging it again */ extra spaces before the comments. Otherwise the refactoring looks nice, thanks.
