On Fri, Nov 15, 2019 at 09:23:22AM -0800, Darrick J. Wong wrote: > On Fri, Nov 15, 2019 at 03:20:55PM +0100, Carlos Maiolino wrote: > > On Thu, Nov 14, 2019 at 01:00:00PM -0800, Darrick J. Wong wrote: > > > On Thu, Nov 14, 2019 at 09:09:55PM +0100, Carlos Maiolino wrote: > > > > This can be replaced by direct calls to kfree() or kvfree() (whenever > > > > allocation is done via kmem_alloc_io() or kmem_alloc_large(). > > > > > > > > This patch has been partially scripted. I used the following sed to > > > > replace all kmem_free() calls by kfree() > > > > > > > > # find fs/xfs/ -type f -name '*.c' -o -name '*.h' | xargs sed -i \ > > > > 's/kmem_free/kfree/g' > > > > > > Coccinelle? ;) > > > > /me Doesn't understand the reference but thinks Darrick is talking about > > Coccinelle fancy brand :P > > > > /me is adept to conference-wear :D > > http://coccinelle.lip6.fr/ > > The semantic patch thing, because understanding the weird spatch > language is slightly less infuriating than making tons of minor code > changes by hand. :P Oh, I didn't know about this. Thanks. The name could be something different other than a fashion brand making googling for it easier :( > > I can't really say we will have any benefits in segmenting it by using kvfree() > > only where kmem_alloc_{large, io} is used, so I just relied on the comments > > above kvfree(), and well, we have an extra function call and a few extra > > instructions using kvfree(). So, even though it might be 'slightly' faster, this > > might build up on hot paths when handling millions of kfree(). > > > > But, at the end, I'd be lying if I say I spotted any significant difference. > > <nod> Though the way I see it, kfree vs. kvfree is another bookkeepping > detail that xfs developers will have to keep straight. But maybe that's > fine for the dozen or so specialized users of _io and _large? What do > others think? Ok, if we decide to move everything to kvfree() I'll just send a V2 of this patch, which should apply cleanly on top of the other 3. > > > Btw, Dave mentioned in a not so far future, kmalloc() requests will be > > guaranteed to be aligned, so, I wonder if we will be able to replace both > > kmem_alloc_large() and kmem_alloc_io() by simple calls to kvmalloc() which does > > the job of falling back to vmalloc() if kmalloc() fails?! > > Sure, but I'll believe that when I see it. And given that Christoph > Lameter seems totally opposed to the idea, I think we should keep our > silly wrapper for a while to see if they don't accidentally revert it or > something. > Sure, I don't have any plans to do it now in this series or in a very near future, I just used the email to share the idea :P Thanks for the review. -- Carlos
