All of lore.kernel.org
 help / color / mirror / Atom feed
From: "'David Gibson'" <david@gibson.dropbear.id.au>
To: "Chen, Kenneth W" <kenneth.w.chen@intel.com>
Cc: "'Hugh Dickins'" <hugh@veritas.com>,
	Andrew Morton <akpm@osdl.org>,
	linux-kernel@vger.kernel.org
Subject: Re: Hugepage regression
Date: Wed, 11 Oct 2006 11:18:16 +1000	[thread overview]
Message-ID: <20061011011816.GA21235@localhost.localdomain> (raw)
In-Reply-To: <000301c6ecc4$a83af8a0$cb34030a@amr.corp.intel.com>

On Tue, Oct 10, 2006 at 04:34:40PM -0700, Chen, Kenneth W wrote:
> Hugh Dickins wrote on Tuesday, October 10, 2006 12:18 PM
> > Yes, I'd expect your i_mmap_lock to solve the problem: and since
> > you're headed in that direction anyway, it makes most sense to use
> > that solution rather than get into defining arrays, or sacrificing
> > the lazy flush, or risking page_count races.
> > 
> > So please extract the __unmap_hugepage_range mods from your shared
> > pagetable patch, and use that to fix the bug.
> 
> 
> OK, here is a bug fix patch fixing earlier "bug fix" patch :-(
> 
> 
> [patch] hugetlb: fix linked list corruption in unmap_hugepage_range
> 
> commit fe1668ae5bf0145014c71797febd9ad5670d5d05 causes kernel to oops with
> libhugetlbfs test suite.  The problem is that hugetlb pages can be shared
> by multiple mappings. Multiple threads can fight over page->lru in the unmap
> path and bad things happen.  We now serialize __unmap_hugepage_range to void
> concurrent linked list manipulation.  Such serialization is also needed for
> shared page table page on hugetlb area. This patch will fixed the bug and
> also serve as a prepatch for shared page table.

Can I suggest that you put a big comment on the linked list
declaration itself saying that you're relying on serialization here.
Otherwise I'm worried someone will try to de-serialize it again, and
break it without realizing.  Given the number of people who failed to
spot the problem with the patch the first time around..

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

  reply	other threads:[~2006-10-11  1:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-10  8:47 Hugepage regression David Gibson
2006-10-10  9:04 ` Andrew Morton
2006-10-10  9:15   ` David Gibson
2006-10-10 17:35     ` Chen, Kenneth W
2006-10-10 19:14       ` Andrew Morton
2006-10-10 19:18       ` Hugh Dickins
2006-10-10 19:30         ` Chen, Kenneth W
2006-10-10 20:10           ` Hugh Dickins
2006-10-10 23:03             ` Chen, Kenneth W
2006-10-13 17:03               ` Hugh Dickins
2006-10-10 23:34         ` Chen, Kenneth W
2006-10-11  1:18           ` 'David Gibson' [this message]
2006-10-11  2:47             ` Chen, Kenneth W

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=20061011011816.GA21235@localhost.localdomain \
    --to=david@gibson.dropbear.id.au \
    --cc=akpm@osdl.org \
    --cc=hugh@veritas.com \
    --cc=kenneth.w.chen@intel.com \
    --cc=linux-kernel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.