linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Henrik Rydberg <rydberg@euromail.se>, Jan Kara <jack@suse.cz>,
	linux-mm <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Oops in 3.7-rc8 isolate_free_pages_block()
Date: Fri, 7 Dec 2012 08:32:48 +0000	[thread overview]
Message-ID: <20121207083248.GF17258@suse.de> (raw)
In-Reply-To: <CA+55aFy4Lv+_aPEakOJNR2F9PR=09jviT6Z70_NkWV5bSH5ABw@mail.gmail.com>

On Thu, Dec 06, 2012 at 11:38:47AM -0800, Linus Torvalds wrote:
> Ok, I've applied the patch.
> 

Thanks.

> Mel, some grepping shows that there is an old line that does
> 
>     end_pfn = ALIGN(low_pfn + pageblock_nr_pages, pageblock_nr_pages);
> 
> which looks bogus.

It's bogus. The impact is that multiple compaction attempts may be needed
to clear a particular block for allocation. THP allocation success rate
under stress will be lower and the latency before a range of pages is
collapsed by khugepaged to a huge page will be higher. The impact of this
is less and it should not result in a bug like Henrik's

An attentive reviewer is going to exclaim that GFP_ATOMIC allocations for
jumbo frames is impacted by this but it isn't. Even with this bogus walk,
compaction will be clearing SWAP_CLUSTER_MAX contiguous chunks which is
enough for jumbo frames.

> That should probably also use "+ 1" instead. But
> I'll consider that an independent issue, so I applied the one patch
> regardless.
> 
> There is also a
> 
>     low_pfn += pageblock_nr_pages;
>     low_pfn = ALIGN(low_pfn, pageblock_nr_pages) - 1;
> 
> that looks suspicious for similar reasons. Maybe
> 
>     low_pfn = ALIGN(low_pfn + 1, pageblock_nr_pages) - 1;
> 

This one is working by co-incidence because the low_pfn will be aligned
in most cases. If it was outright broken then CMA would never work either.

> instead? Although that *can* result in the same low_pfn in the end, so
> maybe that one was correct after all? I just did some grepping, no
> actual semantic analysis...
> 

They need fixing but the impact is much less severe and does not justify
delaying 3.8 over unlike the other last-minute fixes. My performance
writing patches during talks was less than stellar yesterday so I'll avoid
a repeat performance and follow up with Andrew early next week with a cc
to -stable. It'll also give me a chance to run the patches through the
highalloc stress tests and confirm that allocation success rate is higher
and latency lower as would be expected by such a fix.

Thanks.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2012-12-07  8:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20121206091744.GA1397@polaris.bitmath.org>
2012-12-06 14:48 ` Oops in 3.7-rc8 isolate_free_pages_block() Jan Kara
2012-12-06 15:22   ` Henrik Rydberg
2012-12-06 16:10     ` Linus Torvalds
2012-12-06 16:35       ` Mel Gorman
2012-12-06 16:19   ` Mel Gorman
2012-12-06 16:50     ` Linus Torvalds
2012-12-06 17:55       ` Mel Gorman
2012-12-06 18:19         ` Linus Torvalds
2012-12-06 18:21           ` Mel Gorman
2012-12-06 18:32           ` Henrik Rydberg
2012-12-06 18:41             ` Linus Torvalds
2012-12-06 19:01               ` Mel Gorman
2012-12-06 19:28               ` Henrik Rydberg
2012-12-06 19:38                 ` Linus Torvalds
2012-12-06 21:39                   ` Henrik Rydberg
2012-12-07  8:32                   ` Mel Gorman [this message]
2012-12-06 16:58     ` Henrik Rydberg
2012-12-06 17:22     ` Henrik Rydberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121207083248.GF17258@suse.de \
    --to=mgorman@suse.de \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rydberg@euromail.se \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).