Re: [PATCH] xfs: Do not free xfs_extent_busy from inside a spinlock

Jeff Layton
2019-07-23T17:38:11Z

On Tue, 2019-07-23 at 10:08 -0700, Christoph Hellwig wrote:
> On Tue, Jul 23, 2019 at 01:07:00PM -0400, Jeff Layton wrote:
> > Note that those places are already broken. AIUI, the basic issue is that
> > vmalloc/vfree have to fix up page tables and that requires being able to
> > sleep. This patch just makes this situation more evident. If that patch
> > gets merged, I imagine we'll have a lot of places to clean up (not just
> > in xfs).
> >
> > Anyway, in the case of being in an interrupt, we currently queue the
> > freeing to a workqueue. Al mentioned that we could create a new
> > kvfree_atomic that we could use from atomic contexts like this. That may
> > be another option (though Carlos' patch looked reasonable to me and
> > would probably be more efficient).
>
> The point is for XFS we generally only use kmem_free for pure kmalloc
> allocations under spinlocks. But yes, the interfac is a little
> suboptimal and a kmem_free_large would be nicer and then warnings like
> this that might be pretty useful could be added.

Ahh ok, I get it now. You're using it as a generic "free this, no matter
what it is" wrapper, and relying on the caller to ensure that it will
never try to free a vmalloc'ed addr from an atomic context.

I wonder how many other places are doing that? I count 858 call sites
for kvfree. If significant portion of those are doing this, then we may
have to re-think my patch. It seems like the right thing to do, but we
there may be more fallout than I expected.
--
Jeff Layton <jlayton@kernel.org>