Re: [PATCH v2 2/4] mm: vmalloc: use rwsem, mutex for vmap_area_lock and vmap_block->lock

Dave Chinner
2023-03-27T00:39:11Z

On Fri, Mar 24, 2023 at 05:31:55AM +0000, Matthew Wilcox wrote:
> On Fri, Mar 24, 2023 at 04:25:39PM +1100, Dave Chinner wrote:
> > Did you read the comment above this function? I mean, it's all about
> > how poorly kvmalloc() works for the highly concurrent, fail-fast
> > context that occurs in the journal commit fast path, and how we open
> > code it with kmalloc and vmalloc to work "ok" in this path.
> >
> > Then if you go look at the commits related to it, you might find
> > that XFS developers tend to write properly useful changelogs to
> > document things like "it's better, but vmalloc will soon have lock
> > contention problems if we hit it any harder"....
>
> The problem with writing whinges like this is that mm developers don't
> read XFS changelogs. I certainly had no idea this was a problem, and
> I doubt anybody else who could make a start at fixing this problem had
> any idea either. Why go to all this effort instead of sending an email
> to linux-mm?

<sigh>

If you read the mm/vmalloc.c change logs, you'd find that two weeks
later, a bunch of commits went into the vmalloc code to change some
of the stuff mentioned in the above XFS commit. That was a direct
result of the discussion of vmalloc/kvmalloc inadequacies, and if
you followed the links from these three commits:

30d3f01191d3 mm/vmalloc: be more explicit about supported gfp flags.
9376130c390a mm/vmalloc: add support for __GFP_NOFAIL
451769ebb7e7 mm/vmalloc: alloc GFP_NO{FS,IO} for vmalloc

You'd have found those discussions.

I went into great detail in that discussion about the problems with
the kvmalloc/vmalloc APIs and the problems with actually using it in
anger. e.g:

https://lore.kernel.org/all/163184741778.29351.16920832234899124642.stgit@noble.brown/T/#e8bc85de35d432dcbc35a16fc72b6a3daef2a0f78

In that discussion, I gave these examples and use cases about
fail-fast for the kmalloc part of kvmalloc being needed, that
arguments that GFP_NOFS didn't work with vmalloc were bullshit
because we'd been using it heavily for years in GFP_NOFS contexts
without issues, the lack of scope APIs for anything other NOFS/NOIO,
that filesytsems want "retry forever" semantics, not the current
__GFP_NOFAIL semantics that have all sorts of weird side effects,
etc. I also point out that vmalloc is rapidly becoming one of the
hottest paths in XFS in response to the comments that vmalloc "isn't
a hot path".

Indeed, I point that the XFS change in that commit during that
discussion, and you made exactly the same "you should raise this
with mm developers" complaint then, too. Imagine how frsutrating it
is when I was being told to raise vmalloc issues on the linux-mm
list with mm developers during a discussion about vmalloc issues
with mm developers on the linux-mm list. Especially as it wasn't
just one mm developer that responded like that.

And yet, the 3 commits that came out of the discussion did nothing
to change the actual problem we need to fix - fail-fast high-order
kmalloc behaviour in kvmalloc() - and so the XFS commit still stands
and is badly needed.

Repeatedly castigating people saying we should talk to mm developers
rather than working around the API they maintain when we've
repeatedly talked to the mm developers about getting changes made
and repeatedly failed to get the changes we need made? Yeah, that
leads to frustrations and commit messages documenting all the shit
we haven't been able to get changed and so need to work around.....

-Dave.
--
Dave Chinner
david@fromorbit.com