lkml.org 
[lkml]   [2012]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] mm: Warn about costly page allocation
    On Mon, Jul 09, 2012 at 09:50:48PM +0900, Minchan Kim wrote:
    > > <SNIP>
    > >
    > > You're aiming this at embedded QA people according to your changelog so
    > > do whatever you think is going to be the most effective. It's already
    > > "known" that high-order kernel allocations are meant to be unreliable and
    > > apparently this is being ignored. The in-code warning could look
    > > something like
    > >
    > > if (unlikely(order > PAGE_ALLOC_COSTLY_ORDER)) {
    > > printk_once("%s: page allocation high-order stupidity: order:%d, mode:0x%x\n",
    > > current->comm, order, gfp_mask);
    > > if (gfp_flags & __GFP_MOVABLE) {
    > > printk_once("Enable compaction or whatever\n");
    > > dump_stack();
    > > } else {
    > > printk_once("Regular high-order kernel allocations like this will eventually start failing.");
    > > dump_stack();
    > > }
    > > }
    >
    > I'm not sure we have to check further for __GFP_MOVABLE because I have not seen driver
    > uses __GFP_MOVABLE for high order allocation. Although it uses the flag, it's never
    > compactable since it's out of LRU list. So I think it's rather overkill.
    >

    Then I would have considered it even more important to warn them that
    their specific usage is going to break eventually, with or without
    compaction. However, you know the target audience for this warning so it's
    your call.

    > >
    > > There should be a comment above it giving more information if you think
    > > the embedded people will actually read it. Of course, if this warning
    > > triggers during driver initialisation then it might be a completely useless.
    > > You could rate limit the warning (printk_ratelimit()) instead to be more
    > > effective. As I don't know what sort of device drivers you are seeing this
    > > problem with I can't judge what the best style of warning would be.
    >
    > Okay.
    > I will send patch like below tomorrow if there isn't any objection.
    >
    > if (unlikely(order > PAGE_ALLOC_COSTLY_ORDER)) {
    > if (printk_ratelimit()) {
    > printk("%s: page allocation high-order stupidity: order:%d, mode:0x%x\n",
    > current->comm, order, gfp_mask);
    > printk_once("Enable compaction or whatever\n");
    > printk_once("Regular high-order kernel allocations like this will eventually start failing.\n");
    > dump_stack();
    > }
    > }

    The warning message could be improved. I did not expect you to use "Enable
    compaction or whatever" verbatim. I was just illustrating what type of
    warnings I thought might be useful. I expected you would change it to
    something that embedded driver authors would pay attention to :)

    As you are using printk_ratelimit(), you can also use pr_warning to
    annotate this as KERN_WARNING.

    --
    Mel Gorman
    SUSE Labs


    \
     
     \ /
      Last update: 2012-07-09 15:42    [W:4.883 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site