mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org
Cc: Justin Forbes <jmforbes@linuxtx.org>,
	Zwane Mwaikambo <zwane@arm.linux.org.uk>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Dave Jones <davej@redhat.com>,
	Chuck Wolber <chuckw@quantumlinux.com>,
	Chris Wedgwood <reviews@ml.cw.f00f.org>,
	Michael Krufky <mkrufky@linuxtv.org>,
	Chuck Ebbert <cebbert@redhat.com>,
	Domenico Andreoli <cavokz@gmail.com>, Willy Tarreau <w@1wt.eu>,
	Rodrigo Rubira Branco <rbranco@la.checkpoint.com>,
	Jake Edge <jake@lwn.net>, Eugene Teo <eteo@redhat.com>,
	torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk,
	Lee Schermerhorn <lee.schermerhorn@hp.com>,
	Rik van Riel <riel@redhat.com>
Subject: [patch 01/33] Manually revert "mlock: downgrade mmap sem while populating mlocked regions"
Date: Wed, 4 Feb 2009 10:34:15 -0800	[thread overview]
Message-ID: <20090204183415.GB13936@kroah.com> (raw)
In-Reply-To: <20090204183403.GA13936@kroah.com>

[-- Attachment #1: manually-revert-mlock-downgrade-mmap-sem-while-populating-mlocked-regions.patch --]
[-- Type: text/plain, Size: 4888 bytes --]

2.6.28-stable review patch.  If anyone has any objections, please let us know.

------------------

From: Linus Torvalds <torvalds@linux-foundation.org>

commit 27421e211a39784694b597dbf35848b88363c248 upstream.

This essentially reverts commit 8edb08caf68184fb170f4f69c7445929e199eaea.

It downgraded our mmap semaphore to a read-lock while mlocking pages, in
order to allow other threads (and external accesses like "ps" et al) to
walk the vma lists and take page faults etc.  Which is a nice idea, but
the implementation does not work.

Because we cannot upgrade the lock back to a write lock without
releasing the mmap semaphore, the code had to release the lock entirely
and then re-take it as a writelock.  However, that meant that the caller
possibly lost the vma chain that it was following, since now another
thread could come in and mmap/munmap the range.

The code tried to work around that by just looking up the vma again and
erroring out if that happened, but quite frankly, that was just a buggy
hack that doesn't actually protect against anything (the other thread
could just have replaced the vma with another one instead of totally
unmapping it).

The only way to downgrade to a read map _reliably_ is to do it at the
end, which is likely the right thing to do: do all the 'vma' operations
with the write-lock held, then downgrade to a read after completing them
all, and then do the "populate the newly mlocked regions" while holding
just the read lock.  And then just drop the read-lock and return to user
space.

The (perhaps somewhat simpler) alternative is to just make all the
callers of mlock_vma_pages_range() know that the mmap lock got dropped,
and just re-grab the mmap semaphore if it needs to mlock more than one
vma region.

So we can do this "downgrade mmap sem while populating mlocked regions"
thing right, but the way it was done here was absolutely not correct.
Thus the revert, in the expectation that we will do it all correctly
some day.

Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 mm/mlock.c |   47 ++---------------------------------------------
 1 file changed, 2 insertions(+), 45 deletions(-)

--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -293,14 +293,10 @@ static inline int __mlock_posix_error_re
  *
  * return number of pages [> 0] to be removed from locked_vm on success
  * of "special" vmas.
- *
- * return negative error if vma spanning @start-@range disappears while
- * mmap semaphore is dropped.  Unlikely?
  */
 long mlock_vma_pages_range(struct vm_area_struct *vma,
 			unsigned long start, unsigned long end)
 {
-	struct mm_struct *mm = vma->vm_mm;
 	int nr_pages = (end - start) / PAGE_SIZE;
 	BUG_ON(!(vma->vm_flags & VM_LOCKED));
 
@@ -313,20 +309,8 @@ long mlock_vma_pages_range(struct vm_are
 	if (!((vma->vm_flags & (VM_DONTEXPAND | VM_RESERVED)) ||
 			is_vm_hugetlb_page(vma) ||
 			vma == get_gate_vma(current))) {
-		long error;
-		downgrade_write(&mm->mmap_sem);
-
-		error = __mlock_vma_pages_range(vma, start, end, 1);
 
-		up_read(&mm->mmap_sem);
-		/* vma can change or disappear */
-		down_write(&mm->mmap_sem);
-		vma = find_vma(mm, start);
-		/* non-NULL vma must contain @start, but need to check @end */
-		if (!vma ||  end > vma->vm_end)
-			return -ENOMEM;
-
-		return 0;	/* hide other errors from mmap(), et al */
+		return __mlock_vma_pages_range(vma, start, end, 1);
 	}
 
 	/*
@@ -437,41 +421,14 @@ success:
 	vma->vm_flags = newflags;
 
 	if (lock) {
-		/*
-		 * mmap_sem is currently held for write.  Downgrade the write
-		 * lock to a read lock so that other faults, mmap scans, ...
-		 * while we fault in all pages.
-		 */
-		downgrade_write(&mm->mmap_sem);
-
 		ret = __mlock_vma_pages_range(vma, start, end, 1);
 
-		/*
-		 * Need to reacquire mmap sem in write mode, as our callers
-		 * expect this.  We have no support for atomically upgrading
-		 * a sem to write, so we need to check for ranges while sem
-		 * is unlocked.
-		 */
-		up_read(&mm->mmap_sem);
-		/* vma can change or disappear */
-		down_write(&mm->mmap_sem);
-		*prev = find_vma(mm, start);
-		/* non-NULL *prev must contain @start, but need to check @end */
-		if (!(*prev) || end > (*prev)->vm_end)
-			ret = -ENOMEM;
-		else if (ret > 0) {
+		if (ret > 0) {
 			mm->locked_vm -= ret;
 			ret = 0;
 		} else
 			ret = __mlock_posix_error_return(ret); /* translate if needed */
 	} else {
-		/*
-		 * TODO:  for unlocking, pages will already be resident, so
-		 * we don't need to wait for allocations/reclaim/pagein, ...
-		 * However, unlocking a very large region can still take a
-		 * while.  Should we downgrade the semaphore for both lock
-		 * AND unlock ?
-		 */
 		__mlock_vma_pages_range(vma, start, end, 0);
 	}
 


  reply	other threads:[~2009-02-04 18:37 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20090204182823.831027530@mini.kroah.org>
2009-02-04 18:34 ` [patch 00/33] 2.6.28-stable review Greg KH
2009-02-04 18:34   ` Greg KH [this message]
2009-02-04 18:34   ` [patch 02/33] xen: make sysfs files behave as their names suggest Greg KH
2009-02-04 18:34   ` [patch 03/33] sata_mv: fix 8-port timeouts on 508x/6081 chips Greg KH
2009-02-04 18:34   ` [patch 04/33] m68knommu: set NO_DMA Greg KH
2009-02-04 18:34   ` [patch 05/33] PCI/MSI: bugfix/utilize for msi_capability_init() Greg KH
2009-02-04 18:34   ` [patch 06/33] x86: use early clobbers in usercopy*.c Greg KH
2009-02-04 18:34   ` [patch 07/33] cifs: make sure we allocate enough storage for socket address Greg KH
2009-02-04 18:34   ` [patch 08/33] netfilter: ctnetlink: fix scheduling while atomic Greg KH
2009-02-04 18:34   ` [patch 09/33] orinoco: move kmalloc(..., GFP_KERNEL) outside spinlock in orinoco_ioctl_set_genie Greg KH
2009-02-04 18:34   ` [patch 10/33] fbdev/atyfb: Fix DSP config on some PowerMacs & PowerBooks Greg KH
2009-02-04 18:34   ` [patch 11/33] kmalloc: return NULL instead of link failure Greg KH
2009-02-04 18:34   ` [patch 12/33] sata_nv: rename nv_nf2_hardreset() Greg KH
2009-02-04 18:34   ` [patch 13/33] sata_nv: fix MCP5x reset Greg KH
2009-02-04 18:34   ` [patch 14/33] sata_nv: ck804 has borked hardreset too Greg KH
2009-02-04 18:34   ` [patch 15/33] Fix memory corruption in console selection Greg KH
2009-02-04 18:34   ` [patch 16/33] Add enable_ms to jsm driver Greg KH
2009-02-04 18:34   ` [patch 17/33] Input: atkbd - Samsung NC10 key repeat fix Greg KH
2009-02-04 18:34   ` [patch 18/33] nfsd: only set file_lock.fl_lmops in nfsd4_lockt if a stateowner is found Greg KH
2009-02-04 18:34   ` [patch 19/33] nfsd: Ensure nfsv4 calls the underlying filesystem on LOCKT Greg KH
2009-02-04 18:34   ` [patch 20/33] iwlwifi: fix rs_get_rate WARN_ON() Greg KH
2009-02-04 18:34   ` [patch 21/33] p54: fix lm87 checksum endianness Greg KH
2009-02-04 18:34   ` [patch 22/33] p54: fix p54_read_eeprom to cope with tx_hdr_len Greg KH
2009-02-04 18:35   ` [patch 23/33] p54usb: rewriting rx/tx routines to make use of usb_anchors facilities Greg KH
2009-02-04 18:35   ` [patch 24/33] minstrel: fix warning if lowest supported rate index is not 0 Greg KH
2009-02-04 18:35   ` [patch 25/33] PCI: irq and pci_ids patch for Intel Tigerpoint DeviceIDs Greg KH
2009-02-04 18:35   ` [patch 26/33] cpuidle: Add decaying history logic to menu idle predictor Greg KH
2009-02-04 18:35   ` [patch 27/33] ACPI: Avoid array address overflow when _CST MWAIT hint bits are set Greg KH
2009-02-04 18:35   ` [patch 28/33] video: always update the brightness when poking "brightness" Greg KH
2009-02-04 18:35   ` [patch 29/33] Newly inserted battery might differ from one just removed, so Greg KH
2009-02-04 18:35   ` [patch 30/33] ACPI: Do not modify SCI_EN directly Greg KH
2009-02-04 18:35   ` [patch 31/33] dlm: initialize file_lock struct in GETLK before copying conflicting lock Greg KH
2009-02-04 18:35   ` [patch 32/33] sata_mv: Fix chip type for Hightpoint RocketRaid 1740/1742 Greg KH
2009-02-04 18:35   ` [patch 33/33] ACPICA: Allow multiple backslash prefix in namepaths Greg KH

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=20090204183415.GB13936@kroah.com \
    --to=gregkh@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=cavokz@gmail.com \
    --cc=cebbert@redhat.com \
    --cc=chuckw@quantumlinux.com \
    --cc=davej@redhat.com \
    --cc=eteo@redhat.com \
    --cc=jake@lwn.net \
    --cc=jmforbes@linuxtx.org \
    --cc=lee.schermerhorn@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkrufky@linuxtv.org \
    --cc=rbranco@la.checkpoint.com \
    --cc=rdunlap@xenotime.net \
    --cc=reviews@ml.cw.f00f.org \
    --cc=riel@redhat.com \
    --cc=stable@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=w@1wt.eu \
    --cc=zwane@arm.linux.org.uk \
    /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®