Hi Sergey, On Tue, Nov 24, 2015 at 10:29:27AM +0900, Sergey Senozhatsky wrote: > Cc Kyeongdon > > On (11/23/15 16:47), Andrew Morton wrote: > [..] > > > > Doesn't make a lot of sense to me. We use a weakened gfp for the > > kmalloc and if that fails, fall into vmalloc() using the stronger gfp > > anyway. > > Sir, you are right. that was "fixed" in my patch (but I definitely should > have been more attentive when I reviewed Kyeongdon's patch and that was > a mistake to address this in my patch) > > I didn't spot it until I replaced vzalloc() with __vmalloc() working on > my GFP_NOIO patch: > > + return __vmalloc(LZ4_MEM_COMPRESS, > + GFP_NOIO | __GFP_NOWARN | __GFP_HIGHMEM | __GFP_ZERO, > + PAGE_KERNEL); > > > but I agree that we have created a mess already. > > > Perhaps it makes sense for higher-order allocations: we don't want to > > thrash around trying to create an order-2 page - we'd prefer to give up > > and fall into vmalloc to do a bunch of order-0 allocations. > > > > But this argument holds for 1000 other kmalloc->vmalloc allocation > > attempts - what's special about this one? > > > > And whatever is the reason for this peculiar setup, > > > > a) where's the proof that the change is actually beneficial? > > b) let's get a good code comment in place so that future readers are not > > similarly puzzled. > > or > > c) start anew (hopefully Minchan and Kyeongdon are with me) > > > Per Kyeongdon's comment > > :When we're using LZ4 multi compression streams for zram swap, > :we found out page allocation failure message in system running test. > :That was not only once, but a few(2 - 5 times per test). > :Also, some failure cases were continually occurring to try allocation > :order 3. > : > :In order to make parallel compression private data, we should call > :kzalloc() with order 2/3 in runtime(lzo/lz4). But if there is no order > : 2/3 size memory to allocate in that time, page allocation fails. > :This patch makes to use vmalloc() as fallback of kmalloc(), this > :prevents page alloc failure warning. > > > so (what I missed in the first place) is that the patch does not really > prevent page alloc failures warnings, because vmalloc() is still free to > warn us on every failed allocation. second, vmalloc() can fail and, thus, > we still will go down the 'do not attempt to allocate any memory anymore, > just wait for available stream to become idle'. > > > so my proposal > > patch 1: > a) switch to GFP_NOIO in critical parts (OR remove entirely remove the > 'dynamic' stream creation functionality. IOW, do not allocate compression > streams from IO path, wait for and use available streams). > > patch 2: > b) do not fallback to vmalloc (we are prepared to handle kmalloc failures) > c) add NOWARN to kmalloc (just to reduce the warning pressure) > > > well, (b) and (c), technically, can be merged with (a) but I have no > objections to have it as a separate patch. > > > > what do you guys think?
First of all, Thanks for the summary and proposal. I think GFP_NOIO critical part(ie, your lockdep fix patch) should go to -stable so it should stand alone. About vmalloc, I like that. Just problem was gfp and we can pass it from upper layer so I believe it makes code looks clean and solve differnt gfp problem. Please look at my patchset I just sent. Thanks a lot! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/