mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: apw@shadowen.org, agl@us.ibm.com, mbligh@us.ibm.com,
	linux-kernel@vger.kernel.org, linuxppc64-dev@lists.linuxppc.org
Subject: Re: put_page() tries to handle hugepages but fails
Date: Fri, 23 Apr 2004 01:34:37 -0700	[thread overview]
Message-ID: <20040423013437.1f2b8fc6.akpm@osdl.org> (raw)
In-Reply-To: <20040423081856.GJ9243@zax>

David Gibson <david@gibson.dropbear.id.au> wrote:
>
> Andrew, please apply.
> 
> The code of put_page() is misleading, in that it appears to have code
> handling PageCompound pages (i.e. hugepages).  However it won't
> actually handle them correctly - __page_cache_release() will not work
> properly on a compound.  Instead, hugepages should be and are released
> with huge_page_release() from mm/hugetlb.c.  This patch removes the
> broken PageCompound path from put_page(), replacing it with a
> BUG_ON().  This also removes the initialization of page[1].mapping
> from compoound pages, which was only ever used in this broken code
> path.

We could certainly remove the test for a null destructor in there and
require that compound pages have a destructor installed.

But the main reason why that code is in there is for transparently handling
direct-io into hugepage regions.  That code does perform put_page against
4k pageframes within the huge page and it does follow the pointer to the
head page.

With your patch applied get_user_pages() and bio_release_pages() will
manipulate the refcounts of the inner 4k pages rather than the head pages
and things will explode.

We could change follow_hugetlb_page() to always take a ref against the head
page and we could teach bio_release_pages() to perform appropriate pfn
masking to locate the head page, and perform similar tricks for
futexes-in-large-pages.  But with the code as-is the refcounting works
transparently.


If it's "broken" I wanna know why.

  reply	other threads:[~2004-04-23  8:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-04-23  8:18 David Gibson
2004-04-23  8:34 ` Andrew Morton [this message]
2004-04-23 10:28   ` William Lee Irwin III
2004-04-23 10:35     ` Andrew Morton
2004-04-23 10:47       ` William Lee Irwin III
2004-04-23 11:28         ` William Lee Irwin III
2004-04-27  4:36   ` David Gibson
2004-04-27  4:41     ` David Gibson
2004-04-27  4:47     ` Andrew Morton
2004-04-27  5:05       ` David Gibson
2004-04-27  5:15         ` Andrew Morton
2004-04-27  5:16           ` David Gibson

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=20040423013437.1f2b8fc6.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=agl@us.ibm.com \
    --cc=apw@shadowen.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc64-dev@lists.linuxppc.org \
    --cc=mbligh@us.ibm.com \
    /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

all inboxes | Powered by JetHome®