All of lore.kernel.org
 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 [RFC] reduce hugetlb_instantiation_mutex usage 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 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.