linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Chen, Kenneth W" <kenneth.w.chen@intel.com>
To: 'David Gibson' <david@gibson.dropbear.id.au>
Cc: wli@holomorphy.com, 'Andrew Morton' <akpm@osdl.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: RE: [patch] hugetlb strict commit accounting
Date: Thu, 9 Mar 2006 03:43:12 -0800	[thread overview]
Message-ID: <200603091143.k29BhAg19160@unix-os.sc.intel.com> (raw)
In-Reply-To: <20060309112635.GB9479@localhost.localdomain>

David Gibson wrote on Thursday, March 09, 2006 3:27 AM
> Um... as far as I can tell, this patch doesn't actually reserve
> anything.  There are no changes to the fault handler ot
> alloc_huge_page(), so there's nothing to stop PRIVATE mappings dipping
> into the supposedly reserved pool.

Well, the reservation is already done at mmap time for shared mapping. Why
does kernel need to do anything at fault time?  Doing it at fault time is
an indication of weakness (or brokenness) - you already promised at mmap
time that there will be a page available for faulting.  Why check them
again at fault time?

I don't think your implementation handles PRIVATE mapping either, Isn't it?
Private mapping doesn't enter into the page cache hanging of address_space
pointer, so either way, it is busted.


> This looks a bit like a case of "let's make it an atomic_t to sprinkle
> it with magic atomicity dust" without thinking about what operations
> are and need to be atomic.  I think resv_huge_pages should be an
> ordinary int, but protected by a lock (exactly which lock is not
> immediately obvious).

Yeah, I agree.  It crossed my mind whether I should fix that or post a
fairly straightforward back port.  I decided to do the latter and I got
bitten :-(  That is in the pipeline if people agree that this variable
reservation system is a better one.


> What is the list of regions (mapping->private_list) protected by?
> mmap_sem (the only thing I can think of off hand that's already taken)
> doesn't cut it, because the mapping can be accessed by multiple mms.

I think it is the inode->i_mutex.

- Ken

--
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-03-09 11:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-09 10:55 Chen, Kenneth W
2006-03-09 11:26 ` 'David Gibson'
2006-03-09 11:43   ` Chen, Kenneth W [this message]
2006-03-09 12:06     ` 'David Gibson'
2006-03-09 12:31       ` Chen, Kenneth W
2006-03-09 12:54         ` 'David Gibson'
2006-03-09 12:02   ` Chen, Kenneth W
2006-03-09 12:14     ` 'David Gibson'
2006-03-09 12:14 Chen, Kenneth W
2006-03-10  0:45 Chen, Kenneth W
2006-03-10  2:38 ` '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=200603091143.k29BhAg19160@unix-os.sc.intel.com \
    --to=kenneth.w.chen@intel.com \
    --cc=akpm@osdl.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=wli@holomorphy.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