linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nick Piggin <nickpiggin@yahoo.com.au>
To: "Chen, Kenneth W" <kenneth.w.chen@intel.com>
Cc: Hugh Dickins <hugh@veritas.com>,
	'David Gibson' <david@gibson.dropbear.id.au>,
	g@ozlabs.org, Andrew Morton <akpm@osdl.org>,
	'Christoph Lameter' <christoph@schroedinger.engr.sgi.com>,
	bill.irwin@oracle.com, Adam Litke <agl@us.ibm.com>,
	linux-mm@kvack.org
Subject: Re: [RFC] reduce hugetlb_instantiation_mutex usage
Date: Thu, 02 Nov 2006 14:06:34 +1100	[thread overview]
Message-ID: <454960BA.9070801@yahoo.com.au> (raw)
In-Reply-To: <000001c6fd9e$ef709230$8984030a@amr.corp.intel.com>

Chen, Kenneth W wrote:

>Nick Piggin wrote on Tuesday, October 31, 2006 10:19 PM
>
>>So what does the normal page fault path do? Just invalidates the private
>>page out of the page tables. A subsequent fault goes through the normal
>>shared page path, which detects the truncation as it would with any
>>shared fault. Right?
>>
>>hugetlb seems to pretty well follow the same pattern as memory.c in this
>>regard. I don't see the race?
>>
>
>I was originally worried about a case that one thread fault on a private
>mapping and get hold of a fresh page via alloc_huge_page(). While it executes
>clear_huge_page(), 2nd thread come by did a ftruncate. After first thread
>finish zeroing, I thought it will happily install a pte. But no, the inode
>size check will prevent that from happening.
>

Yes it should do.

>I was mislead by the comments in hugetlb_no_page() that page lock is used to
>guard against racing truncation.  Now I'm drifting back into what "racing
>truncation" the comment is referring to. What race does it trying to protect
>with page lock?
>

Probably attempting to fix a similar race as the do_no_page vs 
invalidate race
that I've been trying to fix for normal pages (and is currently handled for
truncate with truncate_count). However AFAIKS it is not page lock which 
protects
from truncate, but the page_table_lock (which the ptl scalability work might
have broken):

Here is the race for regular pagecache:
                     
check i_size          
find_get_page         
                       i_size_write
                       unmap_mapping_range
set_pte               
                       truncate_inode_pages

So we have now mapped a truncated page. I fix this by using find_lock_page
in the filemap_nopage path, but I found that it isn't enough alone.
truncate_inode_pages must also check whether the page is mapped (while
holding the page lock) and be prepared to unmap (like
invalidate_inode_pages does).

Now, when looking at hugepages, it seems to be designed to give you the
unmap_mapping_range vs fault exclusion with ptl (note that it rechecks
i_size inside ptl). However now I think unmap_mapping_range runs without
ptl, you again have a race.

I'll look into it a bit more and see if I can fix it up in my patchset.

--

Send instant messages to your online friends http://au.messenger.yahoo.com 

--
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>

  reply	other threads:[~2006-11-02  3:06 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-26 22:17 Chen, Kenneth W
2006-10-26 22:44 ` Andrew Morton
2006-10-26 23:31   ` 'David Gibson'
2006-10-27  0:04     ` Andrew Morton
2006-10-27  3:11       ` 'David Gibson'
2006-10-27  3:35         ` Andrew Morton
2006-10-27  4:06           ` 'David Gibson'
2006-10-31  2:54             ` Chen, Kenneth W
2006-10-31  3:17               ` 'David Gibson'
2006-10-31  5:15                 ` Chen, Kenneth W
2006-10-31 11:05                   ` 'David Gibson'
2006-10-31 12:48                     ` Hugh Dickins
2006-11-01  6:18                       ` Nick Piggin
2006-11-01 10:17                         ` Chen, Kenneth W
2006-11-02  3:06                           ` Nick Piggin [this message]
2006-11-02  2:29                       ` 'David Gibson'
2006-10-27  1:47     ` 'David Gibson'
2006-10-30 20:55       ` Adam Litke
2006-10-26 23:47 ` '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=454960BA.9070801@yahoo.com.au \
    --to=nickpiggin@yahoo.com.au \
    --cc=agl@us.ibm.com \
    --cc=akpm@osdl.org \
    --cc=bill.irwin@oracle.com \
    --cc=christoph@schroedinger.engr.sgi.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=g@ozlabs.org \
    --cc=hugh@veritas.com \
    --cc=kenneth.w.chen@intel.com \
    --cc=linux-mm@kvack.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