On Tue, 2019-07-23 at 08:51 -0700, Christoph Hellwig wrote: > On Tue, Jul 23, 2019 at 05:31:33PM +0200, Carlos Maiolino wrote: > > CC'ing Jeff so he can maybe chime in too. > > > > > > > Er, what problem does this solve? Does holding on to the pag spinlock > > > too long while memory freeing causes everything else to stall? When is > > > memory freeing slow enough to cause a noticeable impact? > > > > Jeff detected it when using this patch: > > > > https://marc.info/?l=linux-mm&m=156388753722881&w=2 > > > > At first I don't see any specific problem, but I don't think we are supposed to > > use kmem_free() inside interrupt context anyway. So, even though there is no > > visible side effect, it should be fixed IMHO. With the patch above, the side > > effect is a bunch of warnings :P > > This is going to break lots of places in xfs. While we have separate > allocation side wrappers for plain kmalloc vs using a vmalloc fallback we > always use the same free side wrapper. We could fix this by adding a > kmem_free_large and switch all places that allocated using > kmem_alloc_large to that, but it will require a bit of work. (cc'ing Al) 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). -- Jeff Layton <jlayton@xxxxxxxxxx>
